public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Tian Yuchen <a3205153416@gmail•com>
To: Jeff King <peff@peff•net>, git@vger•kernel.org
Cc: Scott Baker <scott@perturb•org>
Subject: Re: [PATCH 5/8] diff-highlight: use test_decode_color in tests
Date: Mon, 23 Mar 2026 01:24:00 +0800	[thread overview]
Message-ID: <b992e118-f948-4145-8d77-96f00b497f99@gmail.com> (raw)
In-Reply-To: <20260320004436.GE3654226@coredump.intra.peff.net>

On 3/20/26 08:44, Jeff King wrote:
> The diff-highlight tests use raw color bytes when comparing expected and
> actual output. Let's use test_decode_color, which is our usual technique
> in other tests. It makes reading test output diffs a bit easier, since
> you're not relying on your terminal to interpret the result (or worse,
> interpreting characters yourself via "cat -A").
> 
> This will also make it easier to add tests with new colors/attributes,
> without having to pre-define the byte sequences ourselves.
> 
> Signed-off-by: Jeff King <peff@peff•net>
> ---
>   .../diff-highlight/t/t9400-diff-highlight.sh  | 37 +++++++++----------
>   1 file changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> index 42d331c6cd..ba80cda7c8 100755
> --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> @@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd)
>   TEST_DIRECTORY="$CURR_DIR"/../../../t
>   DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
>   
> -CW="$(printf "\033[7m")"	# white
> -CR="$(printf "\033[27m")"	# reset
> -
>   GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master
>   export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>   . "$TEST_DIRECTORY"/test-lib.sh
> @@ -42,9 +39,9 @@ dh_test () {
>   	} >/dev/null &&
>   
>   	"$DIFF_HIGHLIGHT" <diff.raw >diff.hi &&
> -	test_strip_patch_header <diff.hi >diff.act
> +	test_strip_patch_header <diff.hi | test_decode_color >diff.act

Although this is just simple text filtering and leaving it as is 
wouldn’t cause any problems IMO, why not go ahead and add the && while 
you’re at it?

I've noticed that there are several missing &&.

>   	"$DIFF_HIGHLIGHT" <commit.raw >commit.hi &&
> -	test_strip_patch_header <commit.hi >commit.act &&
> +	test_strip_patch_header <commit.hi | test_decode_color >commit.act &&
>   	test_cmp patch.exp diff.act &&
>   	test_cmp patch.exp commit.act
>   }
> @@ -126,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-${CW}b${CR}bb
> -		+${CW}0${CR}bb
> +		-<REVERSE>b<NOREVERSE>bb
> +		+<REVERSE>0<NOREVERSE>bb
>   		 ccc
>   	EOF
>   '
> @@ -148,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-bb${CW}b${CR}
> -		+bb${CW}0${CR}
> +		-bb<REVERSE>b<NOREVERSE>
> +		+bb<REVERSE>0<NOREVERSE>
>   		 ccc
>   	EOF
>   '
> @@ -170,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-b${CW}b${CR}b
> -		+b${CW}0${CR}b
> +		-b<REVERSE>b<NOREVERSE>b
> +		+b<REVERSE>0<NOREVERSE>b
>   		 ccc
>   	EOF
>   '
> @@ -213,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' '
>   	dh_test a b <<-EOF
>   		@@ -1,3 +1,3 @@
>   		 aaa
> -		-b${CW}b${CR}b
> -		+b${CW}0${CR}b
> +		-b<REVERSE>b<NOREVERSE>b
> +		+b<REVERSE>0<NOREVERSE>b
>   		+ccc
>   	EOF
>   '
> @@ -232,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' '
>   	echo "unic${o_stroke}de" >b &&
>   	dh_test a b <<-EOF
>   		@@ -1 +1 @@
> -		-unic${CW}${o_accent}${CR}de
> -		+unic${CW}${o_stroke}${CR}de
> +		-unic<REVERSE>${o_accent}<NOREVERSE>de
> +		+unic<REVERSE>${o_stroke}<NOREVERSE>de
>   	EOF
>   '
>   
> @@ -250,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' '
>   	echo "unico${combine_circum}de" >b &&
>   	dh_test a b <<-EOF
>   		@@ -1 +1 @@
> -		-unic${CW}o${combine_accent}${CR}de
> -		+unic${CW}o${combine_circum}${CR}de
> +		-unic<REVERSE>o${combine_accent}<NOREVERSE>de
> +		+unic<REVERSE>o${combine_circum}<NOREVERSE>de
>   	EOF
>   '
>   
> @@ -333,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' '
>   	+++ b/file
>   	@@ -1,3 +1,3 @@
>   	 before
> -	-the ${CW}old${CR} line
> -	+the ${CW}new${CR} line
> +	-the <REVERSE>old<NOREVERSE> line
> +	+the <REVERSE>new<NOREVERSE> line
>   	 -leading dash
>   	EOF
>   	git log --graph -p -1 | "$DIFF_HIGHLIGHT" >actual.raw &&
> -	trim_graph <actual.raw | sed -n "/^---/,\$p" >actual &&
> +	trim_graph <actual.raw | sed -n "/^---/,\$p" | test_decode_color >actual &&
>   	test_cmp expect actual
>   '
>   

Thanks,

Yuchen


  reply	other threads:[~2026-03-22 17:24 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-20  0:41 [PATCH 0/8] some diff-highlight tweaks Jeff King
2026-03-20  0:42 ` [PATCH 1/8] diff-highlight: mention build instructions Jeff King
2026-03-20  0:42 ` [PATCH 2/8] diff-highlight: drop perl version dependency back to 5.8 Jeff King
2026-03-20  0:43 ` [PATCH 3/8] diff-highlight: check diff-highlight exit status in tests Jeff King
2026-03-20  0:43 ` [PATCH 4/8] t: add matching negative attributes to test_decode_color Jeff King
2026-03-20  0:44 ` [PATCH 5/8] diff-highlight: use test_decode_color in tests Jeff King
2026-03-22 17:24   ` Tian Yuchen [this message]
2026-03-22 20:47     ` Jeff King
2026-03-23  5:48       ` Tian Yuchen
2026-03-23  5:53         ` Jeff King
2026-03-20  0:45 ` [PATCH 6/8] diff-highlight: test color config Jeff King
2026-03-20  0:47 ` [PATCH 7/8] diff-highlight: allow module callers to pass in " Jeff King
2026-03-20  0:48 ` [PATCH 8/8] diff-highlight: fetch all config with one process Jeff King
2026-03-22 17:18   ` Tian Yuchen
2026-03-22 20:45     ` Jeff King
2026-03-23  5:39       ` Tian Yuchen
2026-03-23  5:57         ` Jeff King
2026-03-23  6:01 ` [PATCH v2 0/8] some diff-highlight tweaks Jeff King
2026-03-23  6:01   ` [PATCH v2 1/8] diff-highlight: mention build instructions Jeff King
2026-03-23  6:02   ` [PATCH v2 2/8] diff-highlight: drop perl version dependency back to 5.8 Jeff King
2026-03-23  6:02   ` [PATCH v2 3/8] diff-highlight: check diff-highlight exit status in tests Jeff King
2026-03-23  6:02   ` [PATCH v2 4/8] t: add matching negative attributes to test_decode_color Jeff King
2026-03-23  6:02   ` [PATCH v2 5/8] diff-highlight: use test_decode_color in tests Jeff King
2026-03-23  6:02   ` [PATCH v2 6/8] diff-highlight: test color config Jeff King
2026-03-23  6:02   ` [PATCH v2 7/8] diff-highlight: allow module callers to pass in " Jeff King
2026-03-23  6:02   ` [PATCH v2 8/8] diff-highlight: fetch all config with one process Jeff King
2026-03-23 16:38   ` [PATCH v2 0/8] some diff-highlight tweaks Junio C Hamano
2026-03-24  6:50     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b992e118-f948-4145-8d77-96f00b497f99@gmail.com \
    --to=a3205153416@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=peff@peff$(echo .)net \
    --cc=scott@perturb$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox