Skip to content
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

Add getPosAtLineNumber #893

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from

Conversation

HoldYourWaffle
Copy link
Contributor

I have never encountered as many off-by-one-ish errors as I have today...
However, as far as I can tell this function is now working properly 😄

I also changed a comment somewhere to be more clear about what is 1-indexed.

Closes #891.

@Gerrit0
Copy link
Contributor

Gerrit0 commented Dec 6, 2020

Did you know about the getPositionOfLineAndCharacter function provided by TypeScript? It will perform significantly better over repeated calls since it caches the line positions, and avoids copying strings.

@dsherret
Copy link
Owner

dsherret commented Dec 6, 2020

Thanks @Gerrit0 for that suggestion! I didn't know about that.

@HoldYourWaffle what do you think about updating this PR to use that API? It might be nice if it caches stuff for performance and reduces allocations. I think it might be 0-indexed, but ts-morph is 1-indexed.

@HoldYourWaffle
Copy link
Contributor Author

HoldYourWaffle commented Dec 7, 2020

Sounds good! I'll hopefully get to it later today.

Edit: I found ts.getLineAndCharacterOfPosition as well, so StringUtils.getLineNumberAtPos can be replaced too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Get node at (line, col)
3 participants