clang-format: add an option -verbose to list the files being processed

[djasper]

Can you add a test? Tests for this kind of behavior of the clang-format command line go in test/Format/.

tools/clang-format/ClangFormat.cpp

380

As clang-format often returns the result on stdout, maybe it'd be better to use errs() here?

[sylvestre.ledru]

tools/clang-format/ClangFormat.cpp

380

Right, otherwise, this:
clang-format -verbose foo.cpp > bar.cpp
won't work

[sylvestre.ledru]

is that ok @djasper ?

[djasper]

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

test/Format/verbose.cpp

2 ↗

(On Diff #104995)

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.

tools/clang-format/ClangFormat.cpp

377

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;

388

Feel free to turn this into a range-based for loop while you are here. :)

390

Not sure that just checking the number of occurrences is correct. What happens if I write -verbose=false on the command line?

391

The indentation is off.

[sylvestre.ledru]

tools/clang-format/ClangFormat.cpp

377

This isn't correct.
You can have

bin/clang-format  -lines=1:1 foo-1.cpp foo-2.cpp

we won't enter into the "error: -offset" error

[sylvestre.ledru]

test/Format/verbose.cpp

2 ↗

(On Diff #104995)

I changed the structure and added more checks to it.

[djasper]

tools/clang-format/ClangFormat.cpp

377

Right. In my code I had a bug s/== 1/ != 1/. But I don't think you can drop that part entirely. As it is now, you cannot specify any -lines/offsets/lenghts even for a single file?

[sylvestre.ledru]

Fixed, thanks for spotting the mistake.
Looks like we could improve the testsuite as my mistake hasn't been catched.

[djasper]

Yeah, improving the testsuite generally seems like a good idea. But unrelated to this patch. This looks good now.

[sylvestre.ledru]

actually, arc commit never landed this change :(

docs/ClangFormat.rst

-style=<string>           - Coding style, currently supports:
                                   LLVM, Google, Chromium, Mozilla, WebKit.
                                 Use -style=file to load style configuration from
                                 .clang-format file located in one of the parent
                                 directories of the source file (or current
                                 directory for stdin).
                                 Use -style="{key: value, ...}" to set specific
                                 parameters, e.g.:
                                   -style="{BasedOnStyle: llvm, IndentWidth: 8}"
+    -verbose                  - If set, shows the list of processed files
 
   Generic Options:
 
     -help                     - Display available options (-help-hidden for more)
     -help-list                - Display list of available options (-help-list-hidden for more)
     -version                  - Display the version of this program
 
 

docs/ReleaseNotes.rst

* Namespace end comments are now added or updated automatically.
   |   int i;            |   int i;            |
   |   int j;            |   int j;            |
   |   }                 |   }                 |
   +---------------------+---------------------+
 
 * Comment reflow support added. Overly long comment lines will now be reflown with the rest of
   the paragraph instead of just broken. Option **ReflowComments** added and enabled by default.
 
+* Option -verbose added to the command line.
+  Shows the list of processed files.
+
 libclang
 --------
 
 ...
 
 
 Static Analyzer
 ---------------

tools/clang-format/ClangFormat.cpp

Cursor("cursor",
            cl::init(0), cl::cat(ClangFormatCategory));
 
 static cl::opt<bool> SortIncludes(
     "sort-includes",
     cl::desc("If set, overrides the include sorting behavior determined by the "
              "SortIncludes style flag"),
     cl::cat(ClangFormatCategory));
 
+static cl::opt<bool> Verbose(
+    "verbose",
+    cl::desc("If set, shows the list of processed files"),
+    cl::cat(ClangFormatCategory));
+
 static cl::list<std::string> FileNames(cl::Positional, cl::desc("[<file> ...]"),
                                        cl::cat(ClangFormatCategory));
 
 namespace clang {
 namespace format {
 
 static FileID createInMemoryFile(StringRef FileName, MemoryBuffer *Source,
                                  SourceManager &Sources, FileManager &Files,
if (DumpConfig) {
     return 0;
   }
 
   bool Error = false;
   switch (FileNames.size()) {
   case 0:
     Error = clang::format::format("-");
     break;
   case 1:
     Error = clang::format::format(FileNames[0]);
+    if (Verbose.getNumOccurrences() != 0)
+        outs() << "Formatting " << FileNames[0] << '\n';
     break;
   default:
     if (!Offsets.empty() || !Lengths.empty() || !LineRanges.empty()) {
       errs() << "error: -offset, -length and -lines can only be used for "
                 "single file.\n";
       return 1;
     }
-    for (unsigned i = 0; i < FileNames.size(); ++i)
+    for (unsigned i = 0; i < FileNames.size(); ++i) {
       Error |= clang::format::format(FileNames[i]);
+      if (Verbose.getNumOccurrences() != 0)
+          outs() << "Formatting " << FileNames[i] << '\n';
+    }
     break;
   }
   return Error ? 1 : 0;
 }