Add config option to control leading match arm pipes

[calebcartwright]

Resolves #3973 (at least the leading pipe, not the => on its own line)

[topecongiro]

IMO we should support the following three variants for this config option:

• Always remove the leading | (this should be the default)
• Always add the leading |
• Keep the presence or the absence of the leading | (this corresponds to strip_leading_match_arm_pipes == false)

I think that the second variant is essential for those who want to have the leading pipe consistently throughout their codebase. What do you think?

[calebcartwright]

I think that the second variant is essential for those who want to have the leading pipe consistently throughout their codebase. What do you think?

Yeah that sounds reasonable 👍 Perhaps something like the below?

• AlwaysAdd
• AlwaysRemove (default)
• KeepExisting

[topecongiro]

Yeah, that seems reasonable to me!

[topecongiro]

LGTM. @calebcartwright Are you planning to add any updates to this PR? If not, feel free to merge by yourself at any point.

[RalfJung]

Thanks a lot, this is awesome. :)
I particularly like KeepExisting because it means I can use what locally makes sense, but still have rustfmt otherwise ensure the formatting is good. I hope this is something rustfmt can do for other aspects of match formatting as well in the future, such as whether to have the expression after => in the same line as the arrow or not.

[calebcartwright]

Thanks a lot, this is awesome. :)
I particularly like KeepExisting because it means I can use what locally makes sense, but still have rustfmt otherwise ensure the formatting is good.

Glad to hear it @RalfJung!

I hope this is something rustfmt can do for other aspects of match formatting as well in the future, such as whether to have the expression after => in the same line as the arrow or not.

I suspect that match formatting configurability will definitely be an area of review and consideration for us once we get rustfmt 2.0 shipped (which will include this option), especially since there's quite a few match-related bugs and feature requests open.

As usual the defaults for any such future config option(s) would need to adhere to the style guide but I do think there's some opportunities to give users some opt-in control/flexibility via config options.

We'll certainly consider the benefit/possibility of KeepExisting-like variants on a case by case basis as well, along with any associated complexity/idempotency challenges such a variant may carry.

[RalfJung]

I'm eager to try this out. :) What would it take to get this into a nightly? Can I just submit a PR bumping the submodule in rustc or are there things to be aware of?

[calebcartwright]

I'm eager to try this out. :) What would it take to get this into a nightly? Can I just submit a PR bumping the submodule in rustc or are there things to be aware of?

We should be ready to update the submod in rustc with a 2.0 release candidate within the next week or two, but we're not quite ready just yet. The only way to try it out at the moment would be building rustfmt from master.

[RalfJung]

Oh I didn't know there was a major release coming, I'll wait then. :)

[RalfJung]

I am now using rustfmt in the latest rust nightly

$ rustfmt --version
rustfmt 1.4.18-nightly (c1e9b7b 2020-06-13)

but this option still does not seem to be available? For testing, I added this to my config

match_arm_leading_pipes = "Always"

but there is no effect. On the terminal I can even see

Warning: Unknown configuration option `match_arm_leading_pipes`

[RalfJung]

Oh, looks like rust nightly is not tracking rustfmt master.

[calebcartwright]

Oh, looks like rust nightly is not tracking rustfmt master.

That's correct. master is rustfmt 2.0 which has one more issue left (#2908) that we need to get sorted before 2.0 is ready for nightly

I was actually thinking about this one the other day and wondering if folks think Preserve would be a better name than KeepExisting?

[RalfJung]

Preserve sounds a bit nicer, I feel (but I am not a native speaker).

[karyon]

backported in #4436