[PATCH] D55363: [clangd] Expose FileStatus in LSP.
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 19 07:42:27 PST 2018
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
More information about the cfe-commits
mailing list