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

[unstable option] match_block_trailing_comma #3380

Closed
scampi opened this issue Feb 13, 2019 · 10 comments · Fixed by #4145
Closed

[unstable option] match_block_trailing_comma #3380

scampi opened this issue Feb 13, 2019 · 10 comments · Fixed by #4145
Labels
1x-backport:completed unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for unstable option: match_block_trailing_comma

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@scampi scampi changed the title [unstable option] unstable option: match_block_trailing_comma [unstable option] match_block_trailing_comma Feb 18, 2019
@Veykril
Copy link
Member

Veykril commented Oct 27, 2019

Hey, just wanted to ask whether there is any specific reason for this option still being unstable or if it just has gone out of sight over time.

@TheDan64
Copy link

I am also interested in seeing this stabilized. It seems like a simple enough feature

emilio added a commit to emilio/rustfmt that referenced this issue Apr 30, 2020
Servo has used this since forever, and it'd be useful to be able to use
rustfmt stable there so that we can use the same rustfmt version in
both Firefox and Servo.

Feel free to close this if there's any reason it shouldn't be done.

Closes rust-lang#3380
calebcartwright pushed a commit that referenced this issue May 16, 2020
Servo has used this since forever, and it'd be useful to be able to use
rustfmt stable there so that we can use the same rustfmt version in
both Firefox and Servo.

Feel free to close this if there's any reason it shouldn't be done.

Closes #3380
bradleypmartin pushed a commit to bradleypmartin/rustfmt that referenced this issue May 25, 2020
Servo has used this since forever, and it'd be useful to be able to use
rustfmt stable there so that we can use the same rustfmt version in
both Firefox and Servo.

Feel free to close this if there's any reason it shouldn't be done.

Closes rust-lang#3380
@Keavon
Copy link

Keavon commented Feb 19, 2021

Since apparently 2.0 isn't even on the horizon yet according to @calebcartwright, is there any way we could get this (seemingly relatively simple) feature backported to a release in the 1.x series and promoted to stable? It is a rather glaring omission from common-practice styling rules.

@calebcartwright
Copy link
Member

It is a rather glaring omission from common-practice styling rules.

I'm not sure what common rules you're referring to here. The Style Guide for Rust code that governs the default, standard formatting style (which is what is emitted by rustfmt) explicitly codifies the absence of trailing commas

From the match section of the guide:

Use a trailing comma for a match arm if and only if not using a block.

Additionally, while match expressions aren't unique to Rust, they also aren't ubiquitous in the programming language landscape either; much less having a canonical practice to always have trailing commas.

is there any way we could get this (seemingly relatively simple) feature backported to a release in the 1.x series and promoted to stable?

I appreciate the benefits the configuration options provide and that folks often get frustrated when an option they want hasn't yet been stabilized. However, there's a few important things to note both here and for future reference:

  • We've got documentation that enumerates the requirements for stabilizing an option
  • We do not stabilize any option unless and until it can easily clear those thresholds, regardless of whether it's 3 days old or 3 years old
  • The seeming "simplicity" of an option isn't really a gauge for stabilization. with code formatting, even the simplest of things will always have the potential for weird edge cases (often caused by developers placing comments in places one would never expect)
  • Stability is a huge, critical part of Rust, and that theme, along with our formatting stability guarantee, are taken very seriously and that often means moving slowly with option stabilization. Premature stabilization is absolutely unacceptable; we need to be confident the option will work everywhere, not just that it worked one time somewhere
  • Like most open source projects, we've an ever growing backlog of work and limited bandwidth to address that work. Accordingly, stabilizing config options, particularly those that go explicitly against the Style Guide prescriptions, is a relatively low priority

That being said, I will try to backport the stabilization of this particular option in an next release or two

@Keavon
Copy link

Keavon commented Feb 19, 2021

Thank you for the explanation and context!

I should have probably worded what I meant more clearly, but I was referring to the general growing pattern in the programming world to prefer using trailing commas wherever possible. Lots of languages like JS are updating their specs to allow them in additional places, because of the benefits they provide (not having to touch other lines when adding or rearranging lines, which helps with convenience and avoiding version control bloat). I didn't mean to imply that rustfmt was operating against the Rust project's style guide.

I was under the impression that the feature got promoted to stable in the 2.0 release because the feature, including its challenging potential edge cases, was found to be safe— however that promotion only made it into master and not a 1.x release. I'm very glad the project is putting such a high degree of focus into stability and correctness since I fully agree it's one of the most attractive features about the Rust ecosystem. Thank you for being willing to backport this feature for an upcoming release despite your limited time amidst all the tasks, I really appreciate it.

@Keavon
Copy link

Keavon commented Mar 31, 2021

Hi @calebcartwright, I was wondering if you might have any updates about backporting this feature. It looks like there are a number of people interested based on the ❤️ reactions to your reply. Would you like me to file a separate issue related to specifically backporting this, or would reopening this one help to track this as a task for a future 1.x release version? I also noticed there is a bug label called 1x-backport:pending that looks potentially relevant. Thank you kindly.

@calebcartwright calebcartwright added the 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release label Apr 2, 2021
@calebcartwright
Copy link
Member

Thanks for the bump, labels updated accordingly. I may pull this into the upcoming 1.4.37 release, but even if not, the following 1.4.38 seems quite doable.

@est31
Copy link
Member

est31 commented Jun 7, 2021

Personally I like commas after match blocks because non-blocks do need commas and IMO it simplifies the language to just always put a , there.

So I would really like this too :).

calebcartwright pushed a commit to calebcartwright/rustfmt that referenced this issue Oct 11, 2021
Servo has used this since forever, and it'd be useful to be able to use
rustfmt stable there so that we can use the same rustfmt version in
both Firefox and Servo.

Feel free to close this if there's any reason it shouldn't be done.

Closes rust-lang#3380
@karyon
Copy link
Contributor

karyon commented Oct 25, 2021

Backport was done in #5020

@karyon karyon added 1x-backport:completed and removed 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release labels Oct 25, 2021
@calebcartwright
Copy link
Member

Thanks @karyon! I expect this will land in v1.58 for those waiting to use this on stable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1x-backport:completed unstable option tracking issue of an unstable option
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants