[PATCH] D51214: [clangd] Add options to enable/disable fixits and function argument snippets.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/tool/ClangdMain.cpp:205
+static llvm::cl::opt<bool> EnableFunctionArgSnippets(
+    "enable-function-arg-snippets",
+    llvm::cl::desc("Gives snippets for function arguments, when disabled only "
----------------
sammccall wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > I wonder if we can infer this setting from the `-completion-style' (`=bundled` implies `enable-function-arg-snippets == false`)
> > > > > @sammccall, WDYT?
> > > > They're not inextricably linked, the main connection is "these options both need good signaturehelp support to be really useful".
> > > > But we might want to link them just to cut down on number of options.
> > > > 
> > > > I don't have a strong opinion, I don't use `-completion-style=bundled`. Can we find a few people who do and ask if they like this setting?
> > > I would (obviously) want these two options to be packaged together whenever I use `=bundled`.
> > > But I also use VSCode, which has signatureHelp.
> > > 
> > > I think @ioeric wanted to try out the `=bundled` completion style. @ioeric, WDYT?
> > ping on this discussion to @ioeric , but I think linking seems like a good idea. Because it wouldn't be very useful if one already selects a specific overload from completion list.
> This seems reasonable to have as a preference, I'm also fine combining it with bundled.
> 
> If you keep it, naming nits:
>  - drop "enable" prefix (our other bool flags don't have this)
>  - snippets -> placeholders (snippets is both jargon and technically incorrect - we still emit snippets like `foo({$0})`.
I don't think YCM completes the snippets currently, so Eric won't notice the change for bundled vs normal completions.

Anyhow, keeping it as a separate option is a safe bet. 

And some personal experience: have been using bundled completions for a few weeks now and will probably switch this one on immediately when it lands.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214