Update: Add always-multiline option to multiline-ternary (fixes #8770) #8841
Conversation
|
|
|
LGTM |
|
@nwoltman, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kaicataldo and @mysticatea to be potential reviewers. |
not-an-aardvark
added
accepted
lib/rules/multiline-ternary.js
Outdated
| @@ -69,6 +71,10 @@ module.exports = { | |||
| reportError(node.consequent, node, false); | |||
| } | |||
| } else { | |||
| if (allowSingleLine && astUtils.isTokenOnSameLine(node, node)) { | |||
There was a problem hiding this comment.
Ha, interesting use of astUtils.isTokenOnSameLine().
There was a problem hiding this comment.
Not to ruin the fun, but I feel like this makes the code a bit hard to read -- semantically a node would always be on the same line as itself, and this only works due to an implementation detail of astUtils.isTokenOnSameLine where it assumed that the two tokens would be different. Personally I think it would be better to be explicit here instead:
if (allowSingleLine && node.loc.start.line === node.loc.end.line) {There was a problem hiding this comment.
Yes I agree that is more readable. Fixed 👍
|
LGTM |
|
Thanks for contributing to ESLint! 🎉 |
eslint-deprecated
bot
added
the
archived due to age
What is the purpose of this pull request? (put an "X" next to item)
[x] Changes an existing rule
See #8770
What changes did you make? (Give an overview)
This adds an
always-multilineoption tomultiline-ternary. This option enforces newlines between the operands of a ternary expression only if the expression spans multiple lines.Is there anything you'd like reviewers to focus on?
Nope.
However, I would like out point out that with the addition of this option, the name of this rule could be changed to
ternary-newlineto follow the naming convention of similar ESLint rules.