kabouzeid / nvim-lspinstall

Provides the missing :LspInstall for nvim-lspconfig
MIT License
526 stars 66 forks source link

Avoid overriding upstream (lspconfig) behavior and provide contributions back to upstream #118

Open mjlbach opened 3 years ago

mjlbach commented 3 years ago

I've noticed a trend of some changes being PR'd to nvim-lspinstall but not lspconfig (C# being the most recent one), servers being added to lspinstall before lspconfig (tailwind), or the defaults of lspinstall overriding the defaults of lspconfig (java). This can be confusing to users because if they use lspinstall directly for a server vs using it directly via lspconfig (as I ask people to do when filing a bug report), the behavior can be different.

Some ideas:

Anyways, thanks for the great service! I know a lot of users would not be using the built-in client without this plugin. I'm mostly checking in on this as I'm doing some upstream cleanups of things around root directory detection and checked in on lspinstall as I didn't want to break anything you are using.

kabouzeid commented 3 years ago

I've create a PR template. Will get back to you about the other things. What would make things easier is when it would be possible to get the config table from lspconfig without side effects. In particular without loading the config into lspconfigs servers table.

Right now, I'm using a hack for this. https://github.com/kabouzeid/nvim-lspinstall/blob/6752b31295bf410d8768f7f3bac15e6f1f5b8fbf/lua/lspinstall/util.lua#L3-L22

This is also why I had to copy&paste the whole Java config into lspinstall, since it's one of the few configs that can't be passed to vim.deepcopy(config).

mjlbach commented 3 years ago

Sure, we can probably change that. I'm guessing the default_config key doesn't working for you?

kabouzeid commented 3 years ago

The default_config key is all I need, but it would be better if there was a clean way to get it's value without any side effects, in particular without having it loaded into the require'lspconfig/configs' table. lspinstall doesn't use the same naming scheme as lspconfig, for example the cpp config copies the clangd config. In this example I want to avoid that both cpp and clangd show up in :LspInfo, and also if the user has clangd globally installed, then lspconfig would start two language servers (local and global clangd).

This is what I mean, with there is no way to get the default_config without storing it in the configs table: https://github.com/neovim/nvim-lspconfig/blob/7d6e2211571029ace6ddeefed7df18e4dc51274f/lua/lspconfig/angularls.lua#L17-L19

Ultimately this is not a serious issue, except for the java language server, the fix/hack in my previous comment works just fine.