-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Update: improve location for semi and comma-dangle #12380
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.
LGTM, thanks! This approach seems fine to me, but let’s see what other have to say.
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!
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.
One small change request. Also, could you please add unit tests for the new ast-utils method?
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 to me, thanks!
Note that we still need two more team members to 👍 this PR before we can accept.
@kaicataldo Until the linked issue is accepted, would you like to champion this PR's proposal? |
Yes, I’ll champion this! |
LGTM. Thanks for contributing! |
What is the purpose of this pull request? (put an "X" next to item)
[x] Changes an existing rule
What changes did you make? (Give an overview)
Change the report location for semi and comma-dangle. Fixes part of #12334 .
Is there anything you'd like reviewers to focus on?
When reporting the location for a linefeed, there is two reasonable way to do so, the first is to report
{start: {line: m, column: n}, end: {line: m, column: n 1}}
. This approach does not work in VSCode. The second way is to report{start: {line: m, column: n}, end: {line: m 1, column: 0}}
. This does work in VSCode, except for when that start location is the location of EOF. In that case I chose to not report end location.