-
Notifications
You must be signed in to change notification settings - Fork 165
Support rust-analyzer as alternate LSP engine #793
Conversation
Co-authored-by: Vitalii Kryvenko <[email protected]>
Co-authored-by: Aleksey Kladov <[email protected]>
} | ||
|
||
/** Returns a path where persistent data for rust-analyzer should be installed. */ | ||
function metadataDir(): string | undefined { |
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.
@matklad fyi it'd be great to support something like rust-analyzer --version --tag
or simply include just it in the output to drop the persistent data logic (that bit is copied in spirit from the existing rust-analyzer extension) and query the binary itself
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.
The problem here is that nightly
tag moves. I guess, we can change version string to rust-analyzer yyyy-mm-dd [nightly]
. Yeah, I if I am not missing something, this would probably be a better way to handle this.
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.
Though, @Xanewok, could you explain why we need to effectively implement our own persistance here? Shouldn't this be handled by VS Code persistance mechanism? Ie, this:
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.
Shouldn't this be handled by VS Code persistance mechanism?
You're absolutely right! What I did at the beginning was to implement everything except the persistence just to get it working; while polishing it I just bolted it on thinking it's implemented similarly (didn't know about the Memento API). It'd be a good idea to drop this altogether :)
To de-emphasize the current RLS and emphasize that we use either RLS or Rust Analyzer as our underlying language server.
I've been trying to test this extensively in both modes on both Linux and Windows and it seems more or less things are in place and not regressed. What's left is reworking the extension settings a bit, e.g. to separate rustup or rls-specific ones rather than lumping them all together but let's do that as a part of a next PR. |
(I want to review this closesly, but I am pretty swamped at the moment, but, anyway, I am wondering, what would be the easiest way to bridge the rest of rust-analyzer's typescript extension here...) |
One thing I may have missed in looking at this are support for call hierarchy and semantic tokens: https://github.com/rust-analyzer/rust-analyzer/blob/master/editors/code/src/client.ts#L50-L51 |
@kjeremy I've had some problems with module imports while hacking up the prototype and just used more broad This covers more features than call hierarchy and semantic token but does include it as well and looks to be working (although only sending that after modifying the file? might have to do with not porting the |
This mostly reshuffles stuff around and makes some bits more generic and less tied to the current RLS engine.
The behaviour can be changed by setting
rust-client.engine
accordingly, and now the client implementations live atrls.ts
andrustAnalyzer.ts
for the respective engine.Particularly, the workspace configuration is no longer synchronized for every
rust
-prefixed setting and the list is hardcoded to the existingrust.<CFG>
settings for backwards compatibility - this is to prevent RLS from warning about unrecognized future settings and that's because we want to possibly unify all of our configuration under a singlerust
prefix (rather than have a separaterust-client
for client-relevant bits).Previously we spawned each instance of RLS lazily depending on the Rust file that was opened (we crawled the FS and determined a project root where to spawn RLS) but Rust Analyzer currently eagerly crawls every opened workspace (not in sense of an opened text editor tab but rather active workspace folders). To keep the existing logic somewhat compatible, we keep the multi-project layout but only spawn and refer to a singleton client in the rustAnalyzer.ts to keep the implementation simple.
What's left is tweaking the README, documenting more rust-analyzer-specific options (ideally coupled with general settings refresh), reword some tasks and UI to not refer to RLS unconditionally and wait for rust-analyzer to use more LSP-compliant messages in order to fully support its capabilities 😅
cc @matklad
Fixes #780
Addresses #781 partially