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