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

Add a test for #1376 #1414

Merged
merged 3 commits into from Feb 20, 2021
Merged

Add a test for #1376 #1414

merged 3 commits into from Feb 20, 2021

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Feb 20, 2021

The test passes, and I cannot repro the issue with VSCode, so the issue must be down to a difference in how CoC and VSCode implement the LSP spec.

I believe it worked before the lsp-1.0 upgrade, which means that something has changed between haskell-lsp and lsp in how the initial configuration is handled.

The full LSP log (see the Gist below) from the test shows that lsp-test generates a workspace/didChangeConfiguration with the formatter config. It could be that CoC doesn't send this notification, and instead expects the server to request the config. @wz1000 @bubba is it possible that haskell-lsp does request the config at startup, whereas lsp does not?

https://gist.github.com/pepeiborra/fcad435bdf8717ceeecf392549329c19

There's nothing else to do here, so this closes #1376. The next step is to either teach lsp to request the config at startup, or get CoC to send the workspace/didChangeConfiguration as VSCode does. /cc @andys8

I also took the chance to remove an unnecessary use of the AGPL flag in the test suite

@wz1000
Copy link
Collaborator

wz1000 commented Feb 20, 2021

Ahh, I see the problem. Your diagnosis is very helpful.

VSCode doesn't actually send any config on startup with the initialize request (as I believe this is depreciated in the LSP spec), so with lsp-1.0, we stopped trying to parse the configuration from the initialization. VSCode does send a workspace/didChangeConfiguration immediately after startup, which we still do parse.

The short-term solution would be restoring the original behavior of lsp, to parse the configuration from the initial request. I will implement this today. I believe CoC must still be following the old model and including the configuration as part of initialize.

However in the long term, I believe that relying on this behavior is deprecated by the spec, and we should be requesting the config explicitly using workspace/configuration. The trouble with that is that the spec is really unclear on what the semantics of that request actually are.

microsoft/language-server-protocol#972 (comment)

@wz1000
Copy link
Collaborator

wz1000 commented Feb 20, 2021

haskell/lsp#285

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.

Brittany formatting not working anymore, using a different formatter
2 participants