ilya-biryukov added a comment.
Dexplorer is a long name. How about shortening it to `dexp`?
================
Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:56
+
+template <class Function, class TimeUnit = std::chrono::milliseconds>
+void reportTime(StringRef Name, Function F) {
----------------
Why do we want `TimeUnit`?
It adds complexity, if we want to make large values more readable we have other
alternatives:
- printing adaptive units, e.g. "when it's larger than 5000ms, start printing
seconds", "when it's larger than 5minutes, start printing minutes"
- provide separators (`500000000ms` is pretty much unreadable, `500 000 000ms`
is much better)
================
Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:125
+// * print out tokens with least dense posting lists
+void dispatchRequest(const std::unique_ptr<SymbolIndex> &Index,
+ StringRef Request) {
----------------
This function does too many things:
- parses the input.
- dispatches on different kinds of requests.
- handles user input errors.
- actually runs the commands.
This turns out somewhat unreadable at the end, we might want to separate those
into multiple functions. Will need a bit of time to think on how to actually do
it best. Will come back after I have concrete suggestions.
================
Comment at: clang-tools-extra/clangd/index/dex/dexplorer/DExplorer.cpp:139
+ "symbol name.\n";
+ helpRequest();
+ return;
----------------
This produces a ton of output.
Could we simply produce a message to run 'help' to get a list of supported
commands instead?
It could be really annoying to see large help messages on every typo.
https://reviews.llvm.org/D51628
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
