Skip to content

feat: remove lspconfig.utils from newest configs #3711

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

Draft
wants to merge 38 commits into
base: master
Choose a base branch
from

Conversation

TheRealLorenz
Copy link
Contributor

@TheRealLorenz TheRealLorenz commented Apr 13, 2025

Based on #2079.

The majority of configs which rely on lspconfig.util use util.root_pattern to match against wildcards. I've opened an issue here neovim/neovim#33444 to discuss about this.

In the meantime I'll remove lspconfig.util where it's trivial to do so.

And some minor warning fixes
And fix a deprecation warning
@TheRealLorenz
Copy link
Contributor Author

TheRealLorenz commented Apr 14, 2025

I'm not entirely sure if this migration is ok. It seems like util.root_pattern and vim.fs.root behave differently, the first looks for the first pattern going up to before checking the others, while vim.fs.root I think searches every pattern in every folder before going up.

To make it clearer, I think util.root_pattern({ 'a', 'b' }) behaves like vim.fs.root(0, 'a') or vim.fs.root(0, 'b') instead of vim.fs.root(0, { 'a', 'b' })

This is not just aboute these configs, even for the ones already ported.

@TheRealLorenz
Copy link
Contributor Author

I don't know if these is relevant for most of the configs, but if that is the case we would probably need a flag to change the behaviour of vim.fs.root() to specify the precedence of operations.

One example is sourcekit.lua

https://github.com/TheRealLorenz/nvim-lspconfig/blob/deacc03f6480b69c718f70a787e7ccb928c8a64c/lsp/sourcekit.lua#L11-L21

Here it was explicitly said that the precedence was important, so I called vim.fs.root() multiple times.

root_dir = function(bufnr, on_dir)
local fname = vim.api.nvim_buf_get_name(bufnr)
on_dir(util.root_pattern('Makefile', '.git', 'alire.toml', '*.gpr', '*.adc')(fname))
root_markers = function(name, _)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think root_markers accepts a function, based on :help vim.lsp.Config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually does, that's what I was talking about in neovim/neovim#33444 (comment)

Comment on lines +16 to +18
root_markers = function(name, _)
for _, pattern in ipairs(root_files) do
if vim.glob.to_lpeg(pattern):match(name) ~= nil then
Copy link
Member

Choose a reason for hiding this comment

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

If this is the pattern that's actually unavoidable in every config, it's a strong hint that we need to resolve neovim/neovim#33444 (by enhancing vim.fs.find(), and/or accepting a function for root_markers, and/or supporting wildcards directly in root_dir / root_markers).

@justinmk
Copy link
Member

I'm not entirely sure if this migration is ok. It seems like util.root_pattern and vim.fs.root behave differently

See also #3651 . Deciding on well-defined behavior will require some thought.

@justinmk
Copy link
Member

needs a rebase after #3767 and #3766

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.

2 participants