doc: fix instruction about running Rustfmt from source code

[xxchan]

cargo-fmt uses rustfmt from PATH, so cargo run --bin cargo-fmt still uses installed rustfmt instead of built one.

[xxchan]

I also tried RUSTFMT="..." cargo fmt directly, but it usually leads to dyld[19277]: Library not loaded: @rpath/librustc_driver-85e0fbb5b589cfca.dylib

I'm not sure what's the most idiomatic way you usually do @calebcartwright

[ytmimi]

Thanks for the PR, but the current recommendation of running rustfmt with cargo run --bin cargo-fmt or cargo run --bin rustfmt is correct.

[ytmimi]

Maybe I was too quick to close this. We probably want to update the recommendation to say cargo run --bin rustfmt instead of cargo run --bin cargo-fmt.

Also, @xxchan dyld[19277]: Library not loaded: @rpath/librustc_driver-85e0fbb5b589cfca.dylib is related to #5675, and that's partially why we recommend using cargo run to run rustfmt from source while you're developing rustfmt.

[xxchan]

I know cargo run --bin rustfmt works, but I do need a way to run cargo fmt with compiled rustfmt.

[xxchan]

Also, @xxchan dyld[19277]: Library not loaded: @rpath/librustc_driver-85e0fbb5b589cfca.dylib is related to #5675, and that's partially why we recommend using cargo run to run rustfmt from source while you're developing rustfmt

This is also why I wanted to install rustfmt and run cargo +nightly-x fmt.. #5755

[ytmimi]

Got it. Assuming the user hasn't compiled rustfmt maybe we want to add that as an extra step in the instructions or change the command to:

cargo build -bin rustfmt && RUSTFMT="./target/debug/rustfmt" cargo run --bin cargo-fmt -- --manifest-path path/to/project/you/want2test/Cargo.toml

[xxchan]

Added the build step, and also mentioned cargo run --bin rustfmt

[ytmimi]

@xxchan I think you pushed to wrong branch by accident. I'm seeing those changes applied to #5839

[xxchan]

Yes 🤣

[calebcartwright]

I'm fine with this as is (I have run into the current non-ideal behavior myself on occasion, but I haven't heard of anyone else doing so while developing rustfmt)

However, this line is starting to get a bit long. What if we went with the one-liner as your original proposal and just included a note above or below (something to the effect of "you may need to build rustfmt first)"

The audience for these docs are strictly rustfmt developers, and I'm content structuring the docs under the assumption they already know/can easily determine this is one binary invoking another and that a build might be involved

[ytmimi]

@xxchan thanks for helping to improve the docs. If you could rebase on the current master I can go ahead and merge.

[ytmimi]

@xxchan If possible could you squash these into a single commit.

[calebcartwright]

Agreed they should be squashed @ytmimi though just want to note we can also do that on behalf of contributors as part of the merge strategy:

[ytmimi]

@calebcartwright appreciate you pointing that out. I knew we had that capability. I asked to give @xxchan the opportunity to squash the commits themselves and amend the commit message if they'd like to before merging

[xxchan]

Thanks, but I prefer simply clicking the button. 😄

…

On Wed, 19 Jul 2023 at 19:18, Yacin Tmimi ***@***.***> wrote: @calebcartwright <https://github.com/calebcartwright> appreciate you pointing that out. I knew we had that capability. I asked to give @xxchan <https://github.com/xxchan> the opportunity to squash the commits themselves and amend the commit message if they'd like to before merging — Reply to this email directly, view it on GitHub <#5838 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJBQZNPE5QZP2OAEECUHTRTXRAJH7ANCNFSM6AAAAAA2MX554Y> . You are receiving this because you were mentioned.Message ID: ***@***.***>