-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
go-to-definition spawns new LSP server with root at definition #2285
Comments
Hello @jonhoo This doesn't seem to need nightly to trigger, you can use two crates that depend on each other and just jumping between them will spawn a new lsp server which is incorrect (vscode is the reference indeed) The problem boils down to https://github.com/neovim/nvim-lspconfig/blob/master/lua/lspconfig/configs.lua#L227 and https://github.com/neovim/nvim-lspconfig/blob/master/lua/lspconfig/util.lua#L288 when we see a buffer that doesn't have a client attached to it, we spawn a new lsp server, the correct approach is to send I think if this is all correct, the fix is to to detect here https://github.com/neovim/nvim-lspconfig/blob/master/lua/lspconfig/util.lua#L288 if there is already an active lsp of the same language we don't need to spawn a new one |
The traceback for second start is like this (using vim.debug.traceback())
|
I patched the code and indeed seems to fix the origianl issue, so for summery
I'm not sure exactly how to check and where for the filetype though |
This might be related: #842. Personally I think the design of require('lspconfig')[name].setup {
root_dir = function()
return vim.loop.cwd()
end
} This will ensure that no multiple instances of the server will be spawned. |
Just simply looked at the code of emacs lsp-bridge lsp-mode and coc.nvim about the root dir, there are some problems in the design of the root dir of lspconfig. But I haven't thought of a good refactoring solution yet |
I removed the root_dir on the core. Re-made only the server initialization parameters. Use the current cwd. When the server starts successfully, add a unique identifier, which is generated by cwd.
|
Just for reference, here is how coc.nvim deals with workspace folders. It seems that at least coc.nvim also uses detection mechanism like The root cause, I think, should be the lack of support of workspace folders feature at core.
And neovim doesn't have such feature currently. Or more specifically, we do not have such a concept like "workspace" or "project" in editor. I'm not familiar with core and its roadmap, so I cannot dig on it further here. I always launch nvim at the root of the project, thus cwd is already the best approximation to "workspace" for me. I guess this is not the case for everyone, and directly using cwd should not be always safe. |
agree. I checked the emacs lsp-bridge lsp-mode and vim yegappan/lsp . I think we need rethinking about of |
Description
When you use go-to-definition on a symbol that takes you outside of the current project (such as one defined in another crate in the Rust world), nvim-lspconfig will spawn a new LSP server (at least for rust-analyzer) with the current working directory set to the directory of the project that holds the symbol definition, rather than the current nvim working directory. This, in turn, causes issues when the working directory has implications for the LSP server's configuration (or the configurations of the tools it in turn invokes).
It's not entirely clear to me whether this a bug with neovim's lsp support, with nvim-lspconfig, with rust-analyzer, or with LSP more broadly, but I suspect it's a nvim-lspconfig issue as VSCode for example handles this correctly.
I also believe this is related to (but not quite the same as) #804.
I think the right solution here is to either find a way to continue using the same LSP server instance, or to spawn the new LSP server instances in
:pwd
, and then point it at the project directory for the definition. That way it'll still pick up the right configuration.Neovim version
Nvim-lspconfig version
d346335
Operating system and version
Amazon Linux 2 (aarch64; 5.4 kernel)
Affected language servers
rust-analyzer, though likely also others
Steps to reproduce
For example, consider two Rust projects where one depends on the other, and both require nightly Rust, but only the consumer has a Rustup toolchain override file:
This is a pretty common scenario — consider if
needs-nightly
isn't actually a local project, but instead is depended on using a version number, and so its definition is actually somewhere in~/.cargo/src/cache/whatever
. The version that's there is unlikely to have arust-toolchain.toml
file.Actual behavior
In the opened neovim session, all LSP features work just fine. However, if you "go to definition" on
Foo
in theuse
line, notice that the#![feature(allocator_api)]
gets highlighted with the errorDespite
:pwd
having arust-toolchain.toml
that specifiesnightly
. There are many other ways for this to manifest, such as if the current directory has a.cargo/config.toml
that defines other (required) rustflags, new Cargo registries, etc., that can even cause the LSP server to fail to start entirely.(the relevant second LSP server launch is here)
Expected behavior
After go-to-definition, the configuration from
:pwd
should continue to apply, meaningrust-toolchain.toml
should still be respected, so no errors should be shown inneeds-nightly
.Minimal config
LSP log
https://gist.github.com/jonhoo/a4d400ee5d14e153904395e3eb12d497
The text was updated successfully, but these errors were encountered: