[PATCH] clang-apply-replacements: Add code-formatting post processing step

[Guillaume Papin]

  Some minor comment but looks good to me.
  For the changed range tests what have you planned? Will they stay in clang-modernize?


================
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:328-330
@@ +327,5 @@
+
+void reformatRanges(const llvm::StringRef FileName, const RangeVec &Ranges,
+                    clang::SourceManager &SM, clang::format::FormatStyle Style,
+                    std::vector<tooling::Replacement> &Replacements) {
+  const clang::FileEntry *Entry = SM.getFileManager().getFile(FileName);
----------------
Edwin Vane wrote:
> Guillaume Papin wrote:
> > nit: I think we can get rid of the extra qualifications `clang::` and `llvm::`.
> Actually, there's this bug in doxygen that causes it not to be able to match a function definition with it's implementation if we don't fully spell out the namespaces even though there's a using declaration.  You should find that only the function prototypes use unnecessary namespaces. Other code, or file local functions should not have unnecessary names.
I'm aware of this issue but I thought it applied only to overloaded functions where Doxygen was confused in which one to pick-up. I won't point this in the future.

================
Comment at: clang-apply-replacements/include/clang-apply-replacements/Tooling/ApplyReplacements.h:127
@@ +126,3 @@
+void reformatRanges(const StringRef FileName, const RangeVec &Ranges,
+                    clang::SourceManager &SM, clang::format::FormatStyle Style,
+                    std::vector<clang::tooling::Replacement> &Replacements);
----------------
`Style` should be const-ref.

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:165-170
@@ -60,1 +164,8 @@
 
+  // Handle the -format-style option if present.
+  format::FormatStyle FormatStyle;
+  int FormatOptResult = handleFormatStyle(argv[0], FormatStyle, Diagnostics);
+  if (FormatOptResult < 0)
+    return 1;
+  bool DoFormat = FormatOptResult > 0;
+
----------------
What about checking the option here and send the option value as argument to `handleFormatStyle()`?
Return false on error as in the previous version.

```
if (FormatStyleOpt.getNumOccurrences() > 0) {
    if (!handleFormatStyle(argv[0], FormatStyleOpt, FormatStyle, Diagnostics))
      return 1;
    DoFormat = true;
}
```

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:40
@@ +39,3 @@
+    cl::desc(
+        "Enable formatting of code code changed by applying replacements.\n"
+        "<style> can be either a builtin style supported by clang-format,\n"
----------------
code code

================
Comment at: clang-apply-replacements/tool/ClangApplyReplacementsMain.cpp:66-68
@@ +65,5 @@
+
+  std::string OptValue = FormatStyleOpt;
+  if (OptValue.empty())
+    OptValue = "llvm";
+
----------------
No big deal but I think a StringRef is enough here, in both case the lifetime of the string assigned will outlive this variable.


http://llvm-reviews.chandlerc.com/D1730