Skip to content
This repository has been archived by the owner on Nov 18, 2022. It is now read-only.

🔥 merge with rust-analyzer extension 🔥 #812

Open
matklad opened this issue Jul 8, 2020 · 17 comments
Open

🔥 merge with rust-analyzer extension 🔥 #812

matklad opened this issue Jul 8, 2020 · 17 comments
Assignees

Comments

@matklad
Copy link
Member

matklad commented Jul 8, 2020

So, as of today rust-analyzer is available via rustup, which probably is a good place to start actively meting TypeScript code bases.

This is the meta issue to discuss this.

I don't have a particular plan in mind, but here are some thoughts:

Location:

The unified extension should live in this repository. Long term, I still moderately strongly believe that a monorepo makes more sense long term, but:

  • it's not that important, and many people think that a separate repo makes more sense
  • while we support both rls and rust-analyzer, it certainly makes more sense to use a separate repo.

Leadership:

Who would be primarily "in charge" of unification effort and subsequent maintenance of unified codebase? Are there any volunteers?

Implementation:

I see roughly two ways to implement the big picture:

  • "dispatch" between two extensions -- have rls_activate/deactivate and ra_activate/deactivate functions, and call one of them from the top-level entry point. This way, the code of extensions themselves is mostly unmodified, there's just a single top-level shim added
  • "proper" merge into unified code bases, where different parts potentially know about each other
@Xanewok
Copy link
Member

Xanewok commented Jul 9, 2020

Since I started the merging effort, I'll gladly bring this over the finish line.

With regards to the implementation, I'd lean towards the "proper" approach that was started in #793. From what I can tell, people have been using it with more or less a success modulo some missing pieces/customizations that were not supported here. We can keep going and incrementally merge them into one.

Location

As long as the location of the extension goes, I'd say this should be live in the same place as other Rust editor extensions, even if we decide to support support only rust-analyzer at some point.

There's nothing inherently rust-analyzer about it and so I believe this better separates the concerns, so to speak. If there ever will be built-in LSP in rustc itself (as a possible path of migration) we'll be faced with the same dilemma whether to move.

Additionally, in retrospect I believe that having both the editor extension and the LSP together made custom extensions a lot more tempting, which in turn blessed a single editor and made the support in other editors somewhat lacking (e.g. see vim support thread and the initiative to cut down on extensions in rust-analyzer).

As such, I'd be in favor of keeping them separate long-term and rather finding more ways to facilitate the development on both tools, which I understand is the primary motivation for keeping them together in a monorepo.

🚲 🏠

I think it'd be good to rename the extension to something like vscode-rust to be more in line with the rest of the language extension and since it's not going to be rls-vscode for much longer. What do you think about it?

@matklad
Copy link
Member Author

matklad commented Jul 9, 2020

Since I started the merging effort, I'll gladly bring this over the finish line.

Awesome, thank you!

I think it'd be good to rename the extension to something like vscode-rust to be more in line with the rest of the language extension and since it's not going to be rls-vscode for much longer. What do you think about it?

The extension is already called rust I believe. But +1 on renaming the repo to vscode-rust!

With regards to the implementation, I'd lean towards the "proper" approach

Makes sense! I think technically we might start with using some git magic to move editors/code directory from rust-analyzer into this repository (or we can forget the history and just-copy-paste, I am fine with both). I think it would be useful to have to code-bases as is in the same repo at some point, as migration via moving code from one folder to another would be more effective at not loosing important bits of functionality.

We might want to stir existing rust-analyzer extension in place some, to make the later grafting easier. One specific issue I think we need to solve before hand is distribution. Let me make a separate comment about that ...

@bjorn3
Copy link
Member

bjorn3 commented Jul 9, 2020

I think technically we might start with using some git magic to move editors/code directory from rust-analyzer into this repository

git subtree?

@matklad
Copy link
Member Author

matklad commented Jul 9, 2020

So, at the moment, rust-analyzer's binary is distributed via GitHub release, and we take advantage of the fact that the vscode extension and the server are packaged into a single release. Basically, every week we release a new version of vscode extension, and it than downloads the binary for the same tag. So, we effectively re-use vscode auto-update flow to update the binary itself.

I think, at least for transition period, unified extension should have an ability to auto-update rust-analyzer from GitHub releases. It's pretty crucial that we are able to ship incremental breakages improvements. But this auto-update can be done by polling (after asking for user's consent) github releases. We already have logic for this in place, for the nightly channel.

Long term, we should switch to primarily (only) supporting rustup.

@matklad
Copy link
Member Author

matklad commented Jul 9, 2020

@bjorn3 more like git filter-branch I believe

@lnicola
Copy link
Member

lnicola commented Jul 9, 2020

Long term, we should switch to primarily (only) supporting rustup.

TBH, I quite like the current setup because you can still update rust-analyzer without the full toolchain.

For nightly users, rustup update nightly invalidates every cache and incremental build build directory, so it's not worth updating every day.

@matklad
Copy link
Member Author

matklad commented Jul 9, 2020

For nightly users, rustup update nightly invalidates every cache and incremental build, so it's not worth updating every day.

I would assume that the extension would run rustup run --toolchain nightly rust-analyzer --, so that the user don't have to switch manually. Though, we need to make sure that that would use the correct (stable) cargo check and cargo metadata.

@lnicola
Copy link
Member

lnicola commented Jul 9, 2020

Read that as "for those who use nightly as their default toolchain".

@Xanewok
Copy link
Member

Xanewok commented Jul 12, 2020

FWIW the repository is now renamed: https://github.com/rust-lang/vscode-rust.

@matklad
Copy link
Member Author

matklad commented Jul 12, 2020

Nice! I think we are ready to „freeze“ vscode extension in rust-analyzer‘s repo after tomorrow’s release. I‘ll try to transfer the git history to this repo tomorrow

@matklad
Copy link
Member Author

matklad commented Jul 13, 2020

PR is up: #816

TIL: git-filter-repo is the new git filter-branch

@mattburgess
Copy link

Long term, we should switch to primarily (only) supporting rustup.

That would be rather unfortunate. My Linux distro (Fedora) does a really good job at keeping the Rust toolchain up to date via its package manager, and I much prefer my distro's package manager to handle software installs than anything else.

I've a strong preference for the current settings for selectively disabling rustup to be maintained, and having the extension periodically poll GitHub for more recent rust-analyzer releases.

@matklad
Copy link
Member Author

matklad commented Jul 23, 2020

I have opinions on this, but I also think it’s way to early to discuss any specifics here.

@lnicola
Copy link
Member

lnicola commented Oct 15, 2020

I wonder if it wouldn't be better to rename the current extension and keep it for "stable" users (or, alternately, publish rust-analyzer under a new name) than to merge the extensions or try to support both servers with a single one.

@MarkDDR
Copy link

MarkDDR commented Oct 3, 2021

So what is the current status with this merge? It currently seems like this extension is abandoned. If the merge has been given up, we should update the extension to make it clear that this is deprecated and new users should install the rust-analyzer extension instead of this one, also see #927.

@matklad
Copy link
Member Author

matklad commented Oct 3, 2021

See rust-lang/rust-analyzer#4224 (comment) for the current status.

@lnicola
Copy link
Member

lnicola commented May 19, 2022

Update: rust-analyzer has moved to rust-lang.rust-analyzer. We will probably not merge the two extensions, but instead deprecate the old one (RLS / rust-lang.rust), so that existing users (who might prefer RLS) will not see any changes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants