dundalek / lazy-lsp.nvim

Neovim plugin to auto install LSP servers
MIT License
196 stars 13 forks source link

Interaction between `preferred_servers` and `excluded_servers` has changed a bit recently #39

Closed metiulekm closed 5 months ago

metiulekm commented 5 months ago

So I had something like this in my config:

-- rust_analyzer setup via nvim-lspconfig

require("lazy-lsp").setup {
    excluded_servers = {
        "rust_analyzer",
    }
    preferred_servers = {
        rust = { "rust_analyzer" },
    }
}

The intention behind this weird piece of config was that I wanted to setup rust_analyzer manually, essentially due to #22. But back when I was writing this config, there was also the legacy Rust language server, RLS, which I wanted to exclude. And at the time, I wanted to use preferred_servers to set my preferred servers instead of excluding those that I did not want (not unlike https://github.com/dundalek/lazy-lsp.nvim/issues/23#issuecomment-2015293827).

In the past, this worked exactly as I wanted: lazy-lsp would not setup rust_analyzer because I have excluded it, and would also setup nothing else for this filetype because of the preferred_servers value. So in the end, lazy-lsp would not do anything for the rust filetype.

However, some time ago, lazy-lsp started instead setting up a second copy of rust-analyzer. I did not investigate this very deeply, but I suspect this is due to https://github.com/dundalek/lazy-lsp.nvim/commit/26c681ffa5bd68ed254cb409aab858d891fae8e1. If I understand correctly, on the left side we iterate on included_servers, which takes excluded_servers into account. However, on the right side, we iterate on server_to_filetypes, which also takes preferred_servers into account.

In the end, I just switched to using prefer_local (thanks a lot! :heart:). Also I changed my mind on excluded_servers/preferred_servers, and you have removed RLS configuration anyway. And even then I will be first to admit that the snippet I pasted looks kind of absurd, so I won't be surprised if you just close the issue immediately :stuck_out_tongue: But I do feel the previous behavior is slightly more correct, and I just wanted to create this issue to not throw away information.

dundalek commented 5 months ago

Thanks, I appreciate the write-up.

Getting these options to cover needed situations without getting too overwhelming is a journey :)

You are correct, I was able to verify with a new test case that there was indeed a regression and matches the commit you found. I also find the previous behavior more intuitive.

Glad to hear the new option works for you even better with a cleaner config. I think I will push a fix anyway to get a peace of mind :)