ref-filter: consult want_color() before emitting colors
When color placeholders like %(color:red) are used in a ref-filter format, we unconditionally output the colors, even if the user has asked us for no colors. This usually isn't a problem when the user is constructing a --format on the command line, but it means we may do the wrong thing when the format is fed from a script or alias. For example: $ git config alias.b 'branch --format=%(color:green)%(refname)' $ git b --no-color should probably omit the green color. Likewise, running: $ git b >branches should probably also omit the color, just as we would for all baked-in coloring (and as we recently started to do for user-specified colors in --pretty formats). This commit makes both of those cases work by teaching the ref-filter code to consult want_color() before outputting any color. The color flag in ref_format defaults to "-1", which means we'll consult color.ui, which in turn defaults to the usual isatty() check on stdout. However, callers like git-branch which support their own color config (and command-line options) can override that. The new tests independently cover all three of the callers of ref-filter (for-each-ref, tag, and branch). Even though these seem redundant, it confirms that we've correctly plumbed through all of the necessary config to make colors work by default. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Jul 13, 2017 at 11:09 UTC
11b087adfd469ca597f1d269314f8cad32d0d72f
6 files changed
+93
-12
builtin/branch.c
+1
@@ -409,6 +409,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
409
410
if (!format->format)
411
format->format = to_free = build_format(filter, maxwidth, remote_prefix);
412
+ format->use_color = branch_use_color;
413
414
if (verify_ref_format(format))
415
die(_("unable to parse format string"));
ref-filter.c
+8
@@ -104,6 +104,12 @@ static void color_atom_parser(const struct ref_format *format, struct used_atom
104
die(_("expected format: %%(color:<color>)"));
105
if (color_parse(color_value, atom->u.color) < 0)
106
die(_("unrecognized color: %%(color:%s)"), color_value);
107
+ /*
108
+ * We check this after we've parsed the color, which lets us complain
109
+ * about syntactically bogus color names even if they won't be used.
110
+ */
111
+ if (!want_color(format->use_color))
112
+ color_parse("", atom->u.color);
113
}
114
115
static void refname_atom_parser_internal(struct refname_atom *atom,
@@ -675,6 +681,8 @@ int verify_ref_format(struct ref_format *format)
681
if (skip_prefix(used_atom[at].name, "color:", &color))
682
format->need_color_reset_at_eol = !!strcmp(color, "reset");
683
}
684
+ if (format->need_color_reset_at_eol && !want_color(format->use_color))
685
+ format->need_color_reset_at_eol = 0;
686
return 0;
687
}
688
ref-filter.h
+2
-1
@@ -79,12 +79,13 @@ struct ref_format {
79
*/
80
const char *format;
81
int quote_style;
82
+ int use_color;
83
84
/* Internal state to ref-filter */
85
int need_color_reset_at_eol;
86
};
87
87
-#define REF_FORMAT_INIT { NULL, 0 }
88
+#define REF_FORMAT_INIT { NULL, 0, -1 }
89
90
/* Macros for checking --merged and --no-merged options */
91
#define _OPT_MERGED_NO_MERGED(option, filter, h) \
t/t3203-branch-output.sh
+31
@@ -2,6 +2,7 @@
2
3
test_description='git branch display tests'
4
. ./test-lib.sh
5
+. "$TEST_DIRECTORY"/lib-terminal.sh
6
7
test_expect_success 'make commits' '
8
echo content >file &&
@@ -239,4 +240,34 @@ test_expect_success 'git branch --format option' '
240
test_i18ncmp expect actual
241
'
242
243
+test_expect_success "set up color tests" '
244
+ echo "<RED>master<RESET>" >expect.color &&
245
+ echo "master" >expect.bare &&
246
+ color_args="--format=%(color:red)%(refname:short) --list master"
247
+'
248
+
249
+test_expect_success '%(color) omitted without tty' '
250
+ TERM=vt100 git branch $color_args >actual.raw &&
251
+ test_decode_color <actual.raw >actual &&
252
+ test_cmp expect.bare actual
253
+'
254
+
255
+test_expect_success TTY '%(color) present with tty' '
256
+ test_terminal env TERM=vt100 git branch $color_args >actual.raw &&
257
+ test_decode_color <actual.raw >actual &&
258
+ test_cmp expect.color actual
259
+'
260
+
261
+test_expect_success 'color.branch=always overrides auto-color' '
262
+ git -c color.branch=always branch $color_args >actual.raw &&
263
+ test_decode_color <actual.raw >actual &&
264
+ test_cmp expect.color actual
265
+'
266
+
267
+test_expect_success '--color overrides auto-color' '
268
+ git branch --color $color_args >actual.raw &&
269
+ test_decode_color <actual.raw >actual &&
270
+ test_cmp expect.color actual
271
+'
272
+
273
test_done
t/t6300-for-each-ref.sh
+26
-11
@@ -7,6 +7,7 @@ test_description='for-each-ref test'
7
8
. ./test-lib.sh
9
. "$TEST_DIRECTORY"/lib-gpg.sh
10
+. "$TEST_DIRECTORY"/lib-terminal.sh
11
12
# Mon Jul 3 23:18:43 2006 +0000
13
datestamp=1151968723
@@ -412,19 +413,33 @@ test_expect_success 'Check for invalid refname format' '
413
test_must_fail git for-each-ref --format="%(refname:INVALID)"
414
'
415
415
-cat >expected <<EOF
416
-$(git rev-parse --short refs/heads/master) <GREEN>master<RESET>
417
-$(git rev-parse --short refs/remotes/origin/master) <GREEN>origin/master<RESET>
418
-$(git rev-parse --short refs/tags/testtag) <GREEN>testtag<RESET>
419
-$(git rev-parse --short refs/tags/two) <GREEN>two<RESET>
420
-EOF
416
+test_expect_success 'set up color tests' '
417
+ cat >expected.color <<-EOF &&
418
+ $(git rev-parse --short refs/heads/master) <GREEN>master<RESET>
419
+ $(git rev-parse --short refs/remotes/origin/master) <GREEN>origin/master<RESET>
420
+ $(git rev-parse --short refs/tags/testtag) <GREEN>testtag<RESET>
421
+ $(git rev-parse --short refs/tags/two) <GREEN>two<RESET>
422
+ EOF
423
+ sed "s/<[^>]*>//g" <expected.color >expected.bare &&
424
+ color_format="%(objectname:short) %(color:green)%(refname:short)"
425
+'
426
422
-test_expect_success 'Check %(color:...) ' '
423
- git for-each-ref \
424
- --format="%(objectname:short) %(color:green)%(refname:short)" \
425
- >actual.raw &&
427
+test_expect_success TTY '%(color) shows color with a tty' '
428
+ test_terminal env TERM=vt100 \
429
+ git for-each-ref --format="$color_format" >actual.raw &&
430
test_decode_color <actual.raw >actual &&
427
- test_cmp expected actual
431
+ test_cmp expected.color actual
432
+'
433
+
434
+test_expect_success '%(color) does not show color without tty' '
435
+ TERM=vt100 git for-each-ref --format="$color_format" >actual &&
436
+ test_cmp expected.bare actual
437
+'
438
+
439
+test_expect_success 'color.ui=always can override tty check' '
440
+ git -c color.ui=always for-each-ref --format="$color_format" >actual.raw &&
441
+ test_decode_color <actual.raw >actual &&
442
+ test_cmp expected.color actual
443
'
444
445
cat >expected <<\EOF
t/t7004-tag.sh
+25
@@ -9,6 +9,7 @@ Tests for operations with tags.'
9
10
. ./test-lib.sh
11
. "$TEST_DIRECTORY"/lib-gpg.sh
12
+. "$TEST_DIRECTORY"/lib-terminal.sh
13
14
# creating and listing lightweight tags:
15
@@ -1900,6 +1901,30 @@ test_expect_success '--format should list tags as per format given' '
1901
test_cmp expect actual
1902
'
1903
1904
+test_expect_success "set up color tests" '
1905
+ echo "<RED>v1.0<RESET>" >expect.color &&
1906
+ echo "v1.0" >expect.bare &&
1907
+ color_args="--format=%(color:red)%(refname:short) --list v1.0"
1908
+'
1909
+
1910
+test_expect_success '%(color) omitted without tty' '
1911
+ TERM=vt100 git tag $color_args >actual.raw &&
1912
+ test_decode_color <actual.raw >actual &&
1913
+ test_cmp expect.bare actual
1914
+'
1915
+
1916
+test_expect_success TTY '%(color) present with tty' '
1917
+ test_terminal env TERM=vt100 git tag $color_args >actual.raw &&
1918
+ test_decode_color <actual.raw >actual &&
1919
+ test_cmp expect.color actual
1920
+'
1921
+
1922
+test_expect_success 'color.ui=always overrides auto-color' '
1923
+ git -c color.ui=always tag $color_args >actual.raw &&
1924
+ test_decode_color <actual.raw >actual &&
1925
+ test_cmp expect.color actual
1926
+'
1927
+
1928
test_expect_success 'setup --merged test tags' '
1929
git tag mergetest-1 HEAD~2 &&
1930
git tag mergetest-2 HEAD~1 &&