pretty: respect color settings for %C placeholders

The color placeholders have traditionally been
unconditional, showing colors even when git is not otherwise
configured to do so. This was not so bad for their original
use, which was on the command-line (and the user could
decide at that moment whether to add colors or not). But
these days we have configured formats via pretty.*, and
those should operate correctly in multiple contexts.

In 3082517 (log --format: teach %C(auto,black) to respect
color config, 2012-12-17), we gave an extended placeholder
that could be used to accomplish this. But it's rather
clunky to use, because you have to specify it individually
for each color (and their matching resets) in the format.
We shied away from just switching the default to auto,
because it is technically breaking backwards compatibility.

However, there's not really a use case for unconditional
colors. The most plausible reason you would want them is to
redirect "git log" output to a file. But there, the right
answer is --color=always, as it does the right thing both
with custom user-format colors and git-generated colors.

So let's switch to the more useful default. In the
off-chance that somebody really does find a use for
unconditional colors without wanting to enable the rest of
git's colors, we provide a new %C(always,...) to enable the
old behavior. And we can remind them of --color=always in
the documentation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

1 files changed, 50 insertions, 34 deletions

diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 7b97a90bae..b326d550f3 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -59,7 +59,10 @@ test_format () {
 }
 
 # Feed to --format to provide predictable colored sequences.
+BASIC_COLOR='%Credfoo%Creset'
+COLOR='%C(red)foo%C(reset)'
 AUTO_COLOR='%C(auto,red)foo%C(auto,reset)'
+ALWAYS_COLOR='%C(always,red)foo%C(always,reset)'
 has_color () {
 	test_decode_color <"$1" >decoded &&
 	echo "<RED>foo<RESET>" >expect &&
@@ -177,7 +180,7 @@ test_expect_success 'basic colors' '
 	<RED>foo<GREEN>bar<BLUE>baz<RESET>xyzzy
 	EOF
 	format="%Credfoo%Cgreenbar%Cbluebaz%Cresetxyzzy" &&
-	git rev-list --format="$format" -1 master >actual.raw &&
+	git rev-list --color --format="$format" -1 master >actual.raw &&
 	test_decode_color <actual.raw >actual &&
 	test_cmp expect actual
 '
@@ -188,48 +191,61 @@ test_expect_success 'advanced colors' '
 	<BOLD;RED;BYELLOW>foo<RESET>
 	EOF
 	format="%C(red yellow bold)foo%C(reset)" &&
-	git rev-list --format="$format" -1 master >actual.raw &&
+	git rev-list --color --format="$format" -1 master >actual.raw &&
 	test_decode_color <actual.raw >actual &&
 	test_cmp expect actual
 '
 
-test_expect_success '%C(auto,...) does not enable color by default' '
-	git log --format=$AUTO_COLOR -1 >actual &&
-	has_no_color actual
-'
-
-test_expect_success '%C(auto,...) enables colors for color.diff' '
-	git -c color.diff=always log --format=$AUTO_COLOR -1 >actual &&
-	has_color actual
-'
-
-test_expect_success '%C(auto,...) enables colors for color.ui' '
-	git -c color.ui=always log --format=$AUTO_COLOR -1 >actual &&
-	has_color actual
-'
+for spec in \
+	"%Cred:$BASIC_COLOR" \
+	"%C(...):$COLOR" \
+	"%C(auto,...):$AUTO_COLOR"
+do
+	desc=${spec%%:*}
+	color=${spec#*:}
+	test_expect_success "$desc does not enable color by default" '
+		git log --format=$color -1 >actual &&
+		has_no_color actual
+	'
 
-test_expect_success '%C(auto,...) respects --color' '
-	git log --format=$AUTO_COLOR -1 --color >actual &&
-	has_color actual
-'
+	test_expect_success "$desc enables colors for color.diff" '
+		git -c color.diff=always log --format=$color -1 >actual &&
+		has_color actual
+	'
 
-test_expect_success '%C(auto,...) respects --no-color' '
-	git -c color.ui=always log --format=$AUTO_COLOR -1 --no-color >actual &&
-	has_no_color actual
-'
+	test_expect_success "$desc enables colors for color.ui" '
+		git -c color.ui=always log --format=$color -1 >actual &&
+		has_color actual
+	'
 
-test_expect_success TTY '%C(auto,...) respects --color=auto (stdout is tty)' '
-	test_terminal env TERM=vt100 \
-		git log --format=$AUTO_COLOR -1 --color=auto >actual &&
-	has_color actual
-'
+	test_expect_success "$desc respects --color" '
+		git log --format=$color -1 --color >actual &&
+		has_color actual
+	'
 
-test_expect_success '%C(auto,...) respects --color=auto (stdout not tty)' '
-	(
-		TERM=vt100 && export TERM &&
-		git log --format=$AUTO_COLOR -1 --color=auto >actual &&
+	test_expect_success "$desc respects --no-color" '
+		git -c color.ui=always log --format=$color -1 --no-color >actual &&
 		has_no_color actual
-	)
+	'
+
+	test_expect_success TTY "$desc respects --color=auto (stdout is tty)" '
+		test_terminal env TERM=vt100 \
+			git log --format=$color -1 --color=auto >actual &&
+		has_color actual
+	'
+
+	test_expect_success "$desc respects --color=auto (stdout not tty)" '
+		(
+			TERM=vt100 && export TERM &&
+			git log --format=$color -1 --color=auto >actual &&
+			has_no_color actual
+		)
+	'
+done
+
+test_expect_success '%C(always,...) enables color even without tty' '
+	git log --format=$ALWAYS_COLOR -1 >actual &&
+	has_color actual
 '
 
 test_expect_success '%C(auto) respects --color' '