Skip to content

If client declares supports, provide snippet completions by default#671

Closed
joaotavora wants to merge 1 commit intopalantir:developfrom
joaotavora:develop
Closed

If client declares supports, provide snippet completions by default#671
joaotavora wants to merge 1 commit intopalantir:developfrom
joaotavora:develop

Conversation

@joaotavora
Copy link
Copy Markdown

This is the way python-language-server used to work before #519. This
shouldn't conflict with the goals of #515 of #526, i.e. if the client
does not declare support for snippets, none are sent.

  • pyls/plugins/jedi_completion.py (pyls_completions): Default
    should_include_params to True, like before.

This is the way python-language-server used to work before palantir#519.  This
shouldn't conflict with the goals of palantir#515 of palantir#526, i.e. if the client
does _not_ declare support for snippets, none are sent.

* pyls/plugins/jedi_completion.py (pyls_completions): Default
should_include_params to True, like before.
@palantirtech
Copy link
Copy Markdown
Member

Thanks for your interest in palantir/python-language-server, @joaotavora! 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.

@joaotavora
Copy link
Copy Markdown
Author

Signing many pages of agreement just for (re)adding a trivial , True isn't reasonable for me. Take this PR as a suggestion.

@ccordoba12
Copy link
Copy Markdown
Contributor

Take this PR as a suggestion.

Thanks but we're not interested in this.

@ccordoba12 ccordoba12 closed this Oct 11, 2019
@joaotavora
Copy link
Copy Markdown
Author

Thanks but we're not interested in this.

Would you be so kind as to provide a reason? It's very much in vogue these days in the OSS world, providing a short explanation for why something isn't acceptable.

@ccordoba12
Copy link
Copy Markdown
Contributor

I think it's better for clients to declare if they want to use this feature instead of imposing it on them.

@joaotavora
Copy link
Copy Markdown
Author

I think it's better for clients to declare if they want to use this feature instead of imposing it on them.

Thanks for getting back to me with an explanation. I completely agree with you. But if you read my pull request correctly, you will see that what you want to avoid is not what my pull request is doing.

Is is true that before dfff3c9, pyls was imposing snippets. but dfff3c9 went a bit too far.

With my commit, if a client does not want to use the feature, it issues snippetSupport: False: the pyls code guarantees that snippets will not be imposed. But if it issues snippetSupport: True, then pyls should provide snippets without requiring that the user somehow additionally configures the server or Jedi to send snippets.

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.

3 participants