[PATCH] D100798: [clangd][ObjC] Fix issue completing a method decl by name

[Sam McCall via Phabricator via cfe-commits]

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