-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
Fix: Clarify line breaks in object-curly-newline (fixes #14024) #14063
Conversation
@@ -1,10 +1,10 @@ | |||
# enforce consistent line breaks inside braces (object-curly-newline) | |||
# enforce consistent line breaks after opening and before closing braces (object-curly-newline) |
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!
Can we make the same change ("inside braces" -> "after opening and before closing braces") here, in descriptions of string options "never" and "always":
"always"
requires line breaks after opening and before closing braces"never"
disallows line breaks after opening and before closing braces
Also, can we update meta description to be the same as the new title: "enforce consistent line breaks after opening and before closing braces".
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.
Right now this is similar to https://eslint.org/docs/rules/array-bracket-newline.
I think both of them should be written the same way.
I can change this one and make another PR for array-bracket-newline. WDYT?
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.
Sorry for the delay. Yes, sounds like a good plan!
docs/rules/object-curly-newline.md
Outdated
|
||
A number of style guides require or disallow line breaks inside of object braces and other tokens. | ||
|
||
## Rule Details | ||
|
||
This rule enforces consistent line breaks inside braces of object literals or destructuring assignments. | ||
This rule enforces consistent line breaks after opening and before closing braces of object literals or destructuring assignments. |
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.
Thoughts about adding another sentence below this one, to be even more specific?
Maybe something like: "More precisely, the rule requires or disallows a line break between {
and its following token, and between }
and its preceding token."
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.
What about replacing
This rule enforces consistent line breaks after opening and before closing braces of object literals or destructuring assignments
with:
More precisely, the rule requires or disallows a line break between { and its following token, and between } and its preceding token
?
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.
Makes sense to merge these two sentences into one if it would be more understandable.
Pardon me for I haven't noticed your code review, my mailbox might have had a problem in receiving emails. |
31d7325
to
a7022c6
Compare
I changed the PR title to tag this as |
docs/rules/object-curly-newline.md
Outdated
|
||
A number of style guides require or disallow line breaks inside of object braces and other tokens. | ||
|
||
## Rule Details | ||
|
||
This rule enforces consistent line breaks inside braces of object literals or destructuring assignments. | ||
More precisely, the rule requires or disallows a line break between { and its following token, and between } and its preceding token of object literals or destructuring assignments. |
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.
More precisely, the rule requires or disallows a line break between { and its following token, and between } and its preceding token of object literals or destructuring assignments. | |
This rule requires or disallows a line break between `{` and its following token, and between `}` and its preceding token of object literals or destructuring assignments. |
Since this is now the first sentence in this section, it wouldn't be clear what "More precisely" refers to.
Also added backticks around {
and }
a7022c6
to
d6aa4a7
Compare
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, thanks!
Thanks for contributing! |
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
What changes did you make? (Give an overview)
I changed some sentences to clarify what inside braces mean for this rule.
Is there anything you'd like reviewers to focus on?
No.