[PATCH] D34824: clang-format: add an option -verbose to list the files being processed

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Generally upload diffs with full context to phabricator. That makes reviewing much easier.



================
Comment at: test/Format/verbose.cpp:2
+// RUN: clang-format %s -verbose | FileCheck %s
+// CHECK: Formatting
+
----------------
This seems like a pretty minimal test to me. Consider adding the filename (with a regex like "Formatting{{.*}}verbose.cpp"). There might be reasons not to do that though.


================
Comment at: tools/clang-format/ClangFormat.cpp:377
     break;
   case 1:
     Error = clang::format::format(FileNames[0]);
----------------
I think we should restructure the code to not have to duplicate what you are adding here. I think fundamentally, we should be able to change this to:

  if (FileNames.empty()) {
    Error = clang::format::format("-");
    return Error ? 1 : 0;
  }
  if (FileNames.size() == 1 && (!Offsets.empty() || !Lengths.empty() || !LineRanges.empty())) {
    errs() << "error: -offset .... "
    return 1;
  }
  for (const auto& FileName : FileNames) {
    if (Verbose.getNumOccurences() != 0)
      errs() << "Formatting " << Filename << "\n";
    Error |= clang::format::format(FileName);
  }
  return Error ? 1 : 0;


================
Comment at: tools/clang-format/ClangFormat.cpp:388
     }
-    for (unsigned i = 0; i < FileNames.size(); ++i)
+    for (unsigned i = 0; i < FileNames.size(); ++i) {
       Error |= clang::format::format(FileNames[i]);
----------------
Feel free to turn this into a range-based for loop while you are here. :)


================
Comment at: tools/clang-format/ClangFormat.cpp:390
       Error |= clang::format::format(FileNames[i]);
+      if (Verbose.getNumOccurrences() != 0)
+          errs() << "Formatting " << FileNames[i] << '\n';
----------------
Not sure that just checking the number of occurrences is correct. What happens if I write -verbose=false on the command line?


================
Comment at: tools/clang-format/ClangFormat.cpp:391
+      if (Verbose.getNumOccurrences() != 0)
+          errs() << "Formatting " << FileNames[i] << '\n';
+    }
----------------
The indentation is off.


https://reviews.llvm.org/D34824