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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits