Disable commit-characters for all code completion. #358
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The use of commit-characters in clangd had good intentions, but it has terrible UX in practice in VSCode, as documented on the linked bug. We didn't notice this until it was far too late: the commit characters were added in clangd 12, but bugs in vscode-languageclient prevented them from being noticed until languageclient-8. (microsoft/vscode-languageserver-node#673) At this point we can't go back and change the server behavior so we need to override it on the client. Fixes clangd#357
|
Locally verified that reapplying eb61c51 leads to the bad behavior reported in the bug, and then applying this patch reverts to the old code completion behavior. |
hokein
approved these changes
Jul 12, 2022
sam-mccall
added a commit
to llvm/llvm-project
that referenced
this pull request
Jul 12, 2022
This was added in 2a095ff, however it never worked with VSCode due to bugs in vscode-languageclient (microsoft/vscode-languageserver-node#673). Now that it does work, we can tell the interactions with text editing, with snippets, and vscode's select-first-completion behavior are bad. The spec is vague and clients could do something reasonable with the current values. However they could clearly do something unreasonable too, and over time behavior+spec tends to converge on VSCode's behavior. This addresses clangd/vscode-clangd#358 See also clangd/vscode-clangd#358 which hotfixes this on the client-side (as we can't apply this change retroactively to clangd 12-14). Differential Revision: https://reviews.llvm.org/D129579
mem-frob
pushed a commit
to draperlaboratory/hope-llvm-project
that referenced
this pull request
Oct 7, 2022
This was added in 2a095ff6f5028b76, however it never worked with VSCode due to bugs in vscode-languageclient (microsoft/vscode-languageserver-node#673). Now that it does work, we can tell the interactions with text editing, with snippets, and vscode's select-first-completion behavior are bad. The spec is vague and clients could do something reasonable with the current values. However they could clearly do something unreasonable too, and over time behavior+spec tends to converge on VSCode's behavior. This addresses clangd/vscode-clangd#358 See also clangd/vscode-clangd#358 which hotfixes this on the client-side (as we can't apply this change retroactively to clangd 12-14). Differential Revision: https://reviews.llvm.org/D129579
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The use of commit-characters in clangd had good intentions, but it has
terrible UX in practice in VSCode, as documented on the linked bug.
We didn't notice this until it was far too late: the commit characters
were added in clangd 12, but bugs in vscode-languageclient prevented
them from being noticed until languageclient-8.
(microsoft/vscode-languageserver-node#673)
At this point we can't go back and change the server behavior so we need
to override it on the client.
Fixes #357