[clang-tools-extra] 93cd159 - [clangd] Remove `allCommitCharacters`

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 12 12:42:53 PDT 2022


Author: Sam McCall
Date: 2022-07-12T21:42:38+02:00
New Revision: 93cd159ca9d3cfd4afcd20cbfba9ef789a80bdda

URL: https://github.com/llvm/llvm-project/commit/93cd159ca9d3cfd4afcd20cbfba9ef789a80bdda
DIFF: https://github.com/llvm/llvm-project/commit/93cd159ca9d3cfd4afcd20cbfba9ef789a80bdda.diff

LOG: [clangd] Remove `allCommitCharacters`

This was added in 2a095ff6f5028b76, however it never worked with VSCode
due to bugs in vscode-languageclient
(https://github.com/microsoft/vscode-languageserver-node/issues/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 https://github.com/clangd/vscode-clangd/pull/358
See also https://github.com/clangd/vscode-clangd/pull/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

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdLSPServer.cpp
    clang-tools-extra/clangd/test/initialize-params.test

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 54e6765be315b..faef5de14f3fb 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -534,10 +534,11 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
        }},
       {"completionProvider",
        llvm::json::Object{
-           {"allCommitCharacters",
-            {" ", "\t", "(", ")", "[", "]", "{",  "}", "<",
-             ">", ":",  ";", ",", "+", "-", "/",  "*", "%",
-             "^", "&",  "#", "?", ".", "=", "\"", "'", "|"}},
+           // We don't set `(` etc as allCommitCharacters as they interact
+           // poorly with snippet results.
+           // See https://github.com/clangd/vscode-clangd/issues/357
+           // Hopefully we can use them one day without this side-effect:
+           //     https://github.com/microsoft/vscode/issues/42544
            {"resolveProvider", false},
            // We do extra checks, e.g. that > is part of ->.
            {"triggerCharacters", {".", "<", ">", ":", "\"", "/", "*"}},

diff  --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test
index da964ecbd14bb..c795ab5940432 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -13,35 +13,6 @@
 # CHECK-NEXT:        "automaticReload": true
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "completionProvider": {
-# CHECK-NEXT:        "allCommitCharacters": [
-# CHECK-NEXT:          " ",
-# CHECK-NEXT:          "\t",
-# CHECK-NEXT:          "(",
-# CHECK-NEXT:          ")",
-# CHECK-NEXT:          "[",
-# CHECK-NEXT:          "]",
-# CHECK-NEXT:          "{",
-# CHECK-NEXT:          "}",
-# CHECK-NEXT:          "<",
-# CHECK-NEXT:          ">",
-# CHECK-NEXT:          ":",
-# CHECK-NEXT:          ";",
-# CHECK-NEXT:          ",",
-# CHECK-NEXT:          "+",
-# CHECK-NEXT:          "-",
-# CHECK-NEXT:          "/",
-# CHECK-NEXT:          "*",
-# CHECK-NEXT:          "%",
-# CHECK-NEXT:          "^",
-# CHECK-NEXT:          "&",
-# CHECK-NEXT:          "#",
-# CHECK-NEXT:          "?",
-# CHECK-NEXT:          ".",
-# CHECK-NEXT:          "=",
-# CHECK-NEXT:          "\"",
-# CHECK-NEXT:          "'",
-# CHECK-NEXT:          "|"
-# CHECK-NEXT:        ],
 # CHECK-NEXT:        "resolveProvider": false,
 # CHECK-NEXT:        "triggerCharacters": [
 # CHECK-NEXT:          ".",


        


More information about the cfe-commits mailing list