A few improvements to ensure_installed

wookayin commented 2 years ago

Is your feature request related to a problem? Please describe.

No

Describe the solution you'd like

I find some issues with ensure_installed options.

Error messages. First, it doesn't raise any errors when unknown lsp servers are specified. For example,

lsp_installer.setup {
  ensure_installed = {'foobar'}
}

Nothing happens. No warning, No message -- I would expect some error message to occur.

Callback or notification. I would like to see some messages when automatic install is complete, rather than Installing XXX.. which triggers only when automatic installation starts. Or some warning/error message when some error happens. I do not see recieve any callback, so it's hard to troubleshoot if something goes wrong.

By the way, is there any autocmd event or on_server_ready hook that I can use (after the changes in how we should configure: #636)?

Headless v.s. with UI. Currently lsp:install() will be called upon executing of ensure_installed, but this will run in the background silently. I'd prefer seeing some explicit user interface, say the :LspInstallInfo dialogue.

The following is how I was doing this rather manually rather than ensure_installed. It'd be great if there is an alternative way or option to show the dialogue during automatic installation.

-- Automatically install if a required LSP server is missing.
for _, lsp_name in ipairs(builtin_lsp_servers) do
  local ok, lsp = require('nvim-lsp-installer.servers').get_server(lsp_name)
  ---@diagnostic disable-next-line: undefined-field
  if ok and not lsp:is_installed() then
    vim.defer_fn(function()
      -- lsp:install()   -- headless
      lsp_installer.install(lsp_name)   -- with UI (so that users can be notified)
    end, 0)
  end
end

Describe potential alternatives you've considered

Already described in each section.

Additional context

Relevant issues:

Thanks for the great plugin and amazing support!

williamboman commented 2 years ago

Hello, thanks for your feedback!

Nothing happens. No warning, No message -- I would expect some error message to occur.

Ah, yeah this should probably show a warning.

I would like to see some messages when automatic install is complete, rather than Installing XXX.. which triggers only when automatic installation starts. Or some warning/error message when some error happens. I do see recieve any callback, so it's hard

I was thinking of opening the :LspInstallInfo window if it ends up installing a server, instead of printing a message. But I felt like that might be too intrusive?

By the way, is there any autocmd event or on_server_ready hook that I can use (after the changes in how we should configure: https://github.com/williamboman/nvim-lsp-installer/discussions/636)?

For now, just set up your servers as you would if nvim-lsp-installer didn't exist. This would normally just be doing it directly in your initialization script, like

lspconfig.tsserver.setup {}

The following is how I was doing this rather manually rather than ensure_installed. It'd be great if there is an alternative way or option to show the dialogue during automatic installation.

I personally really dislike having options for every single thing, I think we should aim for the best default behavior instead. I'd be fine with opening the UI window, let's just do it and see if people complain haha :)

bryant-the-coder commented 2 years ago

Heyo @williamboman . I have this issue. I don't know if it is the setup's problem or the code base. Do you mind taking a look?

wookayin commented 2 years ago

@bryant-the-coder Your issue is irrelevant to this thread. Please read https://github.com/williamboman/nvim-lsp-installer/discussions/636

wookayin commented 2 years ago

@williamboman I guess a majority of people would prefer non-intrusive way as opposed to me wanting something more explicit. I can agree that we should avoid too much options/configuration points -- in this case I'm fine with running installation rather manually.

Treesitter does this job reasonably okay --- it prints some messages and users can be notified what's going on and what's done. (Like [1/10] Compiling ... [1/10] Treesitter parser for xxx has been installed) Or having another vim.notify call because we have already one when triggering the installation. Maybe these would be enough for the best default behavior.

williamboman commented 2 years ago

I was also thinking of mimicing the nvim-treesitter behavior for this, but the code is not very well suited for capturing the stdout/stderr currently - the UI window "owns" all of this currently, if we were to override the stdout/stderr outlets the UI would come out of sync, will work on improving this.

williamboman / nvim-lsp-installer