ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: clangd/CodeCompletionStrings.cpp:122
+      // Typed-text chunk is the actual name. Text before it is qualifiers.
+      if (Qualifiers)
+        *Qualifiers = std::move(*Signature);
----------------
Qualifier seems a bit ambiguous for this. The documentation for `CK_TypedText` 
says:
```
    /// The piece of text that the user is expected to type to
    /// match the code-completion string, typically a keyword or the name of a
    /// declarator or macro.
    CK_TypedText,
```

For example, it could also be the keyword in "typedef ^". Maybe just 
`TypedText` would be more straightforward? 


================
Comment at: unittests/clangd/CodeCompletionStringsTests.cpp:70
   Builder.AddResultTypeChunk("result no no");
-  labelAndInsertText(*Builder.TakeString());
-  EXPECT_EQ(Label, "X");
-  EXPECT_EQ(InsertText, "X");
+  getSignature(*Builder.TakeString());
+  EXPECT_EQ(Signature, "");
----------------
nit: I find `getSignature` a bit awkward here because we are not getting 
anything back. Maybe `setSignature`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48475



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

Reply via email to