kadircet 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.
+1 to @ilya-biryukov . We might need to wait for a while until all editors
supports additionalTextEdits in a sane way.
================
Comment at: clangd/tool/ClangdMain.cpp:202
+ "Like changing an arrow(->) member access to dot(.) member access."),
+ llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+
----------------
ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > Maybe specify the value here explicitly?
> > > The defaults for users of the clangd binary (what the option is for) is
> > > not necessarily the best default for the `ClangdServer` clients (what the
> > > default in the struct is for).
> > > More importantly, it's useful to be more straightforward about the
> > > defaults we have for the options.
> > Not sure I agree here, this is consistent with the other options above.
> > It's the other `ClangdServer` clients that are the weirdos, they should
> > have to be explicit :-)
> >
> > The defaults are clear when running with `-help`, which is how users will
> > see them.
> > The defaults are clear when running with -help, which is how users will see
> > them.
> Sure, but I would still argue reading the code is simpler without extra
> indirection and defaults of the binary are not necessarily the defaults we
> use in the APIs.
>
> But also feel free to keep the code as is, don't think it's terribly
> important.
Keeping it as it is.
================
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?
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.
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
