-
Notifications
You must be signed in to change notification settings - Fork 40.3k
fix(lsp): updated config for mason to v2 #1663
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
base: master
Are you sure you want to change the base?
Conversation
init.lua
Outdated
-- NOTE: `opts = {}` is the same as calling `require('mason').setup({})` | ||
{ 'mason-org/mason.nvim', opts = {} }, | ||
'mason-org/mason-lspconfig.nvim', | ||
{ 'mason-org/mason-lspconfig.nvim', opts = {} }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not only unnecessary, it causes mason-lspconfig to setup LSP's when other plugins might have been interested in doing that prior to it. For example, for nvim-java this causes two jdtls instances where the first one is setup by mason-lspconfig and prevents the second one which is properly steup from functioning properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for letting me know, it was setting up luals twice as well. It should be fixed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for having done this job! It actually helped me with something else.
This seems to do a similar thing as 1590 which already has some traction |
There are a couple of similars pr, but 1590 and 1475 both swap to the new LSP built in API besides fixing mason, which is only availabe in version >0.11. I think is the same reason we still have blink and lazy instead of using vim.lsp and vim.pack. |
Traction does not mean quality.
I don't like them. Both state a different goal than they are achieving. Both are pushing their authors preferences. On the other hand this one does exactly what it says and does it correctly. |
reference to nvim-lua#1663 merge request: nvim-lua#1663
…ctly pass overrides to LSP server initialization
init.lua
Outdated
-- allow for server-specific overrides. | ||
for server_name, server_config in pairs(servers) do | ||
server_config.capabilities = vim.tbl_deep_extend('force', {}, capabilities, server_config.capabilities or {}) | ||
require('lspconfig')[server_name].setup(server_config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nvim 0.11+ should not use require('lspconfig')
. It should use vim.lsp.config
.
nvim-lspconfig will soon be deleting all of its lua/lspconfig/*
configs (which have been deprecated for 6+ months) in favor of lsp/*
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the notice, changed to vim.lsp api, it should be ok now, testing it in my config
The mason-lspconfig.nvim plugin removed the 'handlers' feature in its v2.0.0 release. This change adapts the LSP configuration to follow the new recommended setup and keeps using the nvim-lspconfig api to configure the servers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the LSP configuration to migrate from mason-lspconfig.nvim v1 to v2, which removed the handlers
feature. The change replaces the deprecated handler-based configuration with a manual loop that iterates through servers and configures them using the native nvim-lspconfig API.
- Removes the deprecated
mason-lspconfig
setup with handlers - Adds a manual loop to configure LSP servers using
vim.lsp.config()
andvim.lsp.enable()
- Maintains the same server configuration capabilities while ensuring v2 compatibility
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This fixes an issue where LSP configs were not being applied because the previous setup using `mason-lspconfig.nvim` was using the `handlers` setting, which has been removed in v2.0.0 in favor of the new native `vim.lsp.config()` API. Fix from nvim-lua/kickstart.nvim#1663
add changes introduced by nvim-lua#1663 The mason-lspconfig.nvim plugin removed the 'handlers' feature in its v2.0.0 release. This change adapts the LSP configuration to follow the new recommended setup and keeps using the nvim-lspconfig api to configure the servers.
for server_name, server_config in pairs(servers) do | ||
server_config.capabilities = vim.tbl_deep_extend('force', {}, capabilities, server_config.capabilities or {}) | ||
vim.lsp.config(server_name, server_config) | ||
vim.lsp.enable(server_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I added this line to my config, it brought up every LSP I had installed without regard to if it's needed. Another approach is necessary. Using ftplugin or something else, maybe autocommands, something needs to trigger vim.lsp.enable for the appropriate LSP's only, at the appropriate time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That simply isn't true, at least not with regards to "this line".
vim.lsp.enable()
only sets an autocmd, which activates if you open a buffer with a 'filetype' that matches an enabled LSP config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, maybe I've made a mistake somewhere in my config which is almost identical in this part. Sorry for the comment and thanks for prompting me.
EDIT: actually I have bashls with config = {} , I have luals with filetypes={'lua'} and it enables both of them without me even opening a buffer, so what you say cannot be true because I don't enable them anywhere and when I comment this line, nothing is brought up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of these claims is covered by tests, the other is a vague description that I can only guess at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you even checked LspInfo when you fire up Neovim without opening any file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vim.lsp.enable()
only sets an autocmd, which activates if you open a buffer with a 'filetype' that matches an enabled LSP config.
After some more research I can say the above statement is misinformed:
vim.lsp.enable()
Auto-starts LSP when a buffer is opened, based on the lsp-config filetypes, root_markers, and root_dir fields.
Nowhere here it is said anything about auto commands. You need to set the auto commands yourself and they need to enable the LSP (call vim.lsp.enable) in case a buffer with a configured file type is opened. We don't want LSP's loaded for no reason. They need to be only loaded when they are needed.
Auto commands can be set right there instead of that line. However since nvim-lspconfig should go as well, this should not be done in its config function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--- @param name string|string[] Name(s) of client(s) to enable.
--- @param enable? boolean `true|nil` to enable, `false` to disable (actively stops and detaches
--- clients as needed)
function lsp.enable(name, enable)
validate('name', name, { 'string', 'table' })
local names = vim._ensure_list(name) --[[@as string[] ]]
for _, nm in ipairs(names) do
if nm == '*' then
error('Invalid name')
end
lsp._enabled_configs[nm] = enable ~= false and {} or nil
end
if not next(lsp._enabled_configs) then
-- If there are no remaining LSPs enabled, remove the enable autocmd.
if lsp_enable_autocmd_id then
api.nvim_del_autocmd(lsp_enable_autocmd_id)
lsp_enable_autocmd_id = nil
end
else
-- Only ever create autocmd once to reuse computation of config merging.
lsp_enable_autocmd_id = lsp_enable_autocmd_id
or api.nvim_create_autocmd('FileType', {
group = api.nvim_create_augroup('nvim.lsp.enable', {}),
callback = function(args)
lsp_enable_callback(args.buf)
end,
})
end
-- Ensure any pre-existing buffers start/stop their LSP clients.
if enable ~= false then
if vim.v.vim_did_enter == 1 then
vim.cmd.doautoall('nvim.lsp.enable FileType')
end
else
for _, nm in ipairs(names) do
for _, client in ipairs(lsp.get_clients({ name = nm })) do
client:stop()
end
end
end
end
You can read the source code yourself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You both seem to be unable to comprehend what I'm talking about. One doesn't need the LSP's to be enabled when they are not needed and vim.lsp.enable here does exactly that.
Did any of you run :LspInfo or :healthcheck vim.lsp on a blank buffer in a freshly started Neovim? You should not see anything there and yet, there's a list of enabled configurations.
However many times you may read the code, that's the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing code suffices. The editor auto enables LSPs for the filetypes a user is concerned with. Enabled only means that the autocmd is set up, and the lsp process will spawn only when the filetype is detected, no other overhead. If you do not want or need an LSP, then remove its configuration. It should be fine for the vast majority of use cases, except "maybe" for those that have configs for LSPs counting far into the double digits, at which point it becomes a personal problem.
If you still want to overengineer this though, you can hijack lsp.enable
or just make your own setup function to enable LSPs whenever you want, because it is pointless to wrap lsp.enable
in an autocmd on the FileType
event, I think you cannot have the autocmds that it sets up fire at that same event (I guess not really, but still pointless). But at that point, you're just going to end up rewriting what lsp.enable
already does. Please read it properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please read it properly.
This one I didn't need to read twice. Thank you for the explanation. Actually I had already started doubting my understanding and your explanation just confirmed my doubts. Thank you!
I think you cannot have the autocmds that it sets up fire at that same event.
If I understand that properly, it means the FileType
autocmd and the autocmd created from the wrapped vim.lsp.enable call cannot fire at the same event. Well, they do. I had taken exactly the path to wrap vim.lsp.enable in an autocmd of the FileType
event and it worked that way. By worked, I mean it enabled and started the LSP's on opening a file from that type.
This pull request updates the LSP configuration to align with the changes in
mason-lspconfig.nvim
v2.0.0
.The
handlers
feature has been removed frommason-lspconfig.nvim
. The PR adapts to that change by iterating through the servers table and callingrequire('lspconfig')[server_name].setup(server_config)
for each server. This approach maintains the use of thenvim-lspconfig
API for server configuration.This change simplifies the LSP configuration and ensures compatibility with the latest version of
mason-lspconfig.nvim
.