[PATCH] D83095: [clangd] Config: compute config in TUScheduler and BackgroundIndex

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:748
 
+Context ClangdServer::createProcessingContext(PathRef File) const {
+  if (!ConfigProvider)
----------------
kadircet wrote:
> why not make this a free function in `ConfigProvider.h` ?
> 
> that way we could just keep passing ConfigProvider around rather than a derived lambda.
> It makes testing more cumbersome, but enables us to move the logic out of ClangdServer
Oh, I thought hiding this logic in ClangdServer was a feature!

For instance, if we want to report configuration errors as LSP diagnostics or as notifications, we need to have access to the ClangdServer to do that.


================
Comment at: clang-tools-extra/clangd/index/Background.h:142
+      std::function<void(BackgroundQueue::Stats)> OnProgress = nullptr,
+      std::function<Context(PathRef)> ContextProvider = nullptr);
   ~BackgroundIndex(); // Blocks while the current task finishes.
----------------
kadircet wrote:
> doesn't need to be in this patch, bu I think it is time we have an opts struct in here.
Yeah, makes sense. (But indeed would rather not do it here...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83095