Skip to content
This repository has been archived by the owner on Nov 12, 2022. It is now read-only.

feat: add automatic_installation option to the setup table #638

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

williamboman
Copy link
Owner

@williamboman williamboman commented Apr 28, 2022

This allows users to automatically install all servers that are setup via lspconfig.

@williamboman williamboman changed the title feat: add 'automatic' option to ensure_installed setting feat: add 'automatic' option to the ensure_installed setting Apr 28, 2022
@williamboman williamboman force-pushed the feat/ensure-installed-on_demand branch 2 times, most recently from c7bc86a to d5d2db0 Compare April 28, 2022 15:55
@williamboman williamboman force-pushed the feat/ensure-installed-on_demand branch from d5d2db0 to b23b6d5 Compare April 28, 2022 21:28
@williamboman williamboman changed the title feat: add 'automatic' option to the ensure_installed setting feat: add automatic_installation option to the setup table Apr 28, 2022
@williamboman williamboman force-pushed the feat/ensure-installed-on_demand branch from b23b6d5 to 20cebcf Compare April 28, 2022 21:30
@@ -25,6 +26,9 @@ function M.register_lspconfig_hook()
local ok, server = servers.get_server(config.name)
if ok then
merge_in_place(config, server._default_options)
if settings.current.automatic_installation and not server:is_installed() then
server:install()
Copy link

@wookayin wookayin Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not blocking, isn't it? So installing would take some time for downloading and unpacking, say 10 seconds. But this hook will be executed when lspconfig[...].setup is called. How do we ensure server is ready and available at this moment, or lspconfig[...].setup is scheduled to be invoked later after the installation is complete?

Copy link
Owner Author

@williamboman williamboman Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it's not blocking!

How do we ensure server is ready and available at this moment, or lspconfig[...].setup is scheduled to be invoked later after the installation is complete?

We don't, anymore. It's one of the drawbacks, but I'd almost argue that's now a usability issue with lspconfig? (the exact same problem exists if you aren't using nvim-lsp-installer)

Copy link

@wookayin wookayin Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, so my understanding was correct. Actually this was to point out some potential problems in the logical flow rather than just questions:

So even if we do server:install(), the installation won't be complete until the actual lspconfig[..].setup { } call that immediately follows this on_setup hook. Therefore the missing LSP still not work as expected this time (despite automatic_installation), with a hope that the missing LSP will work next time. I feel this can be a bit confusing to users. I know there's nothing much we can do here other than starting installation of missing lsp servers, but at least there could be some notifications or documentation about the timing and behavior.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I want to improve this, will let the new things settle for a while first!

@williamboman williamboman merged commit 016cd4b into main Apr 29, 2022
@williamboman williamboman deleted the feat/ensure-installed-on_demand branch April 29, 2022 11:33
@mehalter
Copy link

This is an awesome feature I am looking forward to using it. Is there any way for us to improve this to check the current user's path to see if they already have an language server installed outside of lsp-installer and not automatically install those?

For instance if I wan to use automatic_installation to not have to think about installing different language servers and my machine already has pyright available but not sumneko_lua. I would prefer that it would recognize that pyright is in the users path so it counts as "installed". Not sure if this is even a viable thing to check, but would be a nice feature I think.

@williamboman
Copy link
Owner Author

This is an awesome feature I am looking forward to using it. Is there any way for us to improve this to check the current user's path to see if they already have an language server installed outside of lsp-installer and not automatically install those?

Personally I'm not a fan of such a feature, although I see it could be valuable. I guess what's done in #643 could be further extended in the future to allow skipping servers that are already available on PATH (although, what "being available" actually means is up for debate)

@mehalter
Copy link

@williamboman I looked into this a bit and made an example PR (#645) of what I have in my head. Let me know what you think or if you think it aligns with what you envision. Thanks so much!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants