-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
eslint: tweak "comma-dangle" rule to accommodate curried functions #35
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. We did the same.
@@ -5,7 +5,7 @@ | |||
"arrow-body-style": ["error", "as-needed"], | |||
"arrow-parens": ["error", "as-needed"], | |||
"arrow-spacing": ["error", {"before": true, "after": true}], | |||
"comma-dangle": ["error", "always-multiline"], | |||
"comma-dangle": ["error", {"arrays": "always-multiline", "objects": "always-multiline", "imports": "always-multiline", "exports": "always-multiline", "functions": "never"}], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't want to add line breaks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, for easier sorting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following the existing formatting. This looks fine to me too:
"comma-dangle": ["error", {"arrays": "always-multiline", "objects": "always-multiline", "imports": "always-multiline", "exports": "always-multiline", "functions": "never"}], | |
"comma-dangle": [ | |
"error", | |
{ | |
"arrays": "always-multiline", | |
"objects": "always-multiline", | |
"imports": "always-multiline", | |
"exports": "always-multiline", | |
"functions": "never" | |
} | |
], |
I would like to remain consistent, though. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd vote for no line breaks over adding line breaks to every rule, if those were the only choices. I don't mind either way though, which is why I already approved the PR.
Reducing diff noise is one of the goals of this project. Trailing commas play a significant role. For example:
const tokens = [ 'foo', 'bar', 'baz', + 'quux', ];
In the Sanctuary world we only ever define curried functions. Functions are almost always applied to exactly one argument. Trailing commas are unhelpful in this context. For example:
Unless
foo
is a “foreign” function it will never be applied to more than one argument; the trailing comma does not provide a potential benefit to future diffs.This pull request tweaks the
comma-dangle
rule to prohibit rather than require a trailing comma after a function argument not immediately followed by a)
.