Skip to content

Doc updates for --check #5408

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

Closed
wants to merge 2 commits into from
Closed

Doc updates for --check #5408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fab5f76
docs clarification. diff & --check w/ stdin
carsonRadtke Jun 24, 2022
06f489d
mention stdin in --help:check
carsonRadtke Jun 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ To format individual files or arbitrary codes from stdin, the `rustfmt` binary s
examples follow:

- `rustfmt lib.rs main.rs` will format "lib.rs" and "main.rs" in place
- `rustfmt` will read a code from stdin and write formatting to stdout
- `rustfmt` will read a code from stdin and write formatting to stdout. When reading from stdin, `rustfmt` will not exit with an error code unless there were parsing errors or interal `rustfmt` errors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it may be worth just describing the exit code behavior for both scenarios in this section too (the files input mode), as I think it's not just the stdin input mode that could use more clarity.

What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If any, I think the README should have the most information. Adding more information to increase clarity sounds good. I will try to rework the entire paragraph and see where it gets me.

- `echo "fn main() {}" | rustfmt` would emit "fn main() {}".

For more information, including arguments and emit options, see `rustfmt --help`.
Expand All @@ -126,6 +126,11 @@ make any formatting changes to the input, and `1` if Rustfmt would make changes.
In other modes, Rustfmt will exit with `1` if there was some error during
formatting (for example a parsing or internal error) and `0` if formatting
completed without error (whether or not changes were made).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my comment on the help text, I think we should shuffle this around to be explicit and remove any potential ambiguity

If Rustfmt decides formatting is required, a diff will be printed. The diff has
the following format:
- Lines that start with `-` are poorly formatted.
- Lines that start with `+` are after formatting.
- All other lines are properly formated.



Expand Down
7 changes: 5 additions & 2 deletions src/bin/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,11 @@ fn make_opts() -> Options {
opts.optflag(
"",
"check",
"Run in 'check' mode. Exits with 0 if input is formatted correctly. Exits \
with 1 and prints a diff if formatting is required.",
"Run in 'check' mode. Exits with 0 if input is formatted correctly or reading \
from stdin. Exits with 1 and prints a diff if formatting is required. When \
reading a diff lines that start with '-' are poorly formatted, lines that \
start with '+' are after formatting, and all other lines are properly \
formatted.",
Comment on lines +102 to +106
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may just want to overhaul this altogether to make things more clear, and perhaps differentiate the output vs. exit code. Obviously not this literally, but what about a pivot to something like:

"Run in 'check' mode. The exact output and behavior codes vary depending on whether you are running rustfmt against stdin or files.

For file input, rustfmt will exit with 0 if the input is formatted correctly. If formatting changes are required then rustfmt will exit with a 1 and print a diff.

For stdin input, .....

The diff output, lines with +, -, etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and actually thinking about this more, in light of how wordy this could get, it might be worth trying to follow the same model we do with this (a short help text, and then an extended, more detailed version), e.g. --help=file-lines, but with --help=check

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's best to keep it brief unless the user specifically asks for it, so the --help=check sounds good to me. I will try to keep it brief and then add a "For more info, try --help=check".

);
let is_nightly = is_nightly();
let emit_opts = if is_nightly {
Expand Down