[PATCH] D91103: [tooling] Add support for fixits that indicate code will need reformatting

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:636
+    // Filter out any reformat fixits, we don't handle these.
+    // FIXME: Can we?
+    llvm::erase_if(FixIts,
----------------
kadircet wrote:
> in theory yes, as we have access to source manager, we can fetch file contents and create formatted  replacements (see `cleanupAndFormat`). but formatting those fixes can imply significant delays on clangd's diagnostic cycles (if there are many of those), that's the reason why we currently don't format fixits.
I mean if clangd is extended to support async code actions for diagnostics instead of just using the inline extension. Having said that, if that were the case, this would probably not be where that support is implemented.


================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:637
+    // FIXME: Can we?
+    llvm::erase_if(FixIts,
+                   [](const FixItHint &Fix) { return Fix.isReformatFixit(); });
----------------
kadircet wrote:
> rather than doing an extra loop, can we just skip those in the for loop below ?
We could, however that would result in messier code further down when trying to build a synthetic message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91103