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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
