ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: clangd/ClangdLSPServer.cpp:90
 void ClangdLSPServer::onInitialize(InitializeParams &Params) {
+  if (Params.rootUri && !Params.rootUri->file.empty())
+    Server.setRootPath(Params.rootUri->file);
----------------
This was moved from the end to the top. Is this related to the snippet change 
here?


================
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.
----------------
Use default `Optional`? I think we would treat empty optional the same as 
`false` anyway?


================
Comment at: clangd/tool/ClangdMain.cpp:60
 
-static llvm::cl::opt<bool> EnableSnippets(
-    "enable-snippets",
----------------
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?


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

Reply via email to