[PATCH] D39571: [clangd] DidChangeConfiguration Notification

Simon Marchi via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 24 08:20:54 PST 2018


simark added inline comments.


================
Comment at: clangd/ClangdLSPServer.cpp:302
 
+// FIXME: This function needs to be properly tested.
+void ClangdLSPServer::onChangeConfiguration(
----------------
ilya-biryukov wrote:
> Are you planning to to address this FIXME before checking the code in?
Following what you said here:

https://reviews.llvm.org/D39571?id=124024#inline-359345

I have not really looked into what was wrong with the test, and what is missing in the infrastructure to make it work.  But I assumed that the situation did not change since then.  Can you enlighten me on what the problem was, and what is missing?


================
Comment at: clangd/GlobalCompilationDatabase.h:64
 
+  void setCompileCommandsDir(Path P) {
+    std::lock_guard<std::mutex> Lock(Mutex);
----------------
ilya-biryukov wrote:
> Maybe move definition to `.cpp` file?
> 
> 
Will do.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571





More information about the cfe-commits mailing list