[PATCH] D129579: [clangd] Remove `allCommitCharacters`

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 12 10:54:58 PDT 2022


sammccall created this revision.
sammccall added reviewers: kadircet, hokein.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This was added in 2a095ff6f5028b76 <https://reviews.llvm.org/rG2a095ff6f5028b7612dae50358a2f6939b6cdc9f>, 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).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129579

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


Index: clang-tools-extra/clangd/test/initialize-params.test
===================================================================
--- clang-tools-extra/clangd/test/initialize-params.test
+++ 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:          ".",
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -534,10 +534,6 @@
        }},
       {"completionProvider",
        llvm::json::Object{
-           {"allCommitCharacters",
-            {" ", "\t", "(", ")", "[", "]", "{",  "}", "<",
-             ">", ":",  ";", ",", "+", "-", "/",  "*", "%",
-             "^", "&",  "#", "?", ".", "=", "\"", "'", "|"}},
            {"resolveProvider", false},
            // We do extra checks, e.g. that > is part of ->.
            {"triggerCharacters", {".", "<", ">", ":", "\"", "/", "*"}},


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129579.444015.patch
Type: text/x-patch
Size: 2057 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220712/e0fc7b05/attachment.bin>


More information about the cfe-commits mailing list