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

[Daniel Jasper via cfe-commits]

djasper added inline comments.

================
Comment at: include/clang/Tooling/Core/Replacement.h:237
@@ +236,3 @@
+/// related to the same file entry are put into the same vector.
+FileToReplacementsMap groupReplacementsByFile(const Replacements &Replaces,
+                                              FileManager &Files);
----------------
Hm... I am not sure here. I think I would implement this entirely without FileManager or FileEntries, just based on the names of the file. I guess you are worried about different paths leading to the same FileEntry?

================
Comment at: include/clang/Tooling/ReplacementsFormat.h:1
@@ +1,2 @@
+//===-- ReplacementsFormat.h -- Format code changed by replacements -*- C++ -*//
+//
----------------
I think this can happily live in Refactoring.h/cpp for now.

================
Comment at: include/clang/Tooling/ReplacementsFormat.h:40
@@ +39,3 @@
+                                   Rewriter &Rewrite,
+                                   const format::FormatStyle &Style);
+
----------------
This is probably a pre-existing issue in formatReplacements, but I think conceptually it might be wrong to hand in the Style. I think it might be a better idea to use clang::format::getStyle() with the filename stored inside of the replacement to get the style that should actually be used for any given change. Not sure whether we'd also want the capability to overwrite this. Manuel, what do you think?


http://reviews.llvm.org/D17852