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.

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: Rewrite let-else section for clarity, without changing formatting #112912

Merged
merged 1 commit into from
Jun 24, 2023

Conversation

joshtriplett
Copy link
Member

The section as written did not cover all cases, and left some of them
implicit. Rewrite it to systematically cover all cases. Place examples
immediately following the corresponding case.

In the process, reorder to move the simplest cases first: start with
single-line and add progressively more line breaks.

This does not change the meaning of the section at all, and in
particular does not change the defined style for let-else statements.

…ormatting

The section as written did not cover all cases, and left some of them
implicit. Rewrite it to systematically cover all cases. Place examples
immediately following the corresponding case.

In the process, reorder to move the simplest cases first: start with
single-line and add progressively more line breaks.

This does not change the meaning of the section at all, and in
particular does not change the defined style for let-else statements.
@joshtriplett joshtriplett added the T-style Relevant to the style team, which will review and decide on the PR/issue. label Jun 21, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 2023

r? @compiler-errors

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 21, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 2023

Some changes occurred in src/doc/style-guide

cc @rust-lang/style

@compiler-errors compiler-errors removed their assignment Jun 22, 2023
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.

This is a cleaner read, thank you!

Comment on lines -104 to -109
If a let statement contains an `else` component, also known as a let-else statement,
then the `else` component should be formatted according to the same rules as the `else` block
in [control flow expressions (i.e. if-else, and if-let-else expressions)](./expressions.md#control-flow-expressions).
Apply the same formatting rules to the components preceding
the `else` block (i.e. the `let pattern: Type = initializer_expr ...` portion)
as described [above](#let-statements)
Copy link
Member

Choose a reason for hiding this comment

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

I forget where, but I know there's been some recent discussion around trying to encapsulate certain prescriptions in one place to avoid duplication, and I'd add, the potential for accidental drift (e.g. breaks around the assignment operator).

I completely agree with that goal, and while it's likely best punted to 2024 edition work, I feel like the basics of "how to write an else" block are something we could capture in one place and then reuse (e.g. the else keyword must be followed by a space before the opening brace, if multilined then the closing brace must be preceded by a break, etc.)

Then any syntax specific context can respectively specify where the else keyword should start, and any unique wrapping rules

@calebcartwright
Copy link
Member

Given this is tantamount to an internal word refactoring with no "external" changes (i.e. I 100% agree this doesn't change the formatting rules) I'm just going to go ahead and approve as it doesn't seem like an FCP is warranted.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 24, 2023

📌 Commit b551730 has been approved by calebcartwright

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 24, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 24, 2023
…ications, r=calebcartwright

style-guide: Rewrite let-else section for clarity, without changing formatting

The section as written did not cover all cases, and left some of them
implicit. Rewrite it to systematically cover all cases. Place examples
immediately following the corresponding case.

In the process, reorder to move the simplest cases first: start with
single-line and add progressively more line breaks.

This does not change the meaning of the section at all, and in
particular does not change the defined style for let-else statements.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2023
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#112703 ([-Ztrait-solver=next, mir-typeck] instantiate hidden types in the root universe)
 - rust-lang#112854 (fix: add cfg diagnostic for unresolved import error)
 - rust-lang#112912 (style-guide: Rewrite let-else section for clarity, without changing formatting)
 - rust-lang#112915 (Update runtests.py : grammar correction)
 - rust-lang#112971 (issue template: add clippy entry which points to the clippy repo)
 - rust-lang#112989 (Add a regression test for rust-lang#109141)
 - rust-lang#113002 (bootstrap: Backup `settings.json` to the correct filename)
 - rust-lang#113003 (Fix old python deprecation check in x.py)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6d3d775 into rust-lang:master Jun 24, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-style Relevant to the style team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants