This revision was automatically updated to reflect the committed changes.
sammccall marked 5 inline comments as done.
Closed by commit rL344620: [clangd] Refactor JSON-over-stdin/stdout code into
Transport abstraction. (authored by sammccall, committed by ).
Herald added a subscriber:
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: clangd/JSONRPCDispatcher.h:70
/// A handler responds to requests for a particular method name.
+ /// It returns true if the server should now shut down.
sammccall marked 9 inline comments as done.
sammccall added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:156
+void ClangdLSPServer::onExit(ExitParams ) {
+ // XXX handler should return true.
+}
ioeric wrote:
> ?
Fixed this comment.
I'm not totally
sammccall updated this revision to Diff 169835.
sammccall marked 9 inline comments as done.
sammccall added a comment.
Addressed review comments.
Also inverted the sense of the boolean return from `onNotify` etc, was "should
exit" and is now "should continue", which I think is slightly clearer.
ioeric added a comment.
Nice! The code looks much clearer. Just some nits
Comment at: clangd/ClangdLSPServer.cpp:156
+void ClangdLSPServer::onExit(ExitParams ) {
+ // XXX handler should return true.
+}
?
Comment at:
sammccall updated this revision to Diff 169807.
sammccall added a comment.
Add unit test for JSON transport.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53286
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
sammccall added a comment.
For reference, @jkorous has a WIP in https://reviews.llvm.org/D53290.
It's scope is a superset, and I think everything in common is basically the
same (they were both based on a common prototype).
Jan is out at the moment, so I think it makes sense to move ahead with
sammccall added a comment.
This patch is big and hard to navigate. I tried to keep it contained, but
JSONRPCDispatcher is pretty tangled.
The main parts are:
- `Transport.h` is the key new abstraction that should be understood first
- `JSONTransport.cpp` is its standard implementation. The raw
sammccall created this revision.
sammccall added reviewers: jkorous, ioeric, hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, MaskRay,
ilya-biryukov, mgorny.
This paves the way for alternative transports (mac XPC, maybe messagepack?),
and also generally improves layering:
