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

Handle less-specified initialization #193

Merged
merged 3 commits into from
Oct 23, 2019

Conversation

bollwyvl
Copy link
Contributor

@bollwyvl bollwyvl commented Oct 21, 2019

For #192.

Clients need not specify textDocument or workspace capabilities when initializing (spec).

This PR handles these potentially absent values with an existence check and a touch of destructuring and default values really long existence check chains.

@coveralls
Copy link

coveralls commented Oct 21, 2019

Coverage Status

Coverage decreased (-0.5%) to 66.516% when pulling 8fc85c0 on bollwyvl:handle-no-document-symbol into b51c7d0 on redhat-developer:master.

@bollwyvl
Copy link
Contributor Author

Yeah, i suppose this would change branch coverage... I don't readily see where to inject a different capabilities in the test suite to try it out. Any thoughts?

@JPinkney
Copy link
Contributor

There isn't anywhere currently that would allow you to write a test for the initialization properties AFAIK.

I've tested in vscode and also neovim (though I only was able to get diagnostics to work but thats probably more of a me issue) and everything seems good to me.

Were you testing your changes in neovim?

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 21, 2019 via email

@JPinkney
Copy link
Contributor

@pappasam Do you mind trying this out just to make sure everything is working on your side? If it is, I'll do a release either the 30th or the 31st with the fixes!

@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 22, 2019 via email

@pappasam
Copy link

@JPinkney I confirm this PR fixes the issue for me!

@JPinkney
Copy link
Contributor

Thanks for your contribution!

@JPinkney JPinkney merged commit 9a1c4fe into redhat-developer:master Oct 23, 2019
@bollwyvl
Copy link
Contributor Author

bollwyvl commented Oct 23, 2019 via email

@pappasam
Copy link

@JPinkney when this is part of a release, I'll update the LanguageClient-neovim's documentation to remove the version constraint: https://github.com/autozimu/LanguageClient-neovim/wiki/yaml-language-server#requirements

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.

4 participants