Skip to content

match arm grouping with | can make it hard to distinguish groups of arms #1129

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

Closed
nikomatsakis opened this issue Aug 17, 2016 · 2 comments
Closed

Comments

@nikomatsakis
Copy link

nikomatsakis commented Aug 17, 2016

rustfmt renders the following match as it is shown:

fn foo() {
        match *self {
            NotInRegistry(..) |
            FailedToDownload(..) |
            TestFail(..) |
            BuildFail(..) => None,
            FailedToDownload(_, ref e) => Some(&**e),
        }
}

Personally, I find that it is visually challenging to identify the "groups" of arms here. This would be even more true if (e.g.) BuildFail(..) were a longer pattern like BuildFail(ref a, SomeStruct { b, c }) etc. The problem is that then you have to scan pretty far to the right to find if there is a => or a |. I have definitely had cases in rustc where I thought the grouping was one thing but it was another.

I would prefer a newline after the =>:

fn foo() {
        match *self {
            NotInRegistry(..) |
            FailedToDownload(..) |
            TestFail(..) |
            BuildFail(..) =>
                None,
            FailedToDownload(_, ref e) =>
                Some(&**e),
        }
}

Or maybe, though it's not what I would do myself, a newline separating the groups:

fn foo() {
        match *self {
            NotInRegistry(..) |
            FailedToDownload(..) |
            TestFail(..) |
            BuildFail(..) => None,

            FailedToDownload(_, ref e) => Some(&**e),
        }
}

My rule of thumb is to put a newline after the => if there are | patterns involved (and also to try and put newlines after the => if any other groups of patterns have a newline), unless the expression is a block expression (=> {\n).

I am not really familiar with the totality of the match heuristics involved in rustfmt so I can't say how this fits in.

@jethrogb
Copy link

This is #490

@nrc
Copy link
Member

nrc commented Oct 26, 2016

Closing as a dup of #490, and note that the fmt-rfcs repo is probably the best place to discuss this.

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

3 participants