librustdoc: adopt let else in more places #94139
Conversation
rustbot
added
the
T-rustdoc
|
Some changes occurred in cc @camelid |
|
r? @CraftSpider (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I'm not really in favor of this change. It creates a lot of git history churn, and in most cases, it doesn't seem like an improvement. I think |
est31
force-pushed
the
let_else_rustdoc
branch
from
b0e77c6
to
30b9f81
Compare
There was a problem hiding this comment.
Looks like let_else shines in two places:
- When the code is formatted so that
letandelseare both on the same column (like inunindent_comments.rs) This has a nice visual rythm to it. - Or, when the code is simple enough for the whole thing to fit in a single line.
Apart from these two cases, I'm having some difficulty with the "glanceability" of the statement, which I think might go away as I start reading and writing more code with let_else.
est31
force-pushed
the
let_else_rustdoc
branch
from
30b9f81
to
5f4da5b
Compare
Adopt let else in more places Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011. I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This is the biggest of these PRs and handles the changes outside of rustdoc, rustc_typeck, rustc_const_eval, rustc_trait_selection, which were handled in PRs rust-lang#94139, rust-lang#94142, rust-lang#94143, rust-lang#94144.
Adopt let else in more places Continuation of rust-lang#89933, rust-lang#91018, rust-lang#91481, rust-lang#93046, rust-lang#93590, rust-lang#94011. I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This is the biggest of these PRs and handles the changes outside of rustdoc, rustc_typeck, rustc_const_eval, rustc_trait_selection, which were handled in PRs rust-lang#94139, rust-lang#94142, rust-lang#94143, rust-lang#94144.
|
☔ The latest upstream changes (presumably #93244) made this pull request unmergeable. Please resolve the merge conflicts. |
|
r? @notriddle |
So, basically, this issue is blocked on rust-lang/rustfmt#4914 and rust-lang/style-team#165? |
Use ? operator in one instance instead of manual match As suggested [here](rust-lang#94139 (comment)). r? `@notriddle`
|
I've put As for blocking it on established formatting rules, you can absolutely do that, but I think one can just use whatever formatting one thinks is best and then have rustfmt reformat it once it has established a format. |
|
Ok, now that more of the let-elses have line breaks, I feel better about this change. I do think the churn is not great, but I won't block this PR on that. |
|
☔ The latest upstream changes (presumably #94009) made this pull request unmergeable. Please resolve the merge conflicts. |
Use if let instead of manual match Factored out of rust-lang#94139 . `if let` is better here than both `let ... else` and `let ... = match`.
Use if let instead of manual match Factored out of rust-lang#94139 . `if let` is better here than both `let ... else` and `let ... = match`.
Use if let instead of manual match Factored out of rust-lang#94139 . `if let` is better here than both `let ... else` and `let ... = match`.
Use if let instead of manual match Factored out of rust-lang#94139 . `if let` is better here than both `let ... else` and `let ... = match`.
|
I've updated the PR. @notriddle is there anything left to do? As for the formatting, I can do it any way you want. Rustfmt won't change anything about it until it is established how to format it. |
|
@bors r+ rollup |
|
📌 Commit 565f644 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (b751124): comparison url. Summary: This benchmark run did not return any relevant results. 16 results were found to be statistically significant but too small to be relevant. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
Continuation of #89933, #91018, #91481, #93046, #93590, #94011.
I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This PR handles librustdoc.