Support let-chains #5910
Support let-chains #5910
Conversation
|
Hooray! Thanks for working on this. I assume this will cover #4955 too? |
| Hi { friend } if let None = friend => {} | ||
| aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | ||
| if let Some(foooooooooooooo) = hiiiiiiiiiiiiiii => {} | ||
| aaaaaaaaaaaaaaaaa | ||
| if let Superman { | ||
| powers: Some(goteem), | ||
| .. | ||
| } = all::get_random_being::<Super>() => {} |
There was a problem hiding this comment.
I assume this will cover #4955 too?
@calebcartwright yup, though this seems to be the only test for it (pulled from the original PR). Do we want to support if-let guard formatting with this one or should we hold off on that until a future PR?
|
What's blocking this PR from landing? |
Caleb and his review capacity 👀 |
There was a problem hiding this comment.
There's a couple positions where comments could potentially appear, though they're not "natural" positions comments would often be placed and even if they do, rustfmt will "recover" them anyway since the underlying ast construct is an expression.
Could be worth a todo in the code, but doesn't need to block. Feel free to merge when ready, want to pull this in before running the release train
|
@calebcartwright I'm happy to add a TODO comment (in rewrite_let I assume), We currently don't handle comments well between binary operators #3591, So I'm guessing these are the case that you're thinking about: fn main() {
if aaaa && let /* comment */ Some(b) = foo() {}
if cccc && let Some(d) /* comment */ = foo() {}
} |
Correct, I was referring to comments contained within the let expression's span |
for now, let-chains can only be formatted on a single line if the chain consits of 2 expressions where the first is an identifier proceeded by any number of unary operators and the second is a let-expr.
|
@calebcartwright I added some TODO comments |
…Lapkin,Nilstrieb Format all the let-chains in compiler crates Since rust-lang/rustfmt#5910 has landed, soon we will have support for formatting let-chains (as soon as rustfmt syncs and beta gets bumped). This PR applies the changes [from master rustfmt to rust-lang/rust eagerly](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/out.20formatting.20of.20prs/near/374997516), so that the next beta bump does not have to deal with a 200+ file diff and can remain concerned with other things like `cfg(bootstrap)` -- rust-lang#113637 was a pain to land, for example, because of let-else. I will also add this commit to the ignore list after it has landed. The commands that were run -- I'm not great at bash-foo, but this applies rustfmt to every compiler crate, and then reverts the two crates that should probably be formatted out-of-tree. ``` ~/rustfmt $ ls -1d ~/rust/compiler/* | xargs -I@ cargo run --bin rustfmt -- `@/src/lib.rs` --config-path ~/rust --edition=2021 # format all of the compiler crates ~/rust $ git checkout HEAD -- compiler/rustc_codegen_{gcc,cranelift} # revert changes to cg-gcc and cg-clif ``` cc `@rust-lang/rustfmt` r? `@WaffleLapkin` or `@Nilstrieb` who said they may be able to review this purely mechanical PR :> cc `@Mark-Simulacrum` and `@petrochenkov,` who had some thoughts on the order of operations with big formatting changes in rust-lang#95262 (comment). I think the situation has changed since then, given that let-chains support exists on master rustfmt now, and I'm fairly confident that this formatting PR should land even if *bootstrap* rustfmt doesn't yet format let-chains in order to lessen the burden of the next beta bump.
…lstrieb Format all the let-chains in compiler crates Since rust-lang/rustfmt#5910 has landed, soon we will have support for formatting let-chains (as soon as rustfmt syncs and beta gets bumped). This PR applies the changes [from master rustfmt to rust-lang/rust eagerly](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/out.20formatting.20of.20prs/near/374997516), so that the next beta bump does not have to deal with a 200+ file diff and can remain concerned with other things like `cfg(bootstrap)` -- #113637 was a pain to land, for example, because of let-else. I will also add this commit to the ignore list after it has landed. The commands that were run -- I'm not great at bash-foo, but this applies rustfmt to every compiler crate, and then reverts the two crates that should probably be formatted out-of-tree. ``` ~/rustfmt $ ls -1d ~/rust/compiler/* | xargs -I@ cargo run --bin rustfmt -- `@/src/lib.rs` --config-path ~/rust --edition=2021 # format all of the compiler crates ~/rust $ git checkout HEAD -- compiler/rustc_codegen_{gcc,cranelift} # revert changes to cg-gcc and cg-clif ``` cc `@rust-lang/rustfmt` r? `@WaffleLapkin` or `@Nilstrieb` who said they may be able to review this purely mechanical PR :> cc `@Mark-Simulacrum` and `@petrochenkov,` who had some thoughts on the order of operations with big formatting changes in rust-lang/rust#95262 (comment). I think the situation has changed since then, given that let-chains support exists on master rustfmt now, and I'm fairly confident that this formatting PR should land even if *bootstrap* rustfmt doesn't yet format let-chains in order to lessen the burden of the next beta bump.
This PR builds off of the work from #5203, and extendeds it to support the single-line formatting rules described in r-l/rust#110568
r? @calebcartwright