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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits