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.

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

wants to merge 2 commits into from

Conversation

carsonRadtke
Copy link

@carsonRadtke carsonRadtke commented Jun 24, 2022

Fixes #5364
Update documentation regarding the output of the diff. Describe the difference between lines starting with +, -, and . Also added a snipped to the text printed by Rustfmt in the check optflag section.

Fixes #5376
Also documented that Rustfmt will exit with code 0 when reading from stdin despite seeing formatting errors. I found evidence of this being a design decision here: #2495. I guess the emacs package for Rust passes through stdin when formatting, but requires a 0 exit code. Although I should note that this doesn't appear to be the case today. I observed emacs not caring when Rustfmt returned 1 due to formatting issues.

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

Thanks so much for working on this, and excellent finds on the past decisions! Some inline thoughts left below

Comment on lines +102 to +106
"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.",
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".

@@ -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

@@ -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.

@ytmimi ytmimi added the p-low label Jul 28, 2022
@carsonRadtke carsonRadtke closed this by deleting the head repository Oct 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants