Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add an entry about the setup-hook #1873

Merged
merged 1 commit into from
Aug 23, 2022
Merged

Conversation

kylo252
Copy link
Contributor

@kylo252 kylo252 commented Apr 28, 2022

The on_setup hook is currently undocumented, and it should gain some attention after williamboman/nvim-lsp-installer#631

@mjlbach
Copy link
Contributor

mjlbach commented Apr 28, 2022

Maybe emphasize "this is principally for plugin developers, and overriding this without wrapping the original function may break external integrations with lspconfig"

@kylo252
Copy link
Contributor Author

kylo252 commented Apr 29, 2022

would it make sense to have this setting per-server instead of being global?

diff --git a/lua/lspconfig/util.lua b/lua/lspconfig/util.lua
index 3febafc..3e57fde 100644
--- a/lua/lspconfig/util.lua
+++ b/lua/lspconfig/util.lua
@@ -14,11 +14,9 @@ M.default_config = {
   init_options = vim.empty_dict(),
   handlers = {},
   autostart = true,
+  on_setup = nil
 }
 
--- global on_setup hook
-M.on_setup = nil

it could then be renamed to pre_init or something, if on_setup is ambiguous here I forgot that before_init exists, can we not use that instead?

@williamboman
Copy link
Contributor

williamboman commented Apr 29, 2022

FWIW, I found use for this in my personal configs for making sure to apply a set of default configurations (primarily the ever so present on_attach and capabilities settings).

  1. Register a hook for applying defaults
  2. Call setup while only having to pass server specific settings:
    - https://github.com/williamboman/nvim-config/blob/9668609f2a52f3d4630c0cbb553e4be379035ed3/after/plugin/lsp/servers/pylsp.lua#L6)
    - https://github.com/williamboman/nvim-config/blob/9668609f2a52f3d4630c0cbb553e4be379035ed3/after/plugin/lsp/servers/jsonls.lua#L12-L6

lspconfig.util.on_setup = util.add_hook_before(lspconfig.util.on_setup, function(config)
if some_condition then
local custom_server_prefix = "/my/custom/path"
config.cmd_env = custom_server_prefix
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you perhaps mean to set the cmd? cmd_env is a table

Suggested change
config.cmd_env = custom_server_prefix
config.cmd = { custom_server_prefix }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hopefully this new example makes sense!

FWIW, I found use for this in my personal configs for making sure to apply a set of default configurations (primarily the ever so present on_attach and capabilities settings).

I'm not sure if this is something that @mjlbach wants to advertise (?)

since he suggested the note about this targeting plugins devs, and one can get a somewhat similar result by overriding the global defaults :h lspconfig-global-defaults

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah TIL about global defaults 😅. I guess one difference is that with the hook approach you can make sure to merge settings, for example chaining function on_attach and on_new_config calls.

@kylo252 kylo252 marked this pull request as ready for review April 29, 2022 15:05
@glepnir glepnir merged commit 4a09c34 into neovim:master Aug 23, 2022
@kylo252 kylo252 deleted the hook-docs branch September 4, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants