sammccall added inline comments.
================
Comment at: clangd/ClangdLSPServer.cpp:787
+ notify("window/showMessage",
+ json::Object {
+ {"uri", URI},
----------------
we should have a protocol model object (in Protocol.h) for the file status
updates.
I think it should *likely* it should be distinct from the FileStatus in
TUScheduler - optimizing for easy update/comparison and programmatic checks vs
optimizing for matching the desired protocol.
Naming is hard, I'd be tempted to take `FileStatus` for the protocol object and
and make the TUScheduler one something more obscure like `TUScheduler::TUState`.
Unclear to me which one ClangdServer should expose, my *guess* is we should
start by exposing the protocol object, and switch if it doesn't capture enough
of the semantics that embedders need.
================
Comment at: clangd/TUScheduler.cpp:338
auto Task = [=](decltype(OnUpdated) OnUpdated) mutable {
+ OnUpdated(FileStatus{FileStatus::State::Building, "Building"}, {});
// Will be used to check if we can avoid rebuilding the AST.
----------------
So I'm slightly nervous about just emitting objects directly. The problem is if
you have independent properties (e.g. "is using a fallback command" vs "are we
computing diagnostics"), then it's a burden on the code here to pass them all
every time and a burden on the callback code if you don't.
I think one solution (which *may* be what Ilya's suggesting) is to put a
mutable FileStatus object in the ASTWorker, and mutate it in place and then
invoke the callback, passing the `const FileStatus&`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54796
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
