-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
jsx-wrap-multilines now catches single missing newlines #1984
Conversation
var Hello = createReactClass({ | ||
render: function() { | ||
return ( | ||
|
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.
These extra newlines here seem odd - why is this allowed to be here?
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.
See my earlier comment:
#1984 (comment)
My first attempts at fixing this caused extraneous newlines to disappear which seemed like an undesired side-effect of this particular rule. The extra newline isn't being added by the fix -- its in the original test condition. The test is making sure the extra newline is not being stripped.
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.
That makes sense (although I’d want it stripped, this probably isn’t the rule for it)
Thanks for the review @ljharb. What's the next step to getting this merged? Anything I need to do? |
Fixes #1965. The only interesting question is how the fixer should treat the newlines. I chose to make it add a new line if there was exactly zero after or before the parens. This means:
Fixes to:
And:
Fixes to:
The alternative would be to make exactly one newline regardless of how many there were before -- but I assumed such a behavior should be its own rule instead of attached to this rule.