[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