Some high-level comments. I think we should first figure out the general direction. Happy to chat, too.
================ Comment at: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:38 @@ +37,3 @@ +/// \brief Collection of source ranges. +typedef std::vector<clang::tooling::Range> RangeVec; + ---------------- Not a fan of abbreviations. Fan of "tor" :-). ================ Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:289 @@ +288,3 @@ + +void calculateChangedRanges( + const std::vector<clang::tooling::Replacement> &Replaces, ---------------- I think a lot of the complexity stems from this function accepting non-empty ChangedRanges. I'd prefer to simply impose a restriction until this functionality is actually needed. Or do you already have a use case? ================ Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:117 @@ +116,3 @@ +/// code. +static void formatReplacements(const Rewriter &AppliedReplacements, + const FileToReplacementsMap &GroupedReplacements, ---------------- I think this interface is too complex and has too many assumptions. I think it would get significantly easier if it operated on a single file. Does anything speak against moving the "if (DoFormat)" below into the loop. Also, while you can of course try to reuse rewriters ans SourceManagers, I think this makes not only the interface more complex, but also limits the possibilities for executing this in multiple threads. Why not just hand in the applied replacements and a string with the code? http://llvm-reviews.chandlerc.com/D1730 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
