[PATCH] D98029: [clangd] Introduce a CommandLineConfigProvider

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555
+    };
+    if (Sync) {
+      IndexLoadTask();
----------------
sammccall wrote:
> (This seems a little weird, get passed in a task runner and maybe ignore it. Seems like the asyncprojectindex should know whether it's supposed to be sync or not, and either pass in a null runner or it should handle the asynchrony itself... not something for this patch)
adding a fixme, will follow-up.


================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:756
+        CompileCommandsDir = Path.str().str();
+        // FIXME: Still set for clangd --check. Use config in --check workflow
+        // and get rid of these options?
----------------
sammccall wrote:
> If we fix this, we can move the rest of the logic into the flags config provider constructor.
yes, i was unsure about whether we would like to propagate config into --check mode, (but you didn't oppose the fixme, so doing that now :D)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98029