Use line-index crate for offset to position conversions#325
Merged
vinistock merged 1 commit intoNov 18, 2025
Merged
Conversation
vinistock
commented
Nov 14, 2025
| None => (0, 0, 0, 0), | ||
| }; | ||
|
|
||
| let line_index = LineIndex::new(&source); |
Member
Author
There was a problem hiding this comment.
We will need to think about how to structure this API in the future, but for now I'm creating a new line index every time.
jesse-shopify
approved these changes
Nov 17, 2025
jesse-shopify
left a comment
jesse-shopify
left a comment
Contributor
There was a problem hiding this comment.
This is a very nice change. Good find.
vinistock
deleted the
11-14-use_line-index_crate_for_offset_to_position_conversions
branch
November 18, 2025 14:43
Morriar
reviewed
Nov 18, 2025
| } | ||
|
|
||
| fn parse_location_positions(location: &str) -> (String, Position, Position) { | ||
| fn parse_location_positions(location: &str) -> (String, LineCol, LineCol) { |
Contributor
There was a problem hiding this comment.
Could we keep our Position abstraction on top of LineCol so we can add the features we need to Position and also keep the same naming through the Rust and Ruby APIs?
Member
Author
There was a problem hiding this comment.
Do we currently have any custom functionality we need on top? Or will a type alias do for now?
Contributor
There was a problem hiding this comment.
I think a type alias may suffice 👍
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In #126, I made a bit of a faux pas. I totally forgot to call attention to the fact that Rust Analyzer already publishes a number of crates for LSP related things (such as translating offsets into line/col information) and that we should check what's available.
Indeed, they do publish the
line-indexcrate, which does everything we need to translate locations including all of the different encodings.I propose that we switch to using the crate so that we can avoid having to maintain this translation. The crate supports translating in both directions and it supports handling incremental edits as well.
In the future, we can make each
Documentinstance hold their ownLineIndexinstance and we'll be able to easily translate positions in both directions - regardless of whether the document has been committed to disk or not.