-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
style-guide: Document newline rules for assignment operators #113145
style-guide: Document newline rules for assignment operators #113145
Conversation
The style guide gives general rules for binary operators including assignment, and one of those rules says to put the operator on the subsequent line; the style guide needs to explicitly state the exception of breaking *after* assignment operators rather than before. This is already what rustfmt does and what users do; this fixes the style guide to match the expected default style.
(rustbot has picked a reviewer for you, use r? to override) |
Some changes occurred in src/doc/style-guide cc @rust-lang/style |
While I'm personally in agreement with this change, I wonder if it will need an FCP? I could certainly see both sides of "this is a bug in the Style Guide" and "this is a bug in Rustfmt" |
@calebcartwright Given the stability guarantees rustfmt and the style-edition RFC provide, I don't think changing rustfmt (for the 2015 style edition) would be an option here even if we wanted to. If we wanted to change rustfmt to do what the current style guide claims is the default style, we'd have to do so in the 2024 style edition and that would need an FCP. |
I'm going to go ahead and start an FCP, but I also think we should talk in the next meeting about whether we think "fix style guide to match rustfmt" needs an FCP in general. @rfcbot merge |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Apologies, I was trying to quickly and briefly share a thought and failed to include relevant context. To be clear, I don't know if this needs an FCP, and I'm not actively pushing for one. It's more that, to the best of my recollection, we hadn't yet decided how we wanted to handle this specific case outside the 2024 style edition boundary. I opened rust-lang/style-team#179 so that we could keep track of it, and we had some informal discussion in zulip, but I wasn't sure if we ever reached a consensus. I'm in favor of making this change (certainly as part of 2024 style edition, and I'm open to doing it earlier), though I feel we should either discuss first, or, get everyone's buy in here (whether that's reactions, GitHub PR approvals, FCP, etc. doesn't matter to me) |
Some further info that may be helpful, as evidence that this is likely a textual bug in the style guide: The style guide, in |
I don't know if this needs an FCP either; I just figured we could make the decision for if things like this need one in parallel with kicking one off in case the answer is "yes". |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
@rfcbot concern all-boxes-should-be-checked |
Since 1) we have checkboxes from everyone anyway, and 2) we agreed in today's @rust-lang/style meeting that bugfixes to make the style guide match rustfmt don't require an FCP, I'm going to go ahead and merge this based on team member reviews rather than waiting for 10 days. @bors r+ |
@bors rollup |
…fee1-dead Rollup of 9 pull requests Successful merges: - rust-lang#111119 (style-guide: Add chapter about formatting for nightly-only syntax) - rust-lang#112791 (llvm ffi: Expose `CallInst->setTailCallKind`) - rust-lang#113145 (style-guide: Document newline rules for assignment operators) - rust-lang#113163 (Add a regression test for rust-lang#112895) - rust-lang#113332 (resolve: Use `Interned` for some interned structures) - rust-lang#113334 (Revert the lexing of `c"…"` string literals) - rust-lang#113350 (Fix the issue of wrong diagnosis for extern pub fn) - rust-lang#113371 (Fix submodule handling when the current branch is named after a tag) - rust-lang#113384 (style-guide: Clarify grammar for small patterns (not a semantic change)) 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.
The style guide gives general rules for binary operators including
assignment, and one of those rules says to put the operator on the
subsequent line; the style guide needs to explicitly state the exception
of breaking after assignment operators rather than before.
This is already what rustfmt does and what users do; this fixes the
style guide to match the expected default style.