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 formatting of @, |, and parens patterns #4110

Closed
Centril opened this issue Apr 7, 2020 · 8 comments
Closed

Strange formatting of @, |, and parens patterns #4110

Centril opened this issue Apr 7, 2020 · 8 comments
Milestone

Comments

@Centril
Copy link

Centril commented Apr 7, 2020

In a rust-lang/rust PR, I saw some pretty strange formatting of pattern matching with @, |, and parens patterns: https://github.com/rust-lang/rust/pull/70906/files#r405165086

The diff was:

-                    category: category @ ConstraintCategory::Return,
+                    category:
+                        category
+                        @
+                        (ConstraintCategory::Return
+                        | ConstraintCategory::CallArgument
+                        | ConstraintCategory::OpaqueType),

which seems odd because it is putting @ on its own line and doing ( on the same line as a pattern and then ) also on the same line as a pattern.

I would have expected:

-                    category: category @ ConstraintCategory::Return,
+                    category: category @ (
+                        ConstraintCategory::Return
+                        | ConstraintCategory::CallArgument
+                        | ConstraintCategory::OpaqueType
+                    ),
@calebcartwright
Copy link
Member

Not a duplicate but similar report in #4031. Fixes will likely be similar (if not identical)

@topecongiro
Copy link
Contributor

Did this pattern exist before, or is it something new that has been introduced recently? Either way, we should fix this.

@calebcartwright
Copy link
Member

I believe it's a new-ish change, but not sure precisely when it was introduced

@Centril
Copy link
Author

Centril commented Apr 14, 2020

category: category @ Record { ... }

has existed since Rust 1.0 I think, so that isn't new, but p0 | ... | pn nested within another pattern is new and currently an unstable feature.

@calebcartwright
Copy link
Member

Oops, I misunderstood the question anyway. I don't know definitively but I believe that the poor binding formatting described in #4031 was a change rustfmt started doing relatively recently.

Not sure what, if any, impact that may have on the formatting here

@qm3ster
Copy link

qm3ster commented Apr 3, 2021

Happens equally as root for a match arm:

match head.packet_type()? {
    p
    @
    (PacketType::Connect
    | PacketType::ConnAck
    | PacketType::SubAck
    | PacketType::UnsubAck) => return Err(Error::WrongPacket(p)),

Manually, I would probably format it as:

match head.packet_type()? {
    p @ (
        PacketType::Connect
        | PacketType::ConnAck
        | PacketType::SubAck
        | PacketType::UnsubAck
    ) => return Err(Error::WrongPacket(p)),

Interestingly enough, we still end up with N+2 lines. But moving the output expression to the left is definitely a huge win.

@evanjs
Copy link

evanjs commented Jul 7, 2021

"Fixing" this by locally by increasing max_width for now (not really a fix...) but this is definitely something that would make rustfmt much less annoying when using nested or patterns.

I wonder if this would be more worthwhile to investigate now that rust-lang/rust#54883 is stabilized (?), etc.

@ytmimi
Copy link
Contributor

ytmimi commented Jul 26, 2022

This was fixed in #4978, and a test case was added for this issue

@ytmimi ytmimi closed this as completed Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants