Skip to content
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

WorkspaceDidChangeConfiguration does not work if static? #547

Closed
yav opened this issue Jan 24, 2024 · 7 comments
Closed

WorkspaceDidChangeConfiguration does not work if static? #547

yav opened this issue Jan 24, 2024 · 7 comments

Comments

@yav
Copy link

yav commented Jan 24, 2024

I just spent quite a bit of time trying to get VS Code to send me notifications when the configuration for my language server changes. I managed to get it to work by manually calling registerCability on the titular method in the SMethod_Initialized, bur for some reason it would not work if WorkspaceDidChangeConfiguration was listed in the static handlers. This seems confusing.

Given that the library already does quite a lot of custom handling of configuration processing, it would be a lot less confusing if the static version just worked. In fact, wouldn't it make sense to always enable this, given that the options contain an onConfigChange function?

@michaelpj
Copy link
Collaborator

We certainly do handle those requests automatically, and also support them being set as static handlers.

However, there is no server capability for these, so AFAICT whether or not to send them is entirely up to the client. But perhaps if you dynamically register then that tells the client you want them?

I suspect what is happening is that this is actually VS Code dropping support for the old push-based configuration model. See the text here: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#workspace_configuration

That seems to confirm my suspicion. This is a problem for the lsp library indeed, since while we do send workspace/configuration automatically, we do assume that we're going to get workspace/didChangeConfiguration notifications in order to know when to send it. Otherwise it seems like they're expecting you to send a configuration request every time you ever want to consult the configuration, which seems bizarre.

So I think what this means is that lsp should automatically dynamically register for workspace/didChangeConfiguration. Sigh. I'll try to get on this quickly, since this probably means that configuration is broken for everyone using lsp with VS Code right now :(

@michaelpj
Copy link
Collaborator

I opened microsoft/language-server-protocol#1888 on the spec, but for now we need to deal with the current reality.

@michaelpj
Copy link
Collaborator

Using an older vscode-languageclient version might be a workaround, apparently we don't have this problem in the vscode-haskell extension.

@michaelpj
Copy link
Collaborator

@yav any chance you could see if #548 fixes your issue?

@yav
Copy link
Author

yav commented Jan 29, 2024

@michaelpj it appears to work, thanks!

@michaelpj
Copy link
Collaborator

Great, I'll try and get the lsp-test stuff done and get this merged and released.

@michaelpj
Copy link
Collaborator

Released a version with the fix.

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

No branches or pull requests

2 participants