-
-
Notifications
You must be signed in to change notification settings - Fork 32
feat: add support for getLocFromIndex and getIndexFromLoc
#212
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
feat: add support for getLocFromIndex and getIndexFromLoc
#212
Conversation
…c' of https://github.com/eslint/rewrite into feat-add-support-for-getlocfromindex-and-getindexfromloc
…c' of https://github.com/eslint/rewrite into feat-add-support-for-getlocfromindex-and-getindexfromloc
5bde96b to
92f1094
Compare
|
Thank you for your thorough review! I've resolved all the comments and added the test cases you suggested in #212 (comment) and other comments. The bug you pointed out in #212 (comment) was caused by the following code: this.text.slice(this.#lineStartIndices.at(-1), index + 1)Since I used the I've now fixed this in 842dfb8, so the error no longer occurs. The other private lazy methods already use the approach I implemented in 842dfb8, so the bug was only present in the |
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! Leaving open for @nzakas to verify changes after his approval.
|
ping @nzakas |
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!
Prerequisites checklist
What is the purpose of this pull request?
Hello,
In this PR, I've completed implemention of
getLocFromIndex()andgetIndexFromLoc().I based the implementation on the current JavaScript
SourceCodeclass and incorporated the fast lookup algorithm discussed in eslint/eslint#19782.The logic of
getLocFromIndex()andgetIndexFromLoc()is nearly identical to that in the JavaScriptSourceCodeclass, with the key difference being that it accounts for thelineStartandcolumnStartvalues initialized in the constructor.lineStartandcolumnStartTextSourceCodeBaseshould acceptlineStartandcolumnStart, as these can vary depending on the language, and bothgetLocFromIndex()andgetIndexFromLoc()rely on them for accurate calculations. Here are some examples:What changes did you make? (Give an overview)
I've completed implemention of
getLocFromIndex()andgetIndexFromLoc().Related Issues
fixes: #166
refs: eslint/markdown#376, eslint/css#167, eslint/json#109, eslint/eslint#19782
Is there anything you'd like reviewers to focus on?
Prerequisites
getLocFromIndexeslint#19782