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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
