[PATCH] D50835: [clangd] Add parantheses while auto-completing functions.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/CodeComplete.cpp:1377
+      // Check whether function has any parameters or not.
+      LSP.textEdit->newText += SnippetSuffix.size() > 2 ? "(${0})" : "()";
+    else
----------------
kadircet wrote:
> ioeric wrote:
> > There seems to be no guarantee on whether the snippet is function arg template when `SnippetSuffix.size() > 2`. A heuristic we could use is probably checking that the template starts with `(` and ends with `)`? Alternatively, we could try to avoid generating the proper snippet according to the flag when we still have the function declaration around. Not sure if this is feasible for index results though.
> Actually I was first trying to directly look at function argument count as you mentioned, but unfortunately Symbols coming from index don't have any declarations. Therefore it turns back to looking at Signature in this case, which is same with this one. Apart from that, IIUC, SnippetSuffix contains anything only when completion item is a function otherwise it is empty. So IMO this check should be OK.
> Apart from that, IIUC, SnippetSuffix contains anything only when completion item is a function otherwise it is empty. So IMO this check should be OK.
This seems to be true for the current use cases, but I suspect the assumption can change when we add more snippet-based completion results in the future e.g. a snippet for if-statement `if (${0}) {${1}}`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50835