style-guide: clean up "must"/"should"/"may" #113380
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
Some changes occurred in src/doc/style-guide cc @rust-lang/style |
joshtriplett
force-pushed
the
style-guide-cleanup-must-should-may
branch
from
115324f
to
b58b979
Compare
|
☔ The latest upstream changes (presumably #113391) made this pull request unmergeable. Please resolve the merge conflicts. |
joshtriplett
force-pushed
the
style-guide-cleanup-must-should-may
branch
from
f378942
to
9c1cc76
Compare
This comment has been minimized.
This comment has been minimized.
|
☔ The latest upstream changes (presumably #113676) made this pull request unmergeable. Please resolve the merge conflicts. |
There was a problem hiding this comment.
Mostly LGTM 👍
If you're good with removing the reorderable language in the derive section then feel free to r=me. Otherwise happy to talk about it.
The other comments are just thoughts/suggestions that I don't feel strongly about, and was even starting to second guess myself on things like "prefer" language
| `* let` expressions and before `in` in a `for` expression; the following line | ||
| should be block indented. If the control line is broken for any reason, then the | ||
| opening brace should be on its own line and not indented. Examples: | ||
| If the control line needs to be broken, prefer to break before the `=` in `* |
There was a problem hiding this comment.
Similar thought as earlier wrt to "prefer", but don't feel strongly
| If the control line needs to be broken, prefer to break before the `=` in `* | |
| If the control line needs to be broken, break before the `=` in `* |
There was a problem hiding this comment.
Here, I think that this "prefer" represents "here's how to choose among multiple possible sources of line breaks". For instance, if you have to break it in another place because this break isn't sufficient, and with it broken in that other place you don't need to break before the =, then you shouldn't actually break it before the =.
If we want to say "always break before the = if you have to break at all, even if when done you don't need that break to stay under the line width", we could make this change, but I don't think that's accurate here?
| Prefer not to line-break inside the discriminant expression. There must always | ||
| be a line break after the opening brace and before the closing brace. The match | ||
| arms must be block indented once: | ||
| Prefer not to line-break inside the discriminant expression. Always break after |
There was a problem hiding this comment.
Recognize this is another instance of existing "prefer" language. I don't think it's worth trying to change it now, but I'm curious how operative this really is. Whether the expression needs to be broken should be dependent on the respective rules for that expression, unless there's some match specific context carveout i'm nt familiar with
There was a problem hiding this comment.
@calebcartwright I think this is a semantically meaningful "prefer": if you need to break the line, and you have multiple places you could do so, try others first, and only break within the discriminant if nothing else suffices to get it below the line length.
joshtriplett
force-pushed
the
style-guide-cleanup-must-should-may
branch
from
f3e9370
to
b763b4d
Compare
The style guide discusses the default Rust style. Configurability of Rust formatting tools are not the domain of the style guide.
The style guide requires a trailing comma on where clause components, but then gives an example that doesn't include one. Add the missing trailing comma.
…change) Avoid putting a sentence fragment after a list; integrate it with the sentence before the list.
…default style
The style guide inconsistently used language like "there should be a
space" or "it should be on its own line", or "may be written on a single
line", for things that are required components of the default Rust
style. "should" and especially "may" come across as optional. While the
style guide overall now has a statement at the top that the default
style itself is a *recommendation*, the *definition* of the default
style should not be ambiguous about what's part of the default style.
Rewrite language in the style guide to only use "should" and "may" and
similar for truly optional components of the style (e.g. things a tool
cannot or should not enforce in its default configuration).
In their place, either use "must", or rewrite in imperative style ("put
a space", "start it on the same line"). The latter also substantially
reduces the use of passive voice.
This is a purely editorial change, and does not affect the semantic
definition of the Rust style.
Make it clear the rule for stacking the second line on the first applies recursively, as long as the condition holds.
…rustfmt) An example immediately following "Put each bound on its own line." did not put each bound on its own line.
Co-authored-by: Caleb Cartwright <calebcartwright@users.noreply.github.com>
joshtriplett
force-pushed
the
style-guide-cleanup-must-should-may
branch
from
b763b4d
to
77d09cb
Compare
|
Fixed the reordering language, and address the comments. (Left two instances of "prefer" where I think they're accurately expressing a preference, made changes to address other comments.) @bors r=calebcartwright |
bors
added
S-waiting-on-bors
|
@bors rollup |
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#113380 (style-guide: clean up "must"/"should"/"may") - rust-lang#113723 (Resurrect: rustc_llvm: Add a -Z `print-codegen-stats` option to expose LLVM statistics.) - rust-lang#113780 (Support `--print KIND=PATH` command line syntax) - rust-lang#113810 (Make {Rc,Arc}::allocator associated functions) - rust-lang#113907 (Minor improvements to Windows TLS dtors) Failed merges: - rust-lang#113392 (style-guide: Some cleanups from the fmt-rfcs repo history) r? `@ghost` `@rustbot` modify labels: rollup
…t, r=calebcartwright style-guide: Document style editions, start 2024 style edition Link to a snapshot for the 2015/2018/2021 style edition. This is a draft, because I'd like to wait for a few style guide fixes to merge before snapshotting the 2015/2018/2021 style edition: - rust-lang#113145 - rust-lang#113380 - rust-lang#113384 - rust-lang#113385 - rust-lang#113386 - rust-lang#113392 I'd like to wait for these for two reasons: to make it easier to see the differences between the 2015/2018/2021 style edition and the 2024 style edition (without the noise of guide-wide changes), and to minimize confusion so that bugfixes to the style guide that we include in the previous edition don't look like they're only part of the 2024 style edition. I've used "Miscellaneous `rustfmt` bugfixes" as a starting point for the list of 2024 changes, for now. We can update that when we add more 2024 changes. The section added in this PR can then serve as a baseline for our drafts of 2024 style edition changes. In the meantime, I'd like to get someone from `@rust-lang/style` to review and approve the text here; I'll update it with a commit hash when the above PRs have merged.
Avoid using "should" or "may" for required parts of the default style.
The style guide inconsistently used language like "there should be a space" or
"it should be on its own line", or "may be written on a single line", for
things that are required components of the default Rust style. "should" and
especially "may" come across as optional. While the style guide overall now has
a statement at the top that the default style itself is a recommendation, the
definition of the default style should not be ambiguous about what's part of
the default style.
Rewrite language in the style guide to only use "should" and "may" and similar
for truly optional components of the style (e.g. things a tool cannot or should
not enforce in its default configuration).
In their place, either use "must", or rewrite in imperative style ("put a
space", "start it on the same line"). The latter also substantially reduces the
use of passive voice.
Looking for "should"s also flagged some recommendations the style guide made
for configurability of tools (e.g. a tool "should" have a given configuration
option). I've removed those recommendations, per discussion with the style
team; it's not the domain of the style guide to make such recommendations, only
to define the default Rust style.
In the process of making this change, I also fixed a typo, fixed a text structure
issue, fixed an example that didn't match the Rust style (missing a trailing
comma), and added an additional example for clarity. (Those changes would have
conflicted with this one.) Those changes appear in separate commits.
These are all purely editorial changes, and do not affect the semantic
definition of the Rust style.