[PATCH] D83817: [clangd] Add option to use remote index as static index

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:457
+    "remote-index-address",
+    cat(Remote),
+    desc("Address of the remote index server"),
----------------
not sure whether we need a new category, probably can live in Features?


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:459
+    desc("Address of the remote index server"),
+    Hidden,
+};
----------------
since we hide this in the `CLANGD_ENABLE_REMOTE` flag, maybe remove the `Hidden`?


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:463
+// FIXME(kirillbobyrev): Should this be the location of compile_commands.json?
+opt<std::string> ProjectPath{
+    "project-path",
----------------
maybe `ProjectRoot`?


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:702
   }
+  if (RemoteIndexAddress.empty() != ProjectPath.empty()) {
+    llvm::errs() << "remote-index-address and project-path have to be "
----------------
the new code section here should be guarded under `#ifdef CLANGD_ENABLE_REMOTE`


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:707
+  }
+  if (!RemoteIndexAddress.empty()) {
+    if (!IndexFile.empty()) {
----------------
if remote-index is on, should we turn off the background index? I think this is our plan, right?


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:709
+    if (!IndexFile.empty()) {
+      llvm::errs() << "When remote index is enabled, IndexFile should not be "
+                      "specified. Only one can be used at time.";
----------------
nit: use elog.


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:711
+                      "specified. Only one can be used at time.";
+      return 1;
+    }
----------------
instead of exiting clangd, maybe just continue (since this is not a fatal error), and adjust the error message stating that index-file option is ignored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83817