Skip to content

Conversation

DavisVaughan
Copy link
Collaborator

@DavisVaughan DavisVaughan commented Jun 4, 2025

Closes #353 in favor of this simpler approach (though there are a few things from that PR ill add in later on in another PR)

This is a minimal PR that gets rid of our dependency on biome_lsp_converters in favor of the lighter weight biome_line_index. We use biome_lsp_converters for two things:

  • The LSP agnostic LineIndex type, which is extremely useful for conversion between byte offset and line/column position
  • A few LSP specific conversion utilities:
    • from_proto::offset()
    • from_proto::text_range()
    • to_proto::position()
    • to_proto::range()

The painful thing about biome_lsp_converters is that it locks us in to tower_lsp, therefore also locking us to tokio, due to the fact that their LSP specific conversion utilities use tower_lsp::lsp_types::*.

I'm planning on sending a PR to biome (biomejs/biome#6222) to request that they split out the LSP agnostic LineIndex type into a biome_line_index crate which I've gone ahead and copy/pasted in here. That allows us to use their fantastic LineIndex implementation, but critically we now don't rely on tower-lsp through them in any way. I'm hopeful they accept that PR, at which point we can drop our shim in favor of their real thing (we will also need to sync with biome main to do that though, which may be painful)

This will allow us to switch over to lsp-server without any issues. Previously, we were blocked from doing this due to our deep reliance on tower-lsp through biome-lsp-converters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore, this is Biome code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore, this is Biome code

Avoiding the tower-lsp dependency entirely in favor of "just" the `LineIndex`. We handle our own converter utilities, which are very easy to write on top of the LSP agnostic `LineIndex`.
@DavisVaughan DavisVaughan force-pushed the feature/biome-line-index branch from c5649b3 to 1ec29b9 Compare June 4, 2025 18:20
@DavisVaughan DavisVaughan merged commit 24a7181 into main Jun 10, 2025
4 checks passed
@DavisVaughan DavisVaughan deleted the feature/biome-line-index branch June 10, 2025 19:11
@DavisVaughan
Copy link
Collaborator Author

I'm hopeful this one is not worrisome in any way (since we agreed it's a good idea in general) so I'm going to merge so I can do some additional follow up / refactoring

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.

1 participant