[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

This revision was automatically updated to reflect the committed changes. Closed by commit rGd9b8aada8288: [clang-tidy] Add --use-color command line option and UseColor option to control… (authored by hyd-dev, committed by njames93). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. I don't have commit access. @njames93 Could you please commit it for me (`hyd-dev `)? Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 ___ cfe-commits mailing list

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 ___ cfe-commits mailing list

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. **Ping?** CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. **Ping?** CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. Fix typos in the summary. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev marked 2 inline comments as done. hyd-dev added a comment. Any reply? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. In D79477#2028637 , @njames93 wrote: > What I mean to say is, if the behaviour of the testing environment changes to > pipe the result to a terminal that supports color, it could cause this test > case to also fail. The

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

njames93 added a comment. In D79477#2028618 , @hyd-dev wrote: > > Not a fan of this test case as it only demonstrates the color behaviour of > > the process running the check not the actual option itself > > What does "option itself" mean? What I mean

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. > Would a test case be needed? `clang::tidy::clangTidyMain()` shows that clang-tidy reports all diagnostics by `clang::tidy::handleErrors()`, which constructs a `clang::tidy::(anonymous namespace)::ErrorReporter`, no matter where they come from, so I think a test

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

njames93 added a comment. In D79477#2028609 , @hyd-dev wrote: > This option sets `DiagOpts->ShowColors` to `true`. As I known, it controls > **all** diagnostics reported by the `clang-tidy` program (by > `clang::tidy::(anonymous

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. > Fair point, will this option also control the color of the diagnostics > emitted by clang or just clang tidy specific diagnostics? This option sets `DiagOpts->ShowColors` to `true`. As I known, it controls **all** diagnostics reported by the `clang-tidy` program (by

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

njames93 added a comment. In D79477#2027978 , @hyd-dev wrote: > None of the clang-tidy command line options are prefixed with `-f`. > This command line option used to be `--color-diagnostics`, but I've followed > @hokein's advice to change it to

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added a comment. None of the clang-tidy command line options are prefixed with `-f`. This command line option used to be `--color-diagnostics`, but I've followed @hokein's advice to change it to `--use-color`. If you mean to use the `-fcolor-diagnostics` option from

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

njames93 added a comment. Could this not use the `-fcolor-diagnostics` and `fno-color-diagnostics` command line flags that clang uses for its diagnostics to keep everything the same? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev updated this revision to Diff 262887. hyd-dev marked an inline comment as done. hyd-dev added a comment. Change incorrect `detect` to `detected` in the comment of `UseColor` option. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/ https://reviews.llvm.org/D79477 Files:

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.h:130 + + /// Use colors in diagnostics. If missing, colors will be used if standard + /// output is connected to a terminal. hokein wrote: > nit: ... wil be used if

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev updated this revision to Diff 262817. hyd-dev marked an inline comment as done. hyd-dev added a comment. Say the `--use-color` command line option overrides the `UseColor` option in `.clang-tidy` file in its help text. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79477/new/

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:99 IO.mapOptional("InheritParentConfig", Options.InheritParentConfig); +IO.mapOptional("UseColor", Options.UseColor); } njames93 wrote: > I'm not entirely

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:99 IO.mapOptional("InheritParentConfig", Options.InheritParentConfig); +IO.mapOptional("UseColor", Options.UseColor); } I'm not entirely sure this option

[PATCH] D79477: [clang-tidy] Add --use-color command line option and UseColor option to control colors in diagnostics

hyd-dev updated this revision to Diff 262541. hyd-dev retitled this revision from "[clang-tidy] Add --color-diagnostics command line option and ColorDiagnostics option to control colors in diagnostics" to "[clang-tidy] Add --use-color command line option and UseColor option to control colors in