ilya-biryukov added inline comments.
================
Comment at: clangd/ClangdLSPServer.cpp:59
void ClangdLSPServer::onShutdown(Ctx C, ShutdownParams &Params) {
- IsDone = true;
+ // Before we reply, we could serialize the preambles to disk. For now, let's
+ // just say we're ready to exit.
----------------
The comment is a bit misleading. I'd simply say we don't do need to do anything
on `shutdown` at this point.
offtopic:
Preambles is not something we'd want to serialise (they are rather fast to
build, and we only need them to make code completion fast), properly saving
state of various indexes when we'll have them is something we'll definitely
need to do on shutdown, though.
================
Comment at: clangd/tool/ClangdMain.cpp:117
ResourceDirRef, CompileCommandsDirPath);
- LSPServer.run(std::cin);
+ return LSPServer.run(std::cin) ? 0 : 1;
}
----------------
Maybe add a `const NoShutdownRequestErrorCode = 1;` and use it here instead of
`1`?
Or add a comment on why we exit with error code. (It's not entirely obvious to
someone who hasn't read this part of LSP spec)
================
Comment at: test/clangd/protocol.test:1
-# RUN: clangd -run-synchronously < %s | FileCheck %s
-# RUN: clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
+# RUN: not clangd -run-synchronously < %s | FileCheck %s
+# RUN: not clangd -run-synchronously < %s 2>&1 | FileCheck
-check-prefix=STDERR %s
----------------
Maybe add a comment why we use `not` here?
It's not obvious for someone who hasn't read the whole test file.
https://reviews.llvm.org/D38939
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
