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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm!



================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:474
+        //
+        // fu^(42) -> function(42);
+        if (Snippet->front() == '<') {
----------------
RHS should be `function<int>(42)` right?


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:480
+          unsigned Balance, Index;
+          for (Balance = 1, Index = 1; Balance && (Index != Snippet->size());
+               ++Index) {
----------------
nit:
```
int Balance = 0;
size_t I = 0;
do {
  if(Snippet[I] == '>') --Balance;
  else if(Snippet[I] == '<') ++Balance;
  ++I;
} while(Balance > 0);
return Snippet->substr(0, I);
```

This should handle both the case snippet starts with `<` and not. reducing the nesting a little and making the flow more uniform (i.e. getting rid of the second return statement).


Up to you though, if you keep the existing version please move definition of `Balance` into for-init statement, use `size_t` instead of `unsigned` and array subscripts instead of `at(Index)`.


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:496
+      //
+      // fu^<int>(42) -> function<int>(42);
+      if (NextTokenKind == tok::less && Snippet->front() == '<')
----------------
i think it is better to omit `(42)` in this example.


================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:497
+      // fu^<int>(42) -> function<int>(42);
+      if (NextTokenKind == tok::less && Snippet->front() == '<')
+        return "";
----------------
You've marked the previous nit as done, but this case still seems to be coming after the complicated case, just in case it slipped :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89870