Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rustfmt nightly feature in rustfmt stable help #4798

Closed
silvestrst opened this issue Apr 12, 2021 · 8 comments
Closed

rustfmt nightly feature in rustfmt stable help #4798

silvestrst opened this issue Apr 12, 2021 · 8 comments
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors

Comments

@silvestrst
Copy link

silvestrst commented Apr 12, 2021 •
edited
Loading

Describe the bug

Nightly --file-lines feature description has creeped into the stable rustfmt --help. It is a minor thing but can be misleading. For example, the following description makes it seem like this is a valid option:

-h, --help [=TOPIC] Show this message or help about a specific topic:
                        `config` or `file-lines`

To Reproduce

rustfmt --help
...
-h, --help [=TOPIC] Show this message or help about a specific topic:
                                 `config` or `file-lines`
...
rustfmt -h file-lines
If you want to restrict reformatting to specific sets of lines, you can
use the `--file-lines` option. Its argument is a JSON array of objects
with `file` and `range` properties, where `file` is a file name, and
`range` is an array representing a range of lines like `[7,13]`. Ranges
are 1-based and inclusive of both end points. Specifying an empty array
will result in no files being formatted. For example,

rustfmt --file-lines '[
    {"file":"src/lib.rs","range":[7,13]},
    {"file":"src/lib.rs","range":[21,29]},
    {"file":"src/foo.rs","range":[10,11]},
    {"file":"src/foo.rs","range":[15,15]}]'

would format lines `7-13` and `21-29` of `src/lib.rs`, and lines `10-11`,
and `15` of `src/foo.rs`. No other files would be formatted, even if they
are included as out of line modules from `src/lib.rs`.
...
rustfmt --file-lines
Unrecognized option: 'file-lines'

Expected behavior

When --file-lines option is applied, it is not recognised.

Meta

  • Version: rustfmt 1.4.36-stable (7de6968 2021-02-07)
  • Installed via rustup
  • Run directly
@silvestrst silvestrst added the bug Panic, non-idempotency, invalid code, etc. label Apr 12, 2021
@calebcartwright calebcartwright removed the bug Panic, non-idempotency, invalid code, etc. label Apr 16, 2021
@calebcartwright
Copy link
Member

Thanks for sharing. Running rustfmt --help enumerates the list of supported options and with stable that does not include --file-lines. As such it feels like a bit of a stretch to conflate -h file-lines not causing some kind of error behavior as meaning the option is actually available.

That being said I'm open to any improvement that doesn't add too much complexity and happy to review a proposed change if anyone's up for a PR.

This might be a good place to start for anyone interested

rustfmt/src/bin/main.rs

Lines 433 to 445 in 0bd2b19

fn determine_operation(matches: &Matches) -> Result<Operation, OperationError> {
if matches.opt_present("h") {
let topic = matches.opt_str("h");
if topic == None {
return Ok(Operation::Help(HelpOp::None));
} else if topic == Some("config".to_owned()) {
return Ok(Operation::Help(HelpOp::Config));
} else if topic == Some("file-lines".to_owned()) {
return Ok(Operation::Help(HelpOp::FileLines));
} else {
return Err(OperationError::UnknownHelpTopic(topic.unwrap()));
}
}

@calebcartwright calebcartwright added the good first issue Issues up for grabs, also good candidates for new rustfmt contributors label Apr 16, 2021
@murchu27
Copy link
Contributor

Mind if I give it a go?

@silvestrst
Copy link
Author

Mind if I give it a go?

If you were asking me, then of course, please do!

@murchu27
Copy link
Contributor

Hey folks, been looking at this a few evenings this week trying to wrap my head around it. Just a little bit confused about how the repo is managed; hope I haven't missed something simple...

@calebcartwright I notice that snippet you pasted in your above comment is from the rustfmt-1.4.37 branch, where cli options are being handled with the make_opts() function. In master though, a struct called Opt is being used, which is derived from structopt::StructOpt, a struct that handles the checking of cli options, and the generation of help messages.

What is the master branch representing exactly? And which of these two implementations should I be working off to try and fix the issue?

@calebcartwright
Copy link
Member

What is the master branch representing exactly? And which of these two implementations should I be working off to try and fix the issue?

Very fair question! I'd intentionally pointed you to the 1.4.37 branch because that's what should be targeted for working on this issue, though I should've stated it more explicitly. For now, please focus on that one and working with the existing code there. If afterwards you're interested in also targeting the unreleased 2.0 experiment then we can certainly talk through that as well.

You may find #4801 somewhat illuminating on the topic of branches, but don't get bogged down in the variance and just focus on 1.x branch.

Please let me know if you have any other questions

@murchu27
Copy link
Contributor

Thanks for that! Yeah the first thing I did after cloning was go to that branch, but then I was getting errors when trying to compile (which turned out to be an issue completely on my side, don't worry...), and in the process of trying to understand those, I had switched to master to try and figure things out there. I guess my thought process was "master will work! master always works!`

I'll stick with rustfmt-1.4.37 for now, and I'll see if I can figure out masterafter that!

@murchu27 murchu27 mentioned this issue Apr 24, 2021
Closed
@murchu27
Copy link
Contributor

Just opened #4812, let me know if it needs any work!

@murchu27 murchu27 mentioned this issue Jun 10, 2021
Merged
@calebcartwright
Copy link
Member

Closed via #4865

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@calebcartwright @murchu27 @silvestrst