Skip to content

Explicitly define our TestDocumentSync capabilities#613

Merged
ccordoba12 merged 1 commit intopalantir:developfrom
mpanarin:define_capabilities
Jul 15, 2019
Merged

Explicitly define our TestDocumentSync capabilities#613
ccordoba12 merged 1 commit intopalantir:developfrom
mpanarin:define_capabilities

Conversation

@mpanarin
Copy link
Copy Markdown
Contributor

@mpanarin mpanarin commented Jul 2, 2019

According to specification, passing a number in TextDocumentSync is for legacy support. I think we should move to an explicit declaration of capabilites, as modern clients require that (ex. Emacs lsp-mode will not send notifications that are not defined in capabilities, which is correct, imo).

@palantirtech
Copy link
Copy Markdown
Member

Thanks for your interest in palantir/python-language-server, @mpanarin! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@ccordoba12
Copy link
Copy Markdown
Contributor

@andfoy, please take a look at this one.

Comment thread pyls/python_ls.py
@mpanarin mpanarin force-pushed the define_capabilities branch from 9861806 to a347596 Compare July 2, 2019 12:48
@mpanarin
Copy link
Copy Markdown
Contributor Author

mpanarin commented Jul 2, 2019

@gatesn oops, updated.

@mpanarin
Copy link
Copy Markdown
Contributor Author

@ccordoba12
ping. As I addressed the comment by @gatesn
Maybe this can be merged?

@ccordoba12 ccordoba12 changed the title [fix] explicitly define our TestDocumentSync capabilities Explicitly define our TestDocumentSync capabilities Jul 15, 2019
@ccordoba12 ccordoba12 added this to the 0.28.0 milestone Jul 15, 2019
@ccordoba12
Copy link
Copy Markdown
Contributor

You're right, thanks for the reminder.

@ccordoba12 ccordoba12 merged commit e261b2c into palantir:develop Jul 15, 2019
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