[PATCH] D77645: [clangd] Support dexp -c "some command"

Kirill Bobyrev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 8 03:12:45 PDT 2020


kbobyrev accepted this revision.
kbobyrev added a comment.
This revision is now accepted and ready to land.

LGTM with a couple of nits



================
Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:85
       llvm::cl::ValueDisallowed, llvm::cl::cat(llvm::cl::GeneralCategory)};
+  // FIXME: allow commands to signal failure.
   virtual void run() = 0;
----------------
Nit: capitalization


================
Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:100
+    // must do this before opts are destroyed
+    auto Cleanup = llvm::make_scope_exit(llvm::cl::ResetCommandLineParser);
     if (Help.getNumOccurrences() > 0) {
----------------
nit: const?


================
Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:328
+  if (!ExecCommand.empty())
+    return runCommand(ExecCommand, *Index) ? 0 : 1;
 
----------------
`return !runCommand(...)`?


================
Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:333
+    runCommand(std::move(*Request), *Index);
   return 0;
 }
----------------
Maybe also just delete this one since there's some new code anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77645/new/

https://reviews.llvm.org/D77645





More information about the cfe-commits mailing list