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

Change defaults for TrainingComma hash/array cops #219

Closed
wants to merge 1 commit into from

Conversation

skatkov
Copy link

@skatkov skatkov commented Nov 4, 2020

I think that defaults for Trailing Commas cops should be switched to EnforcedStyleForMultiline: comma.

It helps with consistency, preventing copy-paste errors, and maintaining attribution (minimizing impact on diffs).

Quoting from /r/javascript: https://www.reddit.com/r/javascript/comments/6vqsjh/whats_with_the_trailing_commas/

Here are examples from rubocop:
https://docs.rubocop.org/rubocop/1.0/cops_style.html#styletrailingcommainarrayliteral
https://docs.rubocop.org/rubocop/1.0/cops_style.html#enforcedstyleformultiline-comma-3

@searls
Copy link
Contributor

searls commented Nov 5, 2020

Sorry, we're keeping this no_comma. We'd initially set Standard up this way for about a year, but it was easily the most-complained-about setting. It also caused some obscure bugs & inconsistencies in edge cases around things like trailing commas in keyword args and hash parameters.

@searls searls closed this Nov 5, 2020
@searls searls added the rule change 👩‍⚖️ Suggest a style guide rule change label Nov 5, 2020
@sdhull
Copy link

sdhull commented Dec 10, 2020

Can you elaborate a bit on these "obscure bugs & inconsistencies in edge cases"? The rationale for enforcing trailing commas in multiline literals largely echos the rationale put forth for enforcing leading dots in multiline method chains. I don't have time to put together a sweet presentation like the linked issue but since this was the original setting I'm guessing some maintainers see the usefulness.

I'll just leave you with a quote from that thread

The git churn should be taken seriously here.
@strzibny

@skatkov skatkov deleted the trailing-comma branch December 10, 2020 12:24
@sdhull
Copy link

sdhull commented Feb 19, 2021

trailing commas in keyword args and hash parameters

@searls I think I finally understand the point here. So what about setting preference for trailing commas for hash literals but no trailing comma for method invocations? (This is what we had set up at my previous company, which felt like a good balance.)

@ElMassimo
Copy link

Is there any advantage to not allowing trailing commas?

There are certainly advantages to having them in multi-line arrays and hashes.

@Timmitry
Copy link

This setting really prevents me from adopting Standard as formatter. Especially the git churn is an issue for me, as well as the extra work when wanting to switch two lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule change 👩‍⚖️ Suggest a style guide rule change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants