hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:60 + void + onHighlightingsReady(PathRef File, + std::vector<HighlightingToken> Highlightings) override; ---------------- jvikstrom wrote: > hokein wrote: > > nit: you can remove this override, since we have provided an empty default > > implementation. > I get an `-Winconsistent-missing-override` warnings without the override. sorry for the confusion, I meant we remove `onHighlightingsReady`, not the `override` keyword. ================ Comment at: clang-tools-extra/clangd/unittests/ClangdTests.cpp:864 + ClangdServer Server(MCD, FS, DiagConsumer, ClangdServer::optsForTest()); + Server.addDocument(FooCpp, "int a;"); + ASSERT_EQ(DiagConsumer.Count, 1); ---------------- this is not safe, we need to wait until the server finishes building AST. add `ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for server"; ` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63821/new/ https://reviews.llvm.org/D63821 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
