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

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clangd/tool/ClangdMain.cpp:197
 
+static llvm::cl::opt<bool> IncludeFixIts(
+    "include-fixits",
----------------
ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > I wonder if we should make the `IncludeFixIts` option hidden?
> > > It currently only works for our YCM integration, VSCode and other clients break.
> > why would a user want to turn this on or off?
> Ideally, we want to have it always on.
> However, all editors interpret the results we return in different ways. This is a temporary option until we can define how text edits are handled by LSP.
> We filed the bugs, will dig them up on Monday.
Do we have any more details here? I'm still skeptical that exposing this to end users will help at all, this seems likely that it should be a capability if we do need it.


================
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 "
----------------
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?
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})`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51214