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