[PATCH] D43229: [clangd] Enable snippet completion based on client capabilities.
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 15 04:40:06 PST 2018
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
More information about the cfe-commits
mailing list