[PATCH] D65387: [clangd] Add a callback mechanism for handling responses from client.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:205
+  // Return a call id of the request.
+  int bindReply(Callback<llvm::json::Value> Reply) {
+    llvm::Optional<std::pair<int, Callback<llvm::json::Value>>> OldestCB;
----------------
sammccall wrote:
> nit: I think this function could return json::Value, encapsulating the fact that we use integers inside the MessageHandler class.
> 
> (Or does something outside the class depend on this?)
we just use it for logging, changing it to Value is totally fine.


================
Comment at: clang-tools-extra/clangd/test/fixits-command.test:206
 ---
+{"jsonrpc":"2.0","id":0,"result":{"applied":true}}
+#      CHECK:  "id": 4,
----------------
sammccall wrote:
> this ID is reused
this ID is not reused, it is for server request `applyEdit` (see line `168`).


================
Comment at: clang-tools-extra/clangd/test/request-reply.test:28
+---
+{"jsonrpc":"2.0","id":4,"method":"workspace/executeCommand","params":{"command":"clangd.applyTweak","arguments":[{"file":"file:///clangd-test/main.cpp","selection":{"end":{"character":4,"line":0},"start":{"character":0,"line":0}},"tweakID":"ExpandAutoType"}]}}
+#      CHECK:  "id": 1,
----------------
sammccall wrote:
> ID reuse here (4 was previously used on line 6)
good catch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65387/new/

https://reviews.llvm.org/D65387