sammccall added inline comments.
================ Comment at: clangd/tool/ClangdMain.cpp:60 -static llvm::cl::opt<bool> EnableSnippets( - "enable-snippets", ---------------- ilya-biryukov wrote: > ioeric wrote: > > Would we still have a way to disable snippets (e.g. for debugging) even if > > clients support them? Maybe make this a hidden option instead? > I'm tempted to say in that case we could just recompile clangd so that it > ignores the options (i.e. change `onInitialized`). The code that will take it > into account would be a bit trickier (you can't just pass > `CodeCompleteOptions` to `ClangdLSPServer`, we'll also have to pass > `Optional<bool> OverridenEnableSnippets`). +1 to this - this is really a debugging option, and supporting it has a cost. If we want debugging options to live forever (may be reasonable!) we need a more scalable way to define them than listing them all in ClangdMain. ================ Comment at: test/clangd/completion-snippets-disabled.test:1 +# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s +# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s ---------------- Please don't add three new lit tests for this. One should be certainly enough (off, assuming our normal completion test has this feature on). But really I think this can be a unit test in CodeComplete tests instead, right? If you really want to verify how missing `snippetSupport` parses, that's a unittest for protocol.h right? But it doesn't seem important. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43229 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
