Skip to content

fix(configs): remove utils in config with globs as markers #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 26 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.

@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.

lsp/ada_ls.lua Outdated
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

@justinmk
Copy link
Member

justinmk commented May 4, 2025

#3820 removes root_pattern for non-glob cases.

@TheRealLorenz TheRealLorenz changed the title feat: remove lspconfig.utils from newest configs fix(configs): remove utils in config with globs as markers May 5, 2025
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