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

Strange linebreaks introduced in if-let expressions with let_chains #5946

Closed
RalfJung opened this issue Oct 23, 2023 · 1 comment
Closed

Comments

@RalfJung
Copy link
Member

RalfJung commented Oct 23, 2023

I haven't found a self-contained example of this yet, but this commit demonstrates the problem: rustfmt changes

                if let Ok(name) = str::from_utf8(name) && is_dyn_sym(name) {
                    let ptr = this.fn_ptr(FnVal::Other(DynSym::from_str(name)));
                    this.write_pointer(ptr, dest)?;
                } else {
                    this.write_null(dest)?;
                }

to

                if let Ok(name) = str::from_utf8(name)
                    && is_dyn_sym(name)
                {
                    let ptr = this.fn_ptr(FnVal::Other(DynSym::from_str(name)));
                    this.write_pointer(ptr, dest)?;
                } else {
                    this.write_null(dest)?;
                }

Which looks worse due to splitting the if over multiple lines, and which looks like a bug since in the original code, this line was actually shorter than the next one, so it is clearly below the threshold for breaking long lines. I've never seen normal if behave like that, so it's probably something specific to if let, maybe specific to let_chains even.

This is a recent regression, until recently rustfmt didn't change our formatting here.

@RalfJung RalfJung changed the title Strange linebreaks introduced in if-let expressions Strange linebreaks introduced in if-let expressions with let_chains Oct 23, 2023
@ytmimi
Copy link
Contributor

ytmimi commented Oct 23, 2023

This isn't a regression. rustfmt wasn't formatting things at all before, and just left the let-chain unformatted. Support for let-chains was added in #5910, and those changes were synced with r-l/rust a little while ago rust-lang/rust#117066. Formatting was implemented following the rules outlined in rust-lang/rust#110568.

Given that let-chains are still unstable the formatting rules haven't been fully decided, but we wanted to get something implemented in rustfmt sooner rather than later. The formatting is subject to change, and if you have thoughts on the formatting rules I think it would be best to reach out to @rust-lang/style

@ytmimi ytmimi closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants