ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.
It seems this patch is out of date, we need to merge it with the latests head.
================
Comment at: clangd/DraftStore.cpp:45
std::lock_guard<std::mutex> Lock(Mutex);
+ assert(!File.empty());
----------------
Why do we need this assert? I don't see how empty file is worse than any other
invalid value.
================
Comment at: clangd/DraftStore.cpp:55
std::lock_guard<std::mutex> Lock(Mutex);
+ assert(!File.empty());
----------------
Why do we need this assert? I don't see how empty file is worse than any other
invalid value.
================
Comment at: clangd/GlobalCompilationDatabase.h:65
+ void setCompileCommandsDir(Path P) {
+ CompileCommandsDir = P;
+ CompilationDatabases.clear();
----------------
We need to lock the Mutex here!
================
Comment at: clangd/Protocol.cpp:368
+json::Expr toJSON(const DidChangeConfigurationParams &CCP) {
+
+ return json::obj{
----------------
NIT: maybe remove this empty line?
================
Comment at: test/clangd/did-change-configuration.test:33
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///compile_commands.json","languageId":"json","version":1,"text":"[\n{\n"directory":"/",\n"command":"/usr/bin/c++-DGTEST_HAS_RTTI=0-D_GNU_SOURCE-D__STDC_CONSTANT_MACROS-D__STDC_FORMAT_MACROS-D__STDC_LIMIT_MACROS-Ilib/Demangle-I../lib/Demangle-I/usr/include/libxml2-Iinclude-I../include-fPIC-fvisibility-inlines-hidden-Werror=date-time-std=c++11-Wall-W-Wno-unused-parameter-Wwrite-strings-Wcast-qual-Wno-missing-field-initializers-pedantic-Wno-long-long-Wno-maybe-uninitialized-Wdelete-non-virtual-dtor-Wno-comment-O0-g-fno-exceptions-fno-rtti-o/foo.c.o-c/foo.c",\n"file":"/foo.c"\n},"}}}
+
----------------
Nebiroth wrote:
> ilya-biryukov wrote:
> > clangd won't see this file. `didOpen` only sets contents for diagnostics,
> > not any other features.
> > You would rather want to add more `# RUN:` directives at the top of the
> > file to create `compile_commands.json`, etc.
> >
> > Writing it under root ('/') is obviously not an option. Lit tests allow you
> > to use temporary paths, this is probably an approach you could take. See [[
> > https://llvm.org/docs/TestingGuide.html#substitutions | lit docs ]] for
> > more details.
> Are there examples available on how to use this? I have to use a # RUN: to
> create a file and then use it's path in a workspace/didChangeConfiguration
> notification?
After thinking about it, I really don't see an easy way to test it currently.
We just don't have the infrastructure in place for that.
I think it's fine to add a FIXME to the body of
`ClangdLSPServer::onChangeConfiguration` that we need to test it and remove
this test.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
