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: ignore aligning single line in key-spacing (fixes #11414) #12652
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.
Thanks for working on this!
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'm not sure this fixes the problem in full (see comments). That being said, I recognize that the behavior I'm suggesting below is a lot more work. I would be fine with landing this as-is because it's passing through the options and does seem like an improvement 👍
output: [ | ||
"var obj = {", | ||
" foo : 1,", | ||
" 'bar' : 2, baz : 3, longlonglong: 4", |
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 understand why this is the way it is, but I wonder if this actually makes sense?
Feels like it should be:
var obj = {
foo : 1,
'bar': 2, baz: 3, longlonglong: 4
};
or
var obj = {
foo: 1, 'bar' : 2,
baz: 3, longlonglong: 4
};
output: [ | ||
"var obj = {", | ||
" foo : 1,", | ||
" 'bar' : 2, baz : 3,", |
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.
Same comment as above.
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.
@kaicataldo
Yes, I agree :) Can I work on that in different PR?.
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.
Also fine landing as-is. Thanks for delving into this rule, @yeonjuan!
What is the purpose of this pull request? (put an "X" next to item)
[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
fix #11414
make it ignore aligning single line properties.
Is there anything you'd like reviewers to focus on?