feat: add support for getLocFromIndex and getIndexFromLoc - #376
Conversation
|
Before you get too far on this, is this function currently needed? As discussed in eslint/rewrite#166, this is an expensive operation and I don't think we want to incorporate it unless we absolutely need it. |
|
I thought this method could replace the
It’s currently used in two rules, and I use this kind of logic in my own rules as well (for context, please see https://github.com/lumirlumir/npm-eslint-plugin-mark/blob/main/packages/eslint-plugin-mark/src/core/ast/text-handler/text-handler.js). If this feature isn’t necessary for now, I’ll go ahead and close it. (If it is fine, I would like to keep exploring and look for a more performant solution.) |
|
@lumirlumir ah, I forgot about that. Gotcha. 👍 If you're going to go through the trouble of investigating, then I'd suggest doing it on |
|
@nzakas Thanks for understanding! I’ll keep this PR open for now, move on to implementing Can I assign myself for the issue I've created? eslint/rewrite#166 |
|
Go ahead. |
|
The other thing to keep in mind is that Line 71 in cb5a956 It's finding the offset inside a given node's text. |
lumirlumir
changed the title
getLocFromIndexgetLocFromIndex and getIndexFromLoc
|
Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update. |
| "dependencies": { | ||
| "@eslint/core": "^0.14.0", | ||
| "@eslint/plugin-kit": "^0.3.1", | ||
| "@eslint/plugin-kit": "^0.4.0", |
There was a problem hiding this comment.
As @eslint/plugin-kit v0.4.0 has been released, we could now continue work here?
There was a problem hiding this comment.
There was some delay on my end with the markdown plugin. I'll work on it soon! 👍
| describe("getLocFromIndex()", () => { | ||
| it("should convert index to location correctly", () => { | ||
| const text = "foo\nbar\r\nbaz"; | ||
| const markdownSourceCode = new MarkdownSourceCode({ |
There was a problem hiding this comment.
I've used the markdownSourceCode name here because the following errors occur:
110:10 error 'sourceCode' is already declared in the upper scope on line 63 column 6 no-shadow
173:10 error 'sourceCode' is already declared in the upper scope on line 63 column 6 no-shadow
3 participants
Prerequisites checklist
What is the purpose of this pull request?
In this PR, I've updated the
@eslint/plugin-kitand@eslint/coredependencies to add support for thegetLocFromIndexandgetIndexFromLocmethods.Since these methods were already well tested in the
rewriterepository, I've only added a few simple tests to verify their behavior.What changes did you make? (Give an overview)
In this PR, I've updated the
@eslint/plugin-kitand@eslint/coredependencies to add support for thegetLocFromIndexandgetIndexFromLocmethods.Related Issues
Ref: eslint/rewrite#212
Is there anything you'd like reviewers to focus on?
N/A
Prerequisite
getLocFromIndexandgetIndexFromLocrewrite#212getLocFromIndexeslint#19782