[PATCH] D89870: [clangd] Drop template argument lists from completions followed by <

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

(sorry for forgetting about this)



================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:450
+        if (Snippet->front() == '<')
+          return Snippet->substr(0, Snippet->find('('));
+        return "";
----------------
what if we have `(` in the template parameter list ? i think we need to literally find the matching `>` and include all the text in between instead.


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:456
+      // template argument list but it would be complicated.
+      if (NextTokenKind == tok::less && Snippet->front() == '<')
+        return "";
----------------
nit: again might be worth putting the easy-to-reason case above to reduce mental load when reading.


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:459
+    }
     if (!Snippet)
       // All bundles are function calls.
----------------
nit: move this case above and get rid of `if(Snippet` part of the check?


================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2982
   )cpp";
+  // FIXME(kirillbobyrev): Also strip prefix of identifier token before the
+  // cursor from completion item ("fo" in this case).
----------------
umm, where did this fixme come from exactly? don't we emit these as replacements for the `fo` part of the text anyway ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89870/new/

https://reviews.llvm.org/D89870