ilya-biryukov added inline comments.
================ Comment at: clangd/ClangdLSPServer.cpp:90 void ClangdLSPServer::onInitialize(InitializeParams &Params) { + if (Params.rootUri && !Params.rootUri->file.empty()) + Server.setRootPath(Params.rootUri->file); ---------------- ioeric wrote: > This was moved from the end to the top. Is this related to the snippet change > here? Not really, I just thought it's nicer to `reply` after we process the request (e.g., if we crash for any reason it will happen before client receives the reply). Can move it back down. WDYT? ================ Comment at: clangd/Protocol.h:184 + /// Client supports snippets as insert text. + llvm::Optional<bool> snippetSupport = false; + /// Client supports commit characters on a completion item. ---------------- ioeric wrote: > Use default `Optional`? I think we would treat empty optional the same as > `false` anyway? Thanks for spotting that, I forgot to update this. ================ Comment at: clangd/tool/ClangdMain.cpp:60 -static llvm::cl::opt<bool> EnableSnippets( - "enable-snippets", ---------------- 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`). 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