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

^0.5 is broken for LanguageClient-neovim, likely others #192

Closed
pappasam opened this issue Oct 10, 2019 · 7 comments
Closed

^0.5 is broken for LanguageClient-neovim, likely others #192

pappasam opened this issue Oct 10, 2019 · 7 comments
Labels

Comments

@pappasam
Copy link

pappasam commented Oct 10, 2019

First, thanks for this language server. It's awesome and I look forward to it becoming even more awesome.

Problem

A combination of changes between 0.4.1 and 0.5.0 broke compatibility with my LSP client (LanguageClient-neovim, latest version, commit hash ee6cdb0940d9c2c95f31488f7d463b2d12abcbc4. Using neovim 0.5.0 dev). I suspect it's broken for other Vim/neovim clients as well.

Error with yaml-language-server version ^0.5

"Cannot read property 'hierarchicalDocumentSymbolSupport' of undefined"

Plethora of problems

Line 351 of src/server.ts contains the following line:

hierarchicalDocumentSymbolSupport = !!capabilities.textDocument.documentSymbol.hierarchicalDocumentSymbolSupport;

This errors out when the server capabilities does not contain a documentSymbol. The LSP specification allows a client to omit this information, but the yaml-language-server does not elegantly handle this case (hierarchicalDocumentSymbolSupport is called on the undefined "documentSymbol" attribute).

I tried updating this line to:

hierarchicalDocumentSymbolSupport = !!(capabilities.textDocument.documentSymbol && capabilities.textDocument.documentSymbol.hierarchicalDocumentSymbolSupport);

But now I receive a new error:

[LC] [Error] Notification handler 'workspace/didChangeConfiguration' failed with message: workspaceFolders is not iterable

Therefore, the remedy is more complex than simply catching the "undefined" case.

Full original error message

WARN unnamed src/language_server_protocol.rs:2217 Failed to start language server automatically. Error: Failure { jsonrpc: Some(V2), error: Error { code: InternalError, message: "Request intialize failed with message: Cannot read property \'hierarchicalDocumentSymbolSupport\' of undefined", data: None }, id: Num(1) }

Evidence from coc.nvim

Coc's yaml extension pins this library to 0.4.0, which strengthens my suspicion that yaml-language-server is broken for other vim language clients; coc users wouldn't notice the breakage because they don't use the latest version.

https://github.com/neoclide/coc-yaml/blob/master/package.json

  "dependencies": {
    "yaml-language-server": "^0.4.0"
  }

Reference

@fbricon
Copy link
Contributor

fbricon commented Oct 10, 2019

@JPinkney you need to check if the client capabilities support that feature, during initialization

@JPinkney JPinkney added the bug label Oct 10, 2019
@bollwyvl
Copy link
Contributor

Indeed! it looks like all of the properties of TextDocumentClientCapabilities and DocumentSymbolClientCapabilities are optional:

https://github.com/microsoft/vscode-languageserver-node/blob/9edf211ab020c6bef23d7095014f3fe33c3af732/protocol/src/protocol.ts#L371

@bollwyvl
Copy link
Contributor

Perhaps something as blunt as:

    workspaceFolders = params.workspaceFolders || [];

    try {
        hierarchicalDocumentSymbolSupport = !!capabilities.textDocument.documentSymbol.hierarchicalDocumentSymbolSupport;
    } catch { }

    try {
        clientDynamicRegisterSupport = !!(capabilities.textDocument.rangeFormatting && capabilities.textDocument.rangeFormatting.dynamicRegistration);
    } catch { }

@pappasam
Copy link
Author

@JPinkney I confirm that @bollwyvl 's proposed "blunt" change fixes the issue, at least for me on LanguageClient-neovim. Not sure if that's the change we ultimately want to see in this codebase, but it's a potential path forward.

@JPinkney
Copy link
Contributor

I'm still trying to get neovim working and everything installed but can we do something like:

if (!!capabilities.textDocument && !!capabilities.textDocument.documentSymbol......)

instead?

@bollwyvl do you mind submitting a pr with this code?

@bollwyvl
Copy link
Contributor

bollwyvl commented Oct 16, 2019 via email

bollwyvl added a commit to bollwyvl/yaml-language-server that referenced this issue Oct 21, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@JPinkney
Copy link
Contributor

I've just released 0.6.0 which contains these fixes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants