Re: [PATCH 9/9] add -i: verify in the tests that colors can be overridden

[Jeff King <peff@peff.net>]

On Tue, Nov 10, 2020 at 11:42:19PM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> Now that the Perl version produces the same output as the built-in
> version (mostly fixing bugs in the latter), let's add a regression test
> to verify that it stays this way.
> 
> Note that we only `grep` for the colored error message instead of
> verifying that the entire `stderr` consists of just this one line: when
> running the test script via `dash`, using the `-x` option to trace the
> commands, the sub-shell in `force_color` causes those commands to be
> traced into `err.raw` because we set, but do not export
> `BASH_XTRACEFD`).

Your reasoning here confuses me. If we are using dash, then surely
BASH_XTRACEFD does not matter either way? Perhaps the subshell is
important if we are running bash, though.

Either way, being forgiving to the use of "-x" is a nice convenience and
I approve.

> +test_expect_success 'colors can be overridden' '
> +	test_config color.interactive.header red &&
> +	test_config color.interactive.help green &&
> +	test_config color.interactive.prompt yellow &&
> +	test_config color.interactive.error blue &&
> +	test_config color.diff.frag magenta &&
> +	test_config color.diff.context cyan &&
> +	test_config color.diff.old bold &&
> +	test_config color.diff.new "bold red" &&

Since you are being so thorough, and since it is already in the output,
maybe color color.diff.meta, too? Like:

diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 28549a41a2..cca866c8f4 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -594,6 +594,7 @@ test_expect_success 'colors can be overridden' '
 	test_config color.interactive.help green &&
 	test_config color.interactive.prompt yellow &&
 	test_config color.interactive.error blue &&
+	test_config color.diff.meta italic &&
 	test_config color.diff.frag magenta &&
 	test_config color.diff.context cyan &&
 	test_config color.diff.old bold &&
@@ -625,9 +626,9 @@ test_expect_success 'colors can be overridden' '
 	  1:        +3/-0        +1/-1 <YELLOW>c<RESET>olor-test
 	<YELLOW>Patch update<RESET>>> <RED>           staged     unstaged path<RESET>
 	* 1:        +3/-0        +1/-1 <YELLOW>c<RESET>olor-test
-	<YELLOW>Patch update<RESET>>> <BOLD>diff --git a/color-test b/color-test<RESET>
-	<BOLD>--- a/color-test<RESET>
-	<BOLD>+++ b/color-test<RESET>
+	<YELLOW>Patch update<RESET>>> <ITALIC>diff --git a/color-test b/color-test<RESET>
+	<ITALIC>--- a/color-test<RESET>
+	<ITALIC>+++ b/color-test<RESET>
 	<MAGENTA>@@ -1,3 +1,3 @@<RESET>
 	<CYAN> context<RESET>
 	<BOLD>-old<RESET>

-Peff