[PATCH] D128621: [clangd] Do not try to use $0 as a placeholder in completion snippets

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 447112.
nridge added a comment.

Address final review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128621/new/

https://reviews.llvm.org/D128621

Files:
  clang-tools-extra/clangd/CodeCompletionStrings.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp


Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -150,7 +150,7 @@
 
   // When completing a pattern, the last placeholder holds the cursor position.
   computeSignature(MakeCCS(), /*CompletingPattern=*/true);
-  EXPECT_EQ(Snippet, " ${1:name} = ${0:target};");
+  EXPECT_EQ(Snippet, " ${1:name} = $0;");
 }
 
 TEST_F(CompletionStringTest, IgnoreInformativeQualifier) {
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3137,9 +3137,8 @@
 
   // Last placeholder in code patterns should be $0 to put the cursor there.
   EXPECT_THAT(Results.Completions,
-              Contains(AllOf(
-                  named("while"),
-                  snippetSuffix(" (${1:condition}) {\n${0:statements}\n}"))));
+              Contains(AllOf(named("while"),
+                             snippetSuffix(" (${1:condition}) {\n$0\n}"))));
   // However, snippets for functions must *not* end with $0.
   EXPECT_THAT(Results.Completions,
               Contains(AllOf(named("while_foo"),
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===================================================================
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -192,11 +192,15 @@
     case CodeCompletionString::CK_Placeholder:
       *Signature += Chunk.Text;
       ++SnippetArg;
-      *Snippet +=
-          "${" +
-          std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) + 
':';
-      appendEscapeSnippet(Chunk.Text, Snippet);
-      *Snippet += '}';
+      if (SnippetArg == CursorSnippetArg) {
+        // We'd like to make $0 a placeholder too, but vscode does not support
+        // this (https://github.com/microsoft/vscode/issues/152837).
+        *Snippet += "$0";
+      } else {
+        *Snippet += "${" + std::to_string(SnippetArg) + ':';
+        appendEscapeSnippet(Chunk.Text, Snippet);
+        *Snippet += '}';
+      }
       break;
     case CodeCompletionString::CK_Informative:
       HadInformativeChunks = true;



Index: clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -150,7 +150,7 @@
 
   // When completing a pattern, the last placeholder holds the cursor position.
   computeSignature(MakeCCS(), /*CompletingPattern=*/true);
-  EXPECT_EQ(Snippet, " ${1:name} = ${0:target};");
+  EXPECT_EQ(Snippet, " ${1:name} = $0;");
 }
 
 TEST_F(CompletionStringTest, IgnoreInformativeQualifier) {
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3137,9 +3137,8 @@
 
   // Last placeholder in code patterns should be $0 to put the cursor there.
   EXPECT_THAT(Results.Completions,
-              Contains(AllOf(
-                  named("while"),
-                  snippetSuffix(" (${1:condition}) {\n${0:statements}\n}"))));
+              Contains(AllOf(named("while"),
+                             snippetSuffix(" (${1:condition}) {\n$0\n}"))));
   // However, snippets for functions must *not* end with $0.
   EXPECT_THAT(Results.Completions,
               Contains(AllOf(named("while_foo"),
Index: clang-tools-extra/clangd/CodeCompletionStrings.cpp
===================================================================
--- clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -192,11 +192,15 @@
     case CodeCompletionString::CK_Placeholder:
       *Signature += Chunk.Text;
       ++SnippetArg;
-      *Snippet +=
-          "${" +
-          std::to_string(SnippetArg == CursorSnippetArg ? 0 : SnippetArg) + ':';
-      appendEscapeSnippet(Chunk.Text, Snippet);
-      *Snippet += '}';
+      if (SnippetArg == CursorSnippetArg) {
+        // We'd like to make $0 a placeholder too, but vscode does not support
+        // this (https://github.com/microsoft/vscode/issues/152837).
+        *Snippet += "$0";
+      } else {
+        *Snippet += "${" + std::to_string(SnippetArg) + ':';
+        appendEscapeSnippet(Chunk.Text, Snippet);
+        *Snippet += '}';
+      }
       break;
     case CodeCompletionString::CK_Informative:
       HadInformativeChunks = true;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits