[PATCH] D48475: [clangd] More precise representation of symbol names/labels in the index.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked an inline comment as done.
sammccall added inline comments.


================
Comment at: clangd/CodeCompletionStrings.cpp:122
+      // Typed-text chunk is the actual name. Text before it is qualifiers.
+      if (Qualifiers)
+        *Qualifiers = std::move(*Signature);
----------------
ioeric wrote:
> 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? 
Changed to "RequiredQualifiers" and specified it more clearly in the method comment.

Also added an implementation comment here as it's a little subtle. We're not outputting the current chunk to Qualifiers, but rather moving all the text we recorded so far.


================
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, "");
----------------
ioeric wrote:
> nit: I find `getSignature` a bit awkward here because we are not getting anything back. Maybe `setSignature`?
Agreed. Changed to `computeSignature`.

(I'm not sure the current test structure with the inputs and outputs being fixture members is great, but I don't really want to shave that yak now)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48475