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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
