sammccall added a comment.
Herald added a subscriber: cfe-commits.
The test results look good, but I have trouble following the code, I think more
comments might help :-)
================
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:140
+ // similar except that we use the `RequiredQualifiers` to store the
+ // prefix of our snippet (Objective-C doesn't have C++ style
+ // qualifiers).
----------------
can we say something more concrete than "the prefix of our snippet"? this is
the `-(void)` part right?
================
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:152
//
// TODO: Make previous parameters part of the signature for Objective-C
// methods.
----------------
is this TODO handled now?
================
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:156
HadObjCArguments = true;
+ if (!HadInformativeArguments) {
+ if (RequiredQualifiers)
----------------
brief comment here should explain the significance of !HadInformativeArguments
(what it means if we know we're handling objc)
Maybe even worth an example here, not obvious to me what the data is that we're
shuffling between variables here.
================
Comment at: clang-tools-extra/clangd/CodeCompletionStrings.cpp:187
case CodeCompletionString::CK_Informative:
+ HadInformativeArguments = true;
// For example, the word "const" for a const method, or the name of
----------------
informative chunks? Not sure how we'd know they're arguments here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100798/new/
https://reviews.llvm.org/D100798
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
