williamboman / nvim-lsp-installer

Further development has moved to https://github.com/williamboman/mason.nvim!
https://github.com/williamboman/mason.nvim
Apache License 2.0
2k stars 123 forks source link

Config not enhanced for automatically installed servers on first install #869

Closed Akeboshiwind closed 2 years ago

Akeboshiwind commented 2 years ago

Problem description

I was reviewing the automatic_installation code to see if it was right for what me and I found this point in the code here:

https://github.com/williamboman/nvim-lsp-installer/blob/793f99660fa9212f52ee8b6164454e03ba1f42c9/lua/nvim-lsp-installer/middleware.lua#L59-L65

It looks to me like if a server is installed with automatic_installation then the default_options won't be applied to it after install.

I'm assuming this is a bug, but it might be intentional because of some weird async issues. If so then I think there should be a notification to restart after automatic installation.


Side note: I was trying to see if automatic_installation could be used to lazily install servers when packer lazily runs my config. From reading the code it looks like this should work very nicely! (Other than the bug above) It could maybe be advertised in the README a little better, something in features like Supports lazy installation of servers via automatic_installation

Neovim version (>= 0.7)

N/A

Operating system/version

N/A

I've recently downloaded the latest plugin version of both nvim-lsp-installer and nvim-lspconfig

Affected language servers

all

Actual behavior

See Above

Expected behavior

See Above

LspInstallInfo output

N/A

Installation log

N/A

Healthcheck

N/A

Screenshots

No response

Akeboshiwind commented 2 years ago

@williamboman With the new mason.nvim package (nice work btw), I've looked into this same issue over there.

The code is a bit harder for me to read but I think this issue is still present: https://github.com/williamboman/mason-lspconfig.nvim/blob/3cbd87f0824a88f61b3b8b986fa77428bbad4427/lua/mason-lspconfig/lspconfig_hook.lua#L62-L64

As you can see in the above code, the merge_in_place is done only if the server is already installed. Not if it's auto-installed.

With that being the case do you want me to open a new issue on mason-lspconfig.nvim or shall I keep this one open?

williamboman commented 2 years ago

Hello! Hm, it should work fine with mason-lspconfig. Servers are re-setup once installation successfully completes.

Akeboshiwind commented 2 years ago

I've pulled together a minimal.lua to help show what I'm seeing (see below).

As you can see I lazily run the lspconfig setup function when I open a python file. This means I don't have to have plugins downloaded, or lua namespaces loaded that I'm not currently using. (How useful that is is another matter :P)

When trying a more basic setup if I just open a normal python file on startup I get this same issue oddly 🤔.

What I expect to happen

When I open a python file for the first time the pylsp server is downloaded and then my custom config is loaded and run.

What actually happens

I open a python file and get the error:

Spawning language server with cmd: `pylsp` failed. The language server is either not installed, missing from PATH, or not executable.

My custom print of "Ran custom on_attach" isn't in :messages I check installed servers using the :Mason command and see that python-lsp-server is now installed.

When I close nvim and reopen a python file, my custom print is run and the lsp server starts linting correctly.

Steps to reproduce

I think you can also reproduce this by:

Minimal Config

-- credit to telescope.nvim for this format
vim.cmd [[set runtimepath=$VIMRUNTIME]]
vim.cmd [[set packpath=/tmp/nvim/site]]
local package_root = '/tmp/nvim/site/pack'
local install_path = package_root .. '/packer/start/packer.nvim'
local function load_plugins()
  require('packer').startup {
    {
      'wbthomason/packer.nvim',
      {
        "williamboman/mason-lspconfig.nvim",
        requires = {
          "williamboman/mason.nvim",
          "neovim/nvim-lspconfig",
        },
      },
      -- ADD PLUGINS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
      {
        "./non_existing_dummy_plugin",
        ft = 'python',
        config = function()
          require('lspconfig').pylsp.setup({
            on_attach = function(_, _)
              print("Ran custom on_attach")
            end
          })
        end,
      },
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. '/plugin/packer_compiled.lua',
      display = { non_interactive = true },
    },
  }
end
_G.load_config = function()
  require('mason').setup({
    install_root_dir = '/tmp/nvim/data/mason'
  })
  require('mason-lspconfig').setup({
    automatic_installation = true
  })
  -- ADD INIT.LUA SETTINGS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
  -- require('lspconfig').pylsp.setup({
  --   on_attach = function(_, _)
  --     print("Ran custom on_attach")
  --   end
  -- })
end
if vim.fn.isdirectory(install_path) == 0 then
  print("Installing Telescope and dependencies.")
  vim.fn.system { 'git', 'clone', '--depth=1', 'https://github.com/wbthomason/packer.nvim', install_path }
end
load_plugins()
require('packer').sync()
vim.cmd [[autocmd User PackerComplete ++once echo "Ready!" | lua load_config()]]
williamboman commented 2 years ago

Ah. That's working as intended. When you call lspconfig's .setup() function, it'll immediately set up the triggers for starting the server and whatnot. There's no way to defer this. If you wait until the server is installed and then trigger a FileType autocmd1 (lspconfig uses this to start/attach servers), the server should start just fine. It's definitely awkward, but there's not much that can be done without adding a layer of indirection.

I'd also be wary of using ft as a trigger to set up a language server. lspconfig uses the exact same event and so you need to make sure that lspconfig's autocmd handler runs before yours. I'd just set each server up normally directly within my init script, there's no overhead in doing so. I personally use :h mason-lspconfig.setup_handlers() which helps avoid this particular issue (but, it renders the automatic_installation feature useless as you need to manage installations somewhere else).

1 - this is easily done with simply doing :e

Akeboshiwind commented 2 years ago

Riiight, I don't know why I assumed lspconfig wouldn't use ft as a trigger 🤦.

I'll have to think of another way to achieve this as I like lazy loading plugins for languages I'm not using. I hope I can think of a solution better than running :LspStart after I run setup 😅.