sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
LG, thanks!
I do think it can be further simplified, but if not then land as-is.
================
Comment at: clang-tools-extra/clangd/ClangdServer.cpp:57
public:
+ RefactoringResultCollector(DiagnosticsEngine &DE) : Diags(DE) {}
void handleError(llvm::Error Err) override {
----------------
hokein wrote:
> sammccall wrote:
> > why inject the DE here (and handle mapping errors both in the caller and
> > here) rather than always doing the mapping in the caller?
> We don't have control of the Err caller for this class (the Error is passed
> via the `handleError` interface)...so we need the DE in this class to do the
> expanding.
But we just store the err in Result, and then ClangdServer::rename does
```
if (!ResultCollector.Result.getValue())
return CB(ResultCollector.Result->takeError());
```
can it wrap the takeError() in expandDiagnostics()?
it still has access to the diagnosticsengine I think
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60821/new/
https://reviews.llvm.org/D60821
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
