-
Notifications
You must be signed in to change notification settings - Fork 32
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
Support rust-analyzer as Language Server #315
Comments
Do you know whether rust-analyzer can be installed via rustup? |
No, according to the documentation: https://rust-analyzer.github.io/manual.html#installation it's not possible at the moment. You have to download a binary or wrap it up somehow. |
Rustup installation is a must-have for proper integration in Corrosion. I suggest you open a request to rustup and/or rust-analyzer to work together and share the link here. It will be a pre-requisite. |
Thanks. This is tracked at rust-lang/rust#72978. (cc @matklad) Assuming this lands soon, what needs to be done on the plugin side to fix the interaction with it? (One can already manually test using the current installation method). |
Hopefully, a simple string replacement of |
If the reason here is "we only want to support officially endorsed and distributed language servers", then it makes sense to wait for rustup integration (I'll estimate that as a couple of months). If the reason is "we assume the user has rustup", than I'd argue it is suboptimal user experience. The best "search path" for an IDE is, I think:
|
I agree. To echo Aleksey's reply, it might be worth being more explicit about your assumptions on the tools availability. The "textbook" development workflow of installation via rustup, managing dependencies and building/testing via cargo does not apply everywhere, especially for heterogeneous code base. For instance, Android provides a prebuilt toolchain and uses an external build system. This shouldn't be the default supported case, but clarifying the assumptions would help making that scenario somewhat supported. (Although this conversation would be better continued in another bug.) |
Corrosion offers only 1 installation path, which is the default one for the language. Rustup also makes it very easy to ship Corrosion without risk of wrong IP stuff: basically, the IP and technical parts of installations and updates of the language server are externalized to Rustup and users actions, it's less responsibility and risk. However, the case of different installation is already handled: if you go to Window > Preference > Rust, you can already select "Rust Language Server Location > Other installation" to configure a different path for RLS or rust-analyzer. When rust-analyzer is properly supported by the official installation path with rustup, then we'll just switch to it; unless some blocker issues are found. |
Also cc rust-lang/rust-analyzer#4604, which documents LSP extensions we use. |
That's right and I'm fine with this approach. This was actually what I mentioned in the 1st comment of this bug: corrosion relies on the output of |
If you want to provide a patch that allows more flexibility here (ie just warn in case output cannot be processed), that would be welcome; especially since we'll need it sometime later when moving to rust-analyzer as default. |
Simply fixing the regular expression was enough to get some integration working. I'll fill up other bugs for the specific issues. Thanks. (cc @werwurm) |
Hi @tweksteen , did you identify further issues with rust-analyzer instead of RLS? |
On rust-lang/rust-analyzer#4753 (comment) , @bjorn3 explained "There is already the rust-analyzer-preview rustup component." . Is it worth switching to it soon? |
@mickaelistria No blocking issue. I did have to create a separate config file for the language server, by setting the "RLS config" field. The documented default |
Ok, and do you think rust-analyzer is already better than RLS, and that switching to it as soon as it's in "stable" rustup toolchain would make Corrosion better? |
I wouldn't make such a call myself (as I have never used RLS). It is worth pointing out that upstream is officially supporting the move via RFC 2912. Therefore, it seems that it is just a matter of time until RLS is fully deprecated. |
Is there anything left to do it when it come to allowing to use rust-analyzer; or are the current options good enough? |
We need rustup release rust-lang/rustup#2408 so there is proper proxy for rust-analyzer. Once it's there we can provide ui to switch between rls and rust-analyzer. |
@tweksteen I got some question about rust-analyzer support. Do you think you can document in Corrosion (with a PR) how one can configure Corrosion to use rust-analyzer instead of RLS? |
I stumbled upon https://blogs.gnome.org/chergert/2021/03/04/rust-and-gnome-builder/ , there is apparently a flatpak that assumes rust-analyzer is stable. I'm wondering whether we should try to get Corrosion use rust-analyzer more aggressively; for example by testing against rust-analyzer instead of rls and by hacking the preference page to give more room to rust-analyzer. |
That would be very good approach IMHO. |
Done with #357 . Snapshots promoting and prioritizing rust-analyzers should be available shortly. |
Please try https://download.eclipse.org/corrosion/snapshots/products/ and create new tickets for further issues. |
rust-analyzer is the evolution of RLS and has support for non-Cargo based projects.
Currently, corrosion requires to have RLS. RLS and rust-analyzer should be interoperable but corrosion relies on some static output of RLS (namely,
rls --version
).The text was updated successfully, but these errors were encountered: