-
Notifications
You must be signed in to change notification settings - Fork 38
Fix deprecated use of lspconfig #428
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: main
Are you sure you want to change the base?
Fix deprecated use of lspconfig #428
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
The PR 4084 nvim-lspconfig seems to be merged, so it should be possible to migrate to the the |
|
Thanks. Please either rebase or push an empty commit so we can see if CI agrees. |
|
I hadn't seen your push but it seems CI here says this still isn't quite right, there's lots of failures. If you pull main you'll at least have the 0.10 failures go away as like I mentioned we'd have to drop 0.10 to do this (which I've now done), but the 0.11 ones still seem "real". |
ec9def7 to
a6c2efc
Compare
|
I will look into it. Maybe I forgot something in the lspconfig PR and the configuration. |
76cfe5b to
84069e8
Compare
|
I tested some of the failing test by hand and there they seem to work. Is it possible, that the automated tests do not use the nvim-lspconfig version where the leanls configs are already there? [EDIT] I have just seen, that in packpath there is the right lspconfig |
|
The test runs will use an isolated config, yes (one that generally pulls the latest versions of plugins when you run it). The issues here will be more subtle, from what I saw they have to do with the fact that the change here is not a drop in replacement -- clients are attached at different times via the new interface than via the old one, either because of different autocmd events or sequencing. |
|
Yes, I also think it is something like this, because when I use the new version outside the tests, it works just fine. I will look into the tests and how they are implemented and try to figure out if the new systems attaches LSPs differently, so that the tests do not recognize it. |
|
Ok I am currently very confused by the tests. |
|
Hi, I tried to modify these lua scripts myself and find a possible workaround: In Therefore, I simply change it into I also swapped these two line to make sure the configs are set before it get enabled. Now the ls process can get started as normal. Hope that would be helpful! |
|
Thank you, @yd278, that is very helpful. That your config parameter is nil is strange, when I run it, it is not. When I wrote the config in lspconfig I wrote |
|
Hi, @StamesJames , frankly speaking ,it doesn't. I thought it work because I open the file directly under the root directory. However, I tried a minimal setup with only: and I can confirm that no lean/lake processes were started, a debug message Maybe some other plugins did something on your machine? |
|
I tried some things in the same settings the tests run with cmd = function(dispatchers, config)
local local_cmd = { 'lake', 'serve', '--' }
return vim.lsp.rpc.start(local_cmd, dispatchers, { cwd = config.root_dir })
end,This also lets more tests succeed, but still some fail. One thing I noticed is, that when I open the project folder with |
|
I noticed, that when I manually perform |
I saw that
leanls.luais not yet in the newlsp/directory in thelspconfigplugin. I wrote the PR 4084 to thelspconfigrepo adding it. Once some default configs for leanls are inlsp/this small change should work without the message that the deprecated framework is used.This is related to Issue #427