ilya-biryukov added inline comments.

================
Comment at: clangd/tool/ClangdMain.cpp:202
+        "Like changing an arrow(->) member access to dot(.) member access."),
+    llvm::cl::init(clangd::CodeCompleteOptions().IncludeFixIts));
+
----------------
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.


================
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:
> 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?


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

Reply via email to