ilya-biryukov added a comment.
Could we add a capability to the init request to check for this extension? We
don't want to send those notifications to clients that don't have the code to
handle them.
Another observation: when I played around with the previous version of the
patch, `RunningAction` and `BuildingAST` where constantly "blinking", i.e. it
changes faster than I can read it.
This lead to them being irritating and not providing any actual value.
Maybe consider sending an update after some period of time, e.g. `0.5s`? (I
expect this particular strategy to be a bit complicated and out of scope of
this patch, so another alternative is to simply not send them to the clients in
the first version). WDYT?
================
Comment at: clangd/ClangdLSPServer.cpp:806
+void ClangdLSPServer::onFileUpdated(PathRef File, const TUStatus &Status) {
+ notify("textDocument/clangd.fileStatus", Status.render(File));
+}
----------------
It's somewhat easy to miss the `clangd` in the middle, would personally prefer
the `clangd.textDocument/fileStatus`. But up to you.
================
Comment at: clangd/Protocol.h:999
+ /// Details of the state that are worth sufacing to users.
+ std::vector<ShowMessageParams> details;
+};
----------------
Why not `vector<string>`? What's the use of the `MessageType`?
================
Comment at: clangd/TUScheduler.cpp:741
+ case TUAction::Queued:
+ OS << "Queued";
+ break;
----------------
Maybe start with a lower case?
Arguably `clangd: queued` looks more natural than `clangd: Queued`
Also consider maybe change this to `file is queued` to make it clear it's not
clangd that's queuing up somewhere.
================
Comment at: clangd/TUScheduler.cpp:744
+ case TUAction::RunningAction:
+ OS << "RunningAction"
+ << "(" << Action.Name << ")";
----------------
Maybe make it `running ${action name}`, e.g. `running find references`?
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55363/new/
https://reviews.llvm.org/D55363
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
