-
Notifications
You must be signed in to change notification settings - Fork 27
Rearrange to group proto/ together, and remove rust_analyzer/
#363
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,17 +4,9 @@ | |
| // origin = "https://github.com/rust-lang/rust-analyzer/blob/master/crates/rust-analyzer/src/line_index.rs" | ||
| // --- | ||
|
|
||
| //! Enhances `ide::LineIndex` with additional info required to convert offsets | ||
| //! into lsp positions. | ||
| use settings::LineEnding; | ||
| use triomphe::Arc; | ||
|
|
||
| use crate::proto::PositionEncoding; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| pub struct LineIndex { | ||
| pub index: Arc<biome_line_index::LineIndex>, | ||
| pub endings: LineEnding, | ||
| pub encoding: PositionEncoding, | ||
| } | ||
|
Comment on lines
9
to
12
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because each conversion function now takes the Instead the I think this makes a lot of sense, because we throw away and rebuild the This also becomes apparent in my next PR to optimize |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,14 +4,13 @@ use biome_line_index::LineIndex; | |
| use biome_line_index::WideLineCol; | ||
| use tower_lsp::lsp_types; | ||
|
|
||
| use crate::documents::Document; | ||
| use crate::proto::PositionEncoding; | ||
|
|
||
| /// The function is used to convert a LSP position to TextSize. | ||
| /// From `biome_lsp_converters::from_proto::offset()`. | ||
| pub(crate) fn offset( | ||
| line_index: &LineIndex, | ||
| position: lsp_types::Position, | ||
| line_index: &LineIndex, | ||
| position_encoding: PositionEncoding, | ||
| ) -> anyhow::Result<biome_text_size::TextSize> { | ||
| let line_col = match position_encoding { | ||
|
|
@@ -36,42 +35,11 @@ pub(crate) fn offset( | |
| /// The function is used to convert a LSP range to TextRange. | ||
| /// From `biome_lsp_converters::from_proto::text_range()`. | ||
| pub(crate) fn text_range( | ||
| line_index: &LineIndex, | ||
| range: lsp_types::Range, | ||
| line_index: &LineIndex, | ||
| position_encoding: PositionEncoding, | ||
| ) -> anyhow::Result<biome_text_size::TextRange> { | ||
| let start = offset(line_index, range.start, position_encoding)?; | ||
| let end = offset(line_index, range.end, position_encoding)?; | ||
| let start = offset(range.start, line_index, position_encoding)?; | ||
| let end = offset(range.end, line_index, position_encoding)?; | ||
| Ok(biome_text_size::TextRange::new(start, end)) | ||
| } | ||
|
|
||
| pub fn apply_text_edits( | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only actually used by the client during testing, so it moved there. |
||
| doc: &Document, | ||
| mut edits: Vec<lsp_types::TextEdit>, | ||
| ) -> anyhow::Result<String> { | ||
| let mut text = doc.contents.clone(); | ||
|
|
||
| // Apply edits from bottom to top to avoid inserted newlines to invalidate | ||
| // positions in earlier parts of the doc (they are sent in reading order | ||
| // accorder to the LSP protocol) | ||
| edits.reverse(); | ||
|
|
||
| for edit in edits { | ||
| let start: usize = offset( | ||
| &doc.line_index.index, | ||
| edit.range.start, | ||
| doc.line_index.encoding, | ||
| )? | ||
| .into(); | ||
| let end: usize = offset( | ||
| &doc.line_index.index, | ||
| edit.range.end, | ||
| doc.line_index.encoding, | ||
| )? | ||
| .into(); | ||
|
|
||
| text.replace_range(start..end, &edit.new_text); | ||
| } | ||
|
|
||
| Ok(text) | ||
| } | ||
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.
I'd like to argue that we should drop this wrapper entirely at this point. We currently have no reason to wrap this in an
Arc. I know rust-analyzer does this, but I think we should simplify things until we start to use salsa, and let the implementation built around salsa guide whether or not we should arc this.