-
-
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
Check parentheses with new lines for multiline JSX #1475
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.
It's a breaking change to remove the previous options format; please rework this so it continues to accept the same values, but can optionally also accept your new schema.
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.
LGTM overall!
}, { | ||
code: ARROW_SINGLE_LINE, | ||
options: [] |
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.
should we not keep this test against "the default"?
code: ARROW_PAREN, | ||
options: [] |
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.
should we not keep this test against "the default"?
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.
The two tests above these are using the defaults.
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.
Thanks for clarifying!
Looks like this needs a rebase |
This changes the options from a boolean to an enum with five possible values: true, false, `ignore`, `parens`, and `parens-new-line`. True and false are there for backwards compatibility. Using `ignore` equates to using `false` while `parens` is equal to `true`. The new option (`parens-new-line`) will check that the parentheses around the multiline JSX are on separate lines for better readability.
This changes the options from a boolean to an enum with three possible
values:
ignore
,parens
, andparens-new-line
. Usingignore
is likethe previous
false
whileparens
is like the previoustrue
. The newoption (
parens-new-line
) will check that the parentheses around themultiline JSX are on separate lines for better readability.
This PR attempts to address #1469.