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
----------------
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.
================
Comment at: clangd/CodeComplete.h:85
+
+ /// Enables cursor to be moved around according to completions needs even
when
+ /// snippets are disabled. For example selecting a function with parameters
as
----------------
ilya-biryukov wrote:
> kadircet wrote:
> > ioeric wrote:
> > > IIRC, the goal of this patch is to allow disabling snippet templates for
> > > function parameters while still allow appending `()` or `($0)` to
> > > function candidates. If that's still the case, I think what we want is
> > > probably an option like `DisableSnippetTemplateForFunctionArgs`?
> > Yeah, that makes totally more sense. Thanks!
> - Maybe use positive option names to avoid double negation?
> - Maybe also use a shorter name? E.g. removing 'template' and 'disable' from
> the name gives a name that does not look any worse: `FunctionArgSnippets`.
+1 to a better name. I didn't really give much thought about the name and just
wanted to get the idea across :)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50835
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
