Skip to content

Neovim 0.11 migration #3659

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

Merged
merged 335 commits into from
Apr 12, 2025
Merged

Neovim 0.11 migration #3659

merged 335 commits into from
Apr 12, 2025

Conversation

TheRealLorenz
Copy link
Contributor

@TheRealLorenz TheRealLorenz commented Mar 25, 2025

Closes #3494

This PR introduces the minimal effort to port every configuration to neovim 0.11.The default_configuration field has most of the times everything that's needed, but there are come exceptions that where changed:

  • commands field became raw calls to vim.api.nvim_buf_create_user_command inside on_attach.
  • root_dir became:
    • root_markers whenever the file list was simple didn't need to mach *
    • if the logic was complicated, or needed to match something like '*.c', it was wrapped to work with vim.lsp.Config's root_dir.
  • on_config_change became before_init. I don't actually know if this is the correct approach, but looking around the documentation of nvim-lspconfig a saw that it was defined as the function that gets called as soon as the config have root_dir, and so I thought before_init might be the closest alternative.
  • docs.description became a luadoc annotation prefaced by @brief.
  • single_file_support = false?

Needs #3666

@justinmk
Copy link
Member

I think we should skip to Phase 2. The old configs have already been quasi-frozen for a few months. Now it's time to formalize that.

So I suggest:

  • copy-paste all the old configs to lsp/*
  • do the bare-minimum changes to make them work with vim.lsp.config

@TheRealLorenz
Copy link
Contributor Author

  • copy-paste all the old configs to lsp/*

  • do the bare-minimum changes to make them work with vim.lsp.config

Ok, that's probably better. What about descriptions? Should I annotate the config table with them?

@justinmk
Copy link
Member

Descriptions should probably be luadoc comments at the top of the config. Then we'll need to update the generator in this repo to read those. WDYT @lewis6991 ?

@lewis6991
Copy link
Member

sgtm

@TheRealLorenz TheRealLorenz changed the title Neovim 0.11 migration (phase 1) Neovim 0.11 migration Mar 25, 2025
@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Mar 25, 2025

Update

Now I'll port the LSPs that I usually use first, just because I can test them more easily.

I added the register_commands function, because the old functions didn't seem very ergonomic for this new use case.

For the LuaDoc: should I annotate a local config = { ... } or can i just annotate while I return like I'm doing now?

@justinmk
Copy link
Member

justinmk commented Mar 26, 2025

Sorry, I just remembered something:

The plan is to extend wrap_old_config() while adding files under lsp/ until all servers are supported.

The point of the wrapper was to not have to create configs under lsp/ until Phase 2. The wrapper, if possible, would make vim.lsp.config.enable('foo') work automatically by wrapping the old configs. Without having to write a bunch of per-config code.

If that's possible, then that's still the least-risk approach to start with.

@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Mar 26, 2025

Sorry, I just remembered something:

The plan is to extend wrap_old_config() while adding files under lsp/ until all servers are supported.

The point of the wrapper was to not have to create configs under lsp/ until Phase 2. The wrapper, if possible, would make vim.lsp.config.enable('foo') work automatically by wrapping the old configs. Without having to write a bunch of per-config code.

If that's possible, then that's still the least-risk approach to start with.

I didn't find a feasible approach to it, so I figured we could just port the configs. In the meanwhile I baked in some modifications that wuold fit the new configuration, but I'm more than happy to break things down in separate PRs.

If you prefer a more conservative approach, I could also remove the new Command interface and def_commands function.

I also thought would be smart to use builtin neovim 0.11 in the migrated configs, because those would be used obviously just in neovim>=0.11, but I could just make a really simple migration with little to no effect to the current configurations. This was also meant to use less functions fromlspconfig.utils, as I saw that was a future goal of the plugin.

@justinmk
Copy link
Member

if a wrapper that works automatically isn't possible, then yes we should skip to Phase 2 and start migrating configs. and the old configs are frozen.

If you prefer a more conservative approach, I could also remove the new Command interface and def_commands function.

why is there a new Command interface? where did that requirement come from?

@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Mar 26, 2025

why is there a new Command interface? where did that requirement come from?

Just because the old Commands had a weird syntax which required more complicated parsing (why is there a opt_alias table?). I get it was probably for compatibility issues, but now we can change things more freely.

That being said, I could revert it no problem.

For context, I'm referring to this (probably) over complicated function:
https://github.com/TheRealLorenz/nvim-lspconfig/blob/ea2e1e4dcf9860ee6d58369bb54b7dd246a6e0a1/lua/lspconfig/util.lua#L66C1-L84C7

This was my suggested solution:
https://github.com/TheRealLorenz/nvim-lspconfig/blob/ea2e1e4dcf9860ee6d58369bb54b7dd246a6e0a1/lua/lspconfig/util.lua#L98-L101

@justinmk
Copy link
Member

The commands thing is not supported until we upstream (if necessary) to vim.lsp.config.

The entire purpose of this exercise is to stop depending on framework code in lspconfig, not to replace it with a new flavor.

If the new configs want to have "commands", then for now they will need to hardcode calls to nvim_buf_create_user_command.

Or else figure out a way to do Phase 1 only, and pause Phase 2 until we have a proper answer for the commands situation. neovim/neovim#28329

@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Mar 26, 2025

The commands thing is not supported until we upstream (if necessary) to vim.lsp.config.

The entire purpose of this exercise is to stop depending on framework code in lspconfig, not to replace it with a new flavor.

If the new configs want to have "commands", then for now they will need to hardcode calls to nvim_buf_create_user_command.

Or else figure out a way to do Phase 1 only, and pause Phase 2 until we have a proper answer for the commands situation. neovim/neovim#28329

Fine with me to hardcode nvim_buf_create_user_command calls.

What about util.root_pattern()? In most cases it can simply be root_markers, but sometimes a little bit more complicated approach is needed. In that case should I also hardcode it? Then there's also async.run_job(), should I hardcode that one too? Or should we drop these more complicated features altogether?

These types of things are required in order to port more complicated configs (e.g. clangd or rust_analyzer) that have custom commands and/or custom setup logic.

@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Mar 27, 2025

Update

  • commands field became raw calls to vim.api.nvim_buf_create_user_command inside on_attach.
  • root_dir became:
    • root_markers whenever the file list was simple didn't need to mach *
    • if the logic was complicated, or needed to match something like '*.c', it was wrapped to work with vim.lsp.Config's root_dir.
  • on_config_change became before_init. I don't actually know if this is the correct approach, but looking around the documentation of nvim-lspconfig a saw that it was defined as the function that gets called as soon as the config have root_dir, and so I thought before_init might be the closest alternative.
  • docs.description became a luadoc annotation prefaced by @brief.
  • single_file_support = false?

Needs #3666

Started to port in alphabetical order. Skipping the ones with single_file_support = false and on_new_config for now.

@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Mar 28, 2025

Probably LSPs which have single_file_support = false could simply be ported as something like

{
  ...
  root_dir = function (bufnr, done_callback)
    local root = -- compute root
    done_callback(root or vim.fs.dirname(vim.api.nvim_buf_get_name(bufnr))) -- either
    -- done_callback(root or vim.fn.getcwd()) -- or
  end,
  ...

@fredrikaverpil
Copy link

@TheRealLorenz hey and thank you for your awesome work here.

Today, in the main branch, we can query require("lspconfig").gopls.config_def.default_config to get ahold of the default config, like:

{
  cmd = { "gopls" },
  filetypes = { "go", "gomod", "gowork", "gotmpl" },
  root_dir = <function 1>,
  single_file_support = true
}

Do you know what the equivalent will be to query these new lsp defaults?

@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Apr 1, 2025

@TheRealLorenz hey and thank you for your awesome work here.

Today, in the main branch, we can query require("lspconfig").gopls.config_def.default_config to get ahold of the default config, like:

{
  cmd = { "gopls" },
  filetypes = { "go", "gomod", "gowork", "gotmpl" },
  root_dir = <function 1>,
  single_file_support = true
}

Do you know what the equivalent will be to query these new lsp defaults?

Well, I don't think there's a way that is as straightforward as the old one, but in the majority of cases it isn't needed.

To enable a config you simply vim.lsp.enable('<server_name>'), to edit the config you just pass a table to vim.lsp.config('<server_name>', {}) which extends the default one. To view the applied config you can use :checkhealth vim.lsp.

The problem with getting a default config like in the old ones is that the contents of the lsp/ folder are not require()-able afaik.

@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Apr 3, 2025

@justinmk Sorry to bother.

The "easier" migration is done, now there are just the LSPs which have single_file_support = false and on_new_config left.

Is it feasibile to move on_new_config to a plain before_init? I could wait for neovim/neovim#32287

What's the suggested way to handle single_file_support = false? workspace_required isn't currently available on neovim 0.11 as you already know.

I had an idea which consisted of:
lua

{
  root_dir = function(bufnr, done_callback)
    local root = -- compute root
    if root then
      done_callback(root)
      return
    end
  end,
}

Which shouldn't start the server given that done_callback is not called when the root is nil.

EDIT: Probably this PR could wait until those features are sorted out and implemented in neovim.

@TheRealLorenz
Copy link
Contributor Author

Hey! Really appreciate all the work being put into this!

I’ve been testing out this branch and I ran into an error when opening a Go file:

Error executing callback:
...al/share/nvim/lazy/nvim-lspconfig/lua/lspconfig/util.lua:180: E5560: Vimscript function must not be called in a fast event context
stack traceback:
        [C]: in function 'substitute'
        ...al/share/nvim/lazy/nvim-lspconfig/lua/lspconfig/util.lua:180: in function 'strip_archive_subpath'
        ...al/share/nvim/lazy/nvim-lspconfig/lua/lspconfig/util.lua:102: in function 'get_root'
        .../.local/share/nvim/lazy/nvim-lspconfig/lsp/gopls.lua:37: in function 'on_exit'
        /usr/share/nvim/runtime/lua/vim/_system.lua:309: in function </usr/share/nvim/runtime/lua/vim/_system.lua:279>

The error appears only on the first Go file opened. Opening a second Go file works normally — LSP attaches with no issue. This behavior only started after the most recent update to the fork — it wasn't happening before.

This is what I have in my config:

vim.lsp.config("*", { capabilities = vim.lsp.protocol.make_client_capabilities() })
vim.lsp.config.gopls = { settings = { gopls = { staticcheck = true, gofumpt = true } } }
vim.lsp.enable({ "clangd", "cssls", "gopls", "html", "lua_ls", "rust_analyzer", "ts_ls" })

Let me know how I can help!

Thank you for the insight! Should be fixed by now.

@justinmk

This comment was marked as resolved.

@justinmk
Copy link
Member

justinmk commented Apr 12, 2025

Since we are nearing the end, please avoid force-pushing, just push "fixup" commits instead. I'll squash merge the final PR. (Separating commits for "bisectability" is good, but all of these files are independent, and live in 1 directory, so there is no need for separate commits.)

command 'echohl None'
end

---@brief
Copy link
Member

Choose a reason for hiding this comment

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

the @brief comments should start at top of file. they describe the file (module), not a particular symbol. that might also make docgen a bit easier.

@justinmk
Copy link
Member

LGTM. Only blocker is the failing lint CI. The rest can be in a followup PR.

Thanks again for doing this. Very helpful.

@justinmk justinmk merged commit 81a570f into neovim:master Apr 12, 2025
11 checks passed
muratoffalex added a commit to muratoffalex/nvim that referenced this pull request Apr 13, 2025
@gstokkink
Copy link

@TheRealLorenz thanks a lot for your hard work!

wincent added a commit to wincent/wincent that referenced this pull request Apr 13, 2025
…nfig

See:

- neovim/nvim-lspconfig#3659
- neovim/nvim-lspconfig#3494

Instead of calling `lspconfig['some language server'].setup({})`, call
Neovim built-in `vim.lsp.config('some language server', {})` and it will
automatically load the config from the `lsp/` directory inside the
nvim-lspconfig submodule.

Global config that should apply to all language servers is now set up
using `vim.lsp.config('*', {})`.

Moved the handler overrides into the
`vim.lsp.config('*', { handlers = {} })` call
instead of overwriting the value of
`vim.lsp.handlers['some handler']`.

* aspects/nvim/files/.config/nvim/pack/bundle/opt/nvim-lspconfig 5f395bfe...b6936cb8 (32):
  > docs: update configs.md skip-checks: true
  > fix(docs): docgen.lua reads from `lua/*.lua` #3708
  > feat(typos-lsp): support pyproject.toml and Cargo.toml #3707
  > feat: migrate to vim.lsp.config #3659
  > refactor: replace vim.loop with vim.uv #3703
  > docs: update configs.md skip-checks: true
  > fix(stylelint): update root_file, filetypes #3702
  > docs: update configs.md skip-checks: true
  > feat(tinymist): workspace commands #3639
  > fix(openedge_ls): update configuration #3623
  > docs: update configs.md skip-checks: true
  > feat: bqls #3486
  > docs: update configs.md skip-checks: true
  > feat: rpmspec #3568
  > refactor: deprecate get_active_client_by_name #3697
  > docs: cleanup #3696
  > refactor: deprecate add_hook_before/after #3695
  > refactor: deprecate util.get_lsp_clients #3694
  > refactor: deprecate util functions #3691
  > feat: require Nvim 0.10+ #3692
  > docs: update configs.md skip-checks: true
  > feat: "just" lsp #3655
  > docs: update configs.md skip-checks: true
  > feat(svelte): command to migrate to svelte 5 #3638
  > docs: update configs.md skip-checks: true
  > Revert "fix(eslint): find closest ESLint directory to avoid version mismatch …" #3689
  > docs: update configs.md skip-checks: true
  > fix(eslint): find closest ESLint directory to avoid version mismatch #3686
  > docs: update configs.md skip-checks: true
  > fix(tblgen): find tablegen_compile_commands.yml #3649
  > docs: update configs.md skip-checks: true
  > feat(ada_ls): alire.toml #3667
@neovim neovim locked as resolved and limited conversation to collaborators Apr 13, 2025
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.

Migrate to vim.lsp.config (non-breaking)
8 participants