[PATCH] D30777: Added `applyAtomicChanges` function.

[Daniel Jasper via Phabricator via cfe-commits]

djasper added inline comments.


================
Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:139
+  // kNone: Don't format anything.
+  // kViolations: Format lines exceeding 80 columns.
+  enum FormatOption { kAll, kNone, kViolations };
----------------
Should probably be "exceeding the column limit". 


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:91
+// [Start, End].
+bool ViolatesColumnLimit(llvm::StringRef Code, unsigned ColumnLimit,
+                         unsigned Start, unsigned End) {
----------------
I think LLVM style is lowerCamelCase now, right? Here and below.


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:113
+                      const clang::tooling::Replacements &Replaces) {
+  std::vector<clang::tooling::Range> Ranges;
+  // kNone suppresses formatting entirely.
----------------
nit: I'd move this down and just return {} for kNone.


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:127
+        R.getOffset() > 0 && R.getOffset() <= Code.size() &&
+        Code[R.getOffset() - 1] == '\n') {
+      // If we are inserting at the start of a line and the replacement ends in
----------------
Remove braces?


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:151
+                             const format::FormatStyle &Style) {
+  llvm::SmallVector<llvm::StringRef, 4> InsertedHeaders;
+  llvm::SmallVector<llvm::StringRef, 4> RemovedHeaders;
----------------
What is copying these vectors buying us?


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:164
+    std::string EscapedHeader =
+        (Header.startswith("<") || Header.startswith("\""))
+            ? Header.str()
----------------
I'd remove the parentheses.


================
Comment at: lib/Tooling/Refactoring/AtomicChange.cpp:196
+  Replacements Replaces;
+  for (const auto &Change : Changes) {
+    for (const auto &R : Change.getReplacements()) {
----------------
Remove the braces of the for loops.


https://reviews.llvm.org/D30777