[PATCH] D41720: [clang-tidy] Add a -show-color flag.

[Ian Tessier via Phabricator via cfe-commits]

itessier added inline comments.


================
Comment at: clang-tidy/ClangTidyOptions.h:93
+  /// \brief Show color diagnostics.
+  llvm::Optional<bool> ShowColor;
+
----------------
alexfh wrote:
> This doesn't belong to ClangTidyOptions. It's specific to the CLI, but CLI is not the only frontend for clang-tidy.
Since we have to propagate the value to the ErrorReporter, how about adding a bool param to the ErrorReporter ctor? We could add a setter instead, but that would require moving a diag printer call out of the ctor since it uses the DiagOpts instance.

The colour logic would then be moved into either clangTidyMain or handleErrors.


================
Comment at: clang-tidy/tool/ClangTidyMain.cpp:150-152
+Show color diagnostics. If not specified,
+defaults to on when a color-capable terminal
+is detected.)"),
----------------
itessier wrote:
> aaron.ballman wrote:
> > I think this raw string can be reflowed a bit?
> > 
> > Instead of "defaults to on", how about "Defaults to true when a color-capable terminal is detected."?
> I copied that part from the -fcolor-diagnostics flag to be consistent. I don't changing "on" to "true" if you prefer that instead.
That should have read "I don't mind changing..."


https://reviews.llvm.org/D41720