Re: [PATCH 0/15] making user-format colors conditional on config/tty
Jeff King writes: > This version also takes into account feedback from the original thread. > And as I added tests, it surfaced a few corner cases around color config > that I've dealt with here. The last two patches are the most > interesting bits. > > [01/15]: check return value of verify_ref_format() > [02/15]: docs/for-each-ref: update pointer to color syntax > [03/15]: t: use test_decode_color rather than literal ANSI codes > [04/15]: ref-filter: simplify automatic color reset > [05/15]: ref-filter: abstract ref format into its own struct > [06/15]: ref-filter: move need_color_reset_at_eol into ref_format > [07/15]: ref-filter: provide a function for parsing sort options > [08/15]: ref-filter: make parse_ref_filter_atom a private function > [09/15]: ref-filter: factor out the parsing of sorting atoms > [10/15]: ref-filter: pass ref_format struct to atom parsers > [11/15]: color: check color.ui in git_default_config() > [12/15]: for-each-ref: load config earlier > [13/15]: rev-list: pass diffopt->use_colors through to pretty-print > [14/15]: pretty: respect color settings for %C placeholders > [15/15]: ref-filter: consult want_color() before emitting colors Overall I think the endgame is what we want to have in the future (rather, what I wish we had from the beginning). I'd have to think about 11, 14 and 15 a bit more before saying anything that would be remotely useful. Thanks. > > Documentation/git-for-each-ref.txt | 6 +- > Documentation/pretty-formats.txt | 18 -- > builtin/branch.c | 21 +++--- > builtin/clean.c| 3 +- > builtin/for-each-ref.c | 27 > builtin/grep.c | 2 +- > builtin/rev-list.c | 1 + > builtin/show-branch.c | 2 +- > builtin/tag.c | 61 ++ > builtin/verify-tag.c | 14 ++-- > color.c| 8 --- > config.c | 4 ++ > diff.c | 3 - > pretty.c | 27 ++-- > ref-filter.c | 108 ++- > ref-filter.h | 30 +++-- > t/t3203-branch-output.sh | 31 + > t/t4207-log-decoration-colors.sh | 22 +++ > t/t6006-rev-list-format.sh | 129 > - > t/t6300-for-each-ref.sh| 39 +++ > t/t7004-tag.sh | 25 +++ > t/test-lib-functions.sh| 1 + > 22 files changed, 362 insertions(+), 220 deletions(-) > > -Peff
Re: [PATCH 0/15] making user-format colors conditional on config/tty
On Thu, Jul 13, 2017 at 7:55 AM, Jeff King wrote: > This is a cleanup of the patch I posted last October: > > > https://public-inbox.org/git/20161010151517.6wszhuyp57yfn...@sigill.intra.peff.net/ > > The general idea is that it's rather confusing that "%C(red)" in a > pretty-print format does not currently respect color.ui, --no-color, or > the usual isatty check on stdout. This series changes that. Note that > this is a backwards-incompatible change, but the general sentiment in > that earlier thread seemed to be that the existing behavior is arguably > buggy. See patch 14 for more discussion. > > The patch stalled back then because I wanted to make sure that > ref-filter's color placeholders behaved the same. That required some > refactoring which conflicted badly with kn/ref-filter-branch-list. Now > that it has graduated, I was able to rebase on top. > > This version also takes into account feedback from the original thread. > And as I added tests, it surfaced a few corner cases around color config > that I've dealt with here. The last two patches are the most > interesting bits. > I have reviewed these slightly and think this series is a good change. Thanks, Stefan
