[PATCH] D17852: Added formatAndApplyAllReplacements that works on multiple files in libTooling.

Manuel Klimek via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 21 07:01:30 PDT 2016


klimek added inline comments.

================
Comment at: lib/Tooling/Refactoring.cpp:90
@@ +89,3 @@
+
+// FIXME: duplicated code here. Any better way to overload?
+bool formatAndApplyAllReplacements(const Replacements &Replaces,
----------------
ioeric wrote:
> klimek wrote:
> > Just call the above with the format::getStyle("file", FilePath, "LLVM")?
> > (note that I think "Google" is a bad default style here)
> But there could be more than one FilePath in Replaces? 
Ah, right; In that case, we have 2 options:
1. hand in a callback FormatStyle(StringRef), so users can hand in format::getStyle if needed; adapt the fixed-style one to call the callback-based one with a callback that just returns the fixed style
2. refactor so the inner loop becomes trivial, something like this:
  for (auto &ReplacementsByFile : groupReplacementsByFile(Replaces)) {
    Style = ...
    Result = applyAllReplacements(formatReplacements(getCode(ReplacementsByFile.first, ReplacementsByFile.second, Style))) && Result;
  }
  return Result;


http://reviews.llvm.org/D17852





More information about the cfe-commits mailing list