feat: integrate with lspconfig's on_setup hook

[williamboman]

First step towards not requiring any special interactions in user's init scripts.

[williamboman]

@mjlbach 👋 I'm struggling a bit with the fact that calls to lspconfig's .setup() functions usually takes place immediately in user's init scripts. This makes it difficult for nvim-lsp-installer to initialize what it needs at the right moment, without having users manually orchestrating this by either;

1. making sure to call require('nvim-lsp-installer').setup() before any lspconfig interactions, or
2. have users leverage the different lifecycle mechanisms that plugin managers like packer provide (use { "neovim/nvim-lspconfig", after = "lsp_installer" })

I was wondering if it'd make sense to somehow defer the setup of each server until the after/ scripts are executed, leaving room for plugins to run any necessary initialization scripts inside plugin/. Or do you have any other ideas?

I'm thinking something like this (but better):

diff --git a/after/plugin/lspconfig.vim b/after/plugin/lspconfig.vim
new file mode 100644
index 0000000..cf833b7
--- /dev/null
+++ b/after/plugin/lspconfig.vim
@@ -0,0 +1 @@
+lua require('lspconfig.deferred').flush()
diff --git a/lua/lspconfig/configs.lua b/lua/lspconfig/configs.lua
index a47ecba..8e6b4b4 100644
--- a/lua/lspconfig/configs.lua
+++ b/lua/lspconfig/configs.lua
@@ -1,4 +1,5 @@
 local util = require 'lspconfig.util'
+local deferred = require 'lspconfig.deferred'
 local api, validate, lsp = vim.api, vim.validate, vim.lsp
 local tbl_extend = vim.tbl_extend
 
@@ -50,6 +51,11 @@ function configs.__newindex(t, config_name, config_def)
 
     config = tbl_extend('keep', config, default_config)
 
+    if deferred.should_defer then
+      deferred.defer(config)
+      return
+    end
+
     if util.on_setup then
       pcall(util.on_setup, config)
     end
diff --git a/lua/lspconfig/deferred.lua b/lua/lspconfig/deferred.lua
new file mode 100644
index 0000000..0720522
--- /dev/null
+++ b/lua/lspconfig/deferred.lua
@@ -0,0 +1,21 @@
+local M = {}
+
+M.should_defer = true
+
+local deferred_configs = {}
+
+function M.defer(config)
+  assert(M.should_defer, "defer() was called when it wasn't expected.")
+  table.insert(deferred_configs, config)
+end
+
+function M.flush()
+  local configs = require 'lspconfig.configs'
+  M.should_defer = false
+  for _, config in pairs(deferred_configs) do
+    configs[config.name].setup(config)
+  end
+  deferred_configs = {}
+end
+
+return M

[mjlbach]

Thank you, this is wonderful and addresses my main concern with nvim-lsp-installer

require('nvim-lsp-installer').setup()

I personally feel like this is reasonable? Unfortunately (and it could be said this is a limitation of the vim package system) we don't have a way to enforce a conditional ordering with neovim apart from package managers.

I'm somewhat ok with the idea of making setup {} lazier, but I need to think about it a bit more

[williamboman]

I added a mandatory .setup {} function, similar to most other plugins.

Also ping @kylo252 @VonHeikemen @junnplus - with this change I'm intending to eject nvim-lsp-installer from being explicitly involved in setting up LSP servers. This will first and foremost target new users of the plugin, but I intend to move over existing users as well (trying to time this with neovim releases to make it feel more natural). The APIs that exist today (on_server_ready(), server:setup(), server:on_ready(), etc.) will remain unchanged, so no worries, but some of them will be deprecated and I hope to fully remove them in the future. I'd be happy to fill in any potential gaps to enable certain use cases, but in that case take the opportunity to redesign them to be less intrusive.

With the emergence of "wrapper/glue" plugins like @VonHeikemen's, @junnplus's et al., I'll also start looking into potentially cutting of other parts of nvim-lsp-installer that aren't integral to the installation and management of LSP servers.

An example setup from now on would look something like this:

require("nvim-lsp-installer").setup {} -- <- I hope to to be able to even exclude this in the future, but for now it'll be required
local lspconfig = require("lspconfig")

local function on_attach()
  -- setup buffer keymaps etc.
end

lspconfig.sumneko_lua.setup {
  on_attach = on_attach,
  settings = {
    Lua = {
      format = {
        enable = false
      },
      diagnostics = {
        globals = { "P" },
      },
    }
  }
}

local tsutils = require "nvim-lsp-ts-utils"
lspconfig.tsserver.setup {
  init_options = {
    hostInfo = "neovim",
    preferences = {
      includeInlayParameterNameHints = "none",
      includeInlayParameterNameHintsWhenArgumentMatchesName = false,
      includeInlayFunctionParameterTypeHints = false,
      includeInlayVariableTypeHints = true,
      includeInlayPropertyDeclarationTypeHints = true,
      includeInlayFunctionLikeReturnTypeHints = true,
      includeInlayEnumMemberValueHints = true,
    },
  },
  on_attach = function(client, bufnr)
    on_attach(client, bufnr)
    tsutils.setup {}
    tsutils.setup_client(client)
  end,
}

require("rust-tools").setup {}

for _, server in ipairs { "clangd", "eslint", "ltex" } do
  lspconfig[server].setup { on_attach = on_attach }
end

edit: This comment might also be of interest w.r.t. future plans - pocco81/dap-buddy.nvim#71 (comment)

[mjlbach]

I fundamentally think this is a big step forward for usability, especially if nvim-lsp-installer (as you link to in that comment) moves to a generic lua based system package manager which would allow for the introduction of a variety of companion hooks for other ecosystems (lsp, dap, formatters, linters, etc.) to allow similar degrees of seemlessly subsituting external binaries with the installer-vendored ones.

Once this is merged I am happy to coordinate to keep the integration functional even while we do major changes to lspconfig.

I'm actually hoping that with this we can move away from the wrapper/glue plugins, I think the main two usability barriers are the lack of default keybindings and the currently verbose requirements for nvim-cmp. Some have suggested lspconfig fundamentally change the method for setting up servers (which is kinda planned), but that can be discussed somewhere else.

The main two features/concerns I know this brings are the elegance of imperatively installing language servers/having a default hook for servers (which I believe can be achieved currently by overriding the global default config for lspconfig, this just needs to be more documented) and 2 removing the last remaining boiler plate, so adding lspinstaller "just works". for substituting language servers.

[kylo252]

I've always preferred this approach, the only blocker at the time was the race condition between the installer still running and lspconfig attempting to launch the server.

semi-relevant: #216

It's even trivial currently to get the the cmd_env from nvim-lsp-installer and just pass it directly to lspconfig, e.g. https://github.com/LunarVim/LunarVim/blob/f49f687bc244ff603f3dde6828a85538c50df220/tests/minimal_lsp.lua#L91-L84

so it's a non-issue either way, in case you wanted to take your time with the API.

[VonHeikemen]

The current implementation of lsp-zero should not be affected by this change. I merge the servers default options with the users options and feed it to lspconfig. If I understand this PR correctly, I don't even need to call the setup function.

Thanks for including me in the conversation.

[williamboman]

It's even trivial currently to get the the cmd_env from nvim-lsp-installer and just pass it directly to lspconfig, e.g. https://github.com/LunarVim/LunarVim/blob/f49f687bc244ff603f3dde6828a85538c50df220/tests/minimal_lsp.lua#L91-L84

Worth noting is that while just extracting cmd_env will work for a majority of servers, there are instances where this won't suffice (for example, powershell_es and angularls). For these reasons, I think as a principal rule entire tables should always be merged (which happens automatically now if you call require("nvim-lsp-installer").setup()).

I've always preferred this approach, the only blocker at the time was the race condition between the installer still running and lspconfig attempting to launch the server.

This will actually now be the case when using the newly suggested config (going directly via lspconfig). lspconfig will fail to launch configured servers until they're actually installed (this was avoided with the .on_server_ready() approach and one of the original motivations behind it). But, all mechanisms around server setup are now being externalized from this plugin, to be solved/improved elsewhere!

---

To those I pinged in my previous comment, after this PR is merged I'd encourage you all to find ways to reduce your dependency on nvim-lsp-installer to one single line: require("nvim-lsp-installer").setup {}. If this breaks some functionality or if you struggle with something please ping me somewhere here on GitHub or on IRC (williamboman @libera.chat) !

[williamboman]

Note that if the require("nvim-lsp-installer").setup {} function has been called, setting up servers the "old way" via nvim-lsp-installer (server:setup {}) will now throw an error. This should be fine as-is, but some users might end up adding that line to their configs while using one of the available auxiliary plugins to set up servers.

Also, I forgot to ping @ray-x with navigator.lua in my comment earlier.

Will merge this now 🤞

[axieax]

Hi, just wondering if there is a migration guide for this breaking change for those using on_server_ready.

[kylo252]

Worth noting is that while just extracting cmd_env will work for a majority of servers, there are instances where this won't suffice (for example, powershell_es and angularls). For these reasons, I think as a principal rule entire tables should always be merged (which happens automatically now if you call require("nvim-lsp-installer").setup()).

Thanks for the heads-up! you can see I'm already using server:get_default_options() so it's just a matter of merging the options like you mentioned.

---

Unless I'm missing something, I think you might've forgotten to mention how lspconfig would find the server, like read from server:get_default_options(), or maybe add the installation root-dir to path.

nvim-lsp-installer/doc/nvim-lsp-installer.txt

Lines 51 to 68 in 110e30b

	The only thing needed to get started with nvim-lsp-installer is to make sure
	to call the `setup()` function _before_ you set up any servers: >
	require("nvim-lsp-installer").setup {}
	<
	Next, in your initialization files |init.lua|, setup the servers you want to use.
	Refer to |lspconfig| for more information! For example: >
	local lspconfig = require("lspconfig")
	local function on_attach(client, bufnr)
	-- set up buffer keymaps, etc.
	end
	lspconfig.sumneko_lua.setup { on_attach = on_attach }
	lspconfig.tsserver.setup { on_attach = on_attach }
	<

[williamboman]

Hi, just wondering if there is a migration guide for this breaking change for those using on_server_ready.

Ah I see some people have interpreted this as a breaking change - there are no breaking changes introduced here! Everything will continue to function as before, there's just a new preferred way of setting things up. I see that one of the commit messages was marked as breaking change and made it through in the squashed commit message, so packer is picking that up as a breaking change when updating the plugin version. I'm drafting an announcement with a guide on how to migrate atm, will publish under Discussions when it's ready!

Unless I'm missing something, I think you might've forgotten to mention how lspconfig would find the server, like read from server:get_default_options(), or maybe add the installation root-dir to path.

I'm not sure that needs to be documented, does it? Just call .setup(), then you can keep nvim-lsp-installer out of sight & out of mind. Tbh I'm hoping to either completely get rid of the Server class, or alternatively just expose :install(), :is_installed(), and :uninstall() as public methods.

[kylo252]

I'm not sure that needs to be documented, does it? Just call .setup(), then you can keep nvim-lsp-installer out of sight & out of mind. Tbh I'm hoping to either completely get rid of the Server class, or alternatively just expose :install(), :is_installed(), and :uninstall() as public methods.

I did actually originally miss what that the middleware is doing, I see now how it's updating the defaults of lspconfig.

I'd still say it's worth mentioning that this is what's happening behind the scenes. Especially since on_setup is currently undocumented, so we could add the docs for it in lspconfig, and just link to it from here.

[kylo252]

Tbh I'm hoping to either completely get rid of the Server class, or alternatively just expose :install(), :is_installed(), and :uninstall() as public methods.

on a semi-related note: what's your plans for the integration with Server:get_settings_schema()? it looks a bit odd now that the installer isn't directly involved with the setup, but I also know it was quite a bit of effort to get that part up and running smoothly 😢

[williamboman]

I'd still say it's worth mentioning that this is what's happening behind the scenes. Especially since on_setup is currently undocumented, so we could add the docs for it in lspconfig, and just link to it from here.

I'm not sure the average user would need to know this though? I feel like it's more geared towards plugin authors? It is actually mentioned briefly in the introduction in the README!

on a semi-related note: what's your plans for the integration with Server:get_settings_schema()? it looks a bit odd now that the installer isn't directly involved with the setup, but I also know it was quite a bit of effort to get that part up and running smoothly 😢

I don't consider that method part of the public API, only things documented in :h nvim-lsp-installer are what I consider public, things not included there are subject to breaking changes without notice. I will probably at the very least restructure things internally to make sure not to colocate internals with public interfaces. Is this method being used elsewhere currently?

[kylo252]

I don't consider that method part of the public API, only things documented [in :h nvim-lsp-installer] I will probably at the very least restructure things internally to make sure not to colocate internals with public interfaces.

oh I'm talking about the info panel integration, rather than the API itself. I personally like it, but I worry it might cause some confusion since the installer is now decoupled from the these settings, old example: #451

[mehalter]

This is an awesome PR and I'm really enjoying the decoupling of the lsp installer and the lsp configuration. I do have a quick clarification question. is the on_server_ready function going to be deprecated in the future? Or is that API going to stay in place? It makes it nice to be able to have a function that sets up the servers easily without me needing to add a configuration line simply because I have installed it and it is ready to be used.

[williamboman]

This is an awesome PR and I'm really enjoying the decoupling of the lsp installer and the lsp configuration. I do have a quick clarification question. is the on_server_ready function going to be deprecated in the future? Or is that API going to stay in place? It makes it nice to be able to have a function that sets up the servers easily without me needing to add a configuration line simply because I have installed it and it is ready to be used.

It is already deprecated, so continued use is not recommended. As for removal, I don't have any concrete plans. There will probably be tons of configs that still make use of it so I want to give proper time for people to move away from it. Realistically it will probably remain for a long time (although completely removed from any docs).

As for the functionality you're describing, it was one of the main motivations behind it! I'll have to let this new approach brew for a while, but I'd imagine a simpler hook API like the following would emerge soon:

require("nvim-lsp-installer").setup {
    hooks = {
        server_installed = function (server_name)
            lspconfig[server_name].setup { }
        end,
        server_updated = function (server_name)
           -- do things like stop and restart relevant client(s)
        end,
        server_uninstalled = function (server_name)
            -- do things like stopping relevant client(s), idk, up to you
        end
    }
}

[mehalter]

Ah awesome @williamboman. I also was able to move to using the get_installed_servers() function in my configuration to remedy this deprecation. I am one of the developers for the AstroNvim configuration and we wanted to move our LSP code for our users to use the new APIs. Instead of having a hook when we build the LSP set up we just check which servers are actively installed. Should be a good use case for now. This also makes our configuration much easier for users to manage both language servers installed with nvim-lsp-installer and ones that they already have installed on their machine. Thanks so much for the great work!

[neevek]

Hi, just wondering if there is a migration guide for this breaking change for those using on_server_ready.

Ah I see some people have interpreted this as a breaking change - there are no breaking changes introduced here! Everything will continue to function as before, there's just a new preferred way of setting things up. I see that one of the commit messages was marked as breaking change and made it through in the squashed commit message, so packer is picking that up as a breaking change when updating the plugin version. I'm drafting an announcement with a guide on how to migrate atm, will publish under Discussions when it's ready!

Unless I'm missing something, I think you might've forgotten to mention how lspconfig would find the server, like read from server:get_default_options(), or maybe add the installation root-dir to path.

I'm not sure that needs to be documented, does it? Just call .setup(), then you can keep nvim-lsp-installer out of sight & out of mind. Tbh I'm hoping to either completely get rid of the Server class, or alternatively just expose :install(), :is_installed(), and :uninstall() as public methods.

Hi, I am still new to this stuff, but what's the alternative to server:get_default_options()? With on_server_ready, I use server:get_default_options() to get the default options and merge them with my custom options, with the new API I don't know how to do that.

[williamboman]

Hi, I am still new to this stuff, but what's the alternative to server:get_default_options()? With on_server_ready, I use server:get_default_options() to get the default options and merge them with my custom options, with the new API I don't know how to do that.

Have you seen #636? Hopefully it answers your questions, if not send a comment over there instead!