On Tue, Apr 01, 2025 at 08:55:22PM +0200, Johannes Schindelin wrote: > On Thu, 27 Mar 2025, Patrick Steinhardt wrote: > > diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh > > index c7d8eb12453..f904fc19f69 100755 > > --- a/t/t4030-diff-textconv.sh > > +++ b/t/t4030-diff-textconv.sh > > @@ -26,13 +20,10 @@ cat >expect.text <<'EOF' > > +1 > > EOF > > > > -cat >hexdump <<'EOF' > > -#!/bin/sh > > -"$PERL_PATH" -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' < "$1" > > -EOF > > -chmod +x hexdump > > - > > test_expect_success 'setup binary file with history' ' > > + write_script hexdump <<-\EOF && > > + tr "\000\001" "01" <"$1" > > + EOF > > So here the `hexdump` script is written, basically replacing NUL and SOH > with the digits zero and one, respectively. I wonder why the script does > not call `test-tool hexdump` instead? And I wonder even more why no test > case has to be adapted below this change in the same file. I _guess_ that > the reason is that the file named, creatively, "file" is initialized with > a NUL and a newline, committed, then a line is appended that contains SOH > and a newline, and then the test cases verify the hunk _headers_ only? > > If using `test-tool hexdump <"$1"` would work here, too, I'd actually have > preferred that over the `tr` invocation, even if would still not be > recapitulating the functionality of that Perl script (which, contrary to > its name, seemed never to have output hexadecimal values...). > > To be clear: I do not suggest to change the patch, I am merely puzzled why > the more obvious `test-tool hexdump <"$1"` was not used here? Phillip had the same comment, and I was trying to address that by improving the commit message a bit. But seems like it still isn't clear enough. The reason why I decided against using `test-tool hexdump` is that it would have a ripple effect. The output generated by that helper is not the same as the output generated by the Perl script, so if we started to use the hexdump helper I would have to adapt a bunch of tests in this test file to update their expectations. The result would look something like the appended patch, which I think is quite awkward. On the one hand we have trailing whitespace in the expectation, on the other hand we have weird seemingly-unrelated changes in other tests. So I shied away from that and instead decided to use a simpler variant of the textconv script. Let me adapt the commit message once again and make it a bit more concrete compared to the current fuzzy description. Patrick diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh index f904fc19f69..16d7fd4c5ca 100755 --- a/t/t4030-diff-textconv.sh +++ b/t/t4030-diff-textconv.sh @@ -15,14 +15,14 @@ EOF cat >expect.text <<'EOF' --- a/file +++ b/file -@@ -1 +1,2 @@ - 0 -+1 +@@ -1 +1 @@ +-00 0a ++00 0a 01 0a EOF test_expect_success 'setup binary file with history' ' write_script hexdump <<-\EOF && - tr "\000\001" "01" <"$1" + test-tool hexdump <"$1" EOF test_commit --printf one file "\\0\\n" && test_commit --printf --append two file "\\01\\n" @@ -92,7 +92,7 @@ test_expect_success 'show blob produces binary' ' test_expect_success 'show --textconv blob produces text' ' git show --textconv HEAD:file >actual && - printf "0\\n1\\n" >expect && + printf "00 0a 01 0a \n" >expect && test_cmp expect actual ' @@ -103,7 +103,7 @@ test_expect_success 'show --no-textconv blob produces binary' ' ' test_expect_success 'grep-diff (-G) operates on textconv data (add)' ' - echo one >expect && + printf "two\none\n" >expect && git log --root --format=%s -G0 >actual && test_cmp expect actual ' @@ -115,7 +115,7 @@ test_expect_success 'grep-diff (-G) operates on textconv data (modification)' ' ' test_expect_success 'pickaxe (-S) operates on textconv data (add)' ' - echo one >expect && + printf "two\none\n" >expect && git log --root --format=%s -S0 >actual && test_cmp expect actual ' @@ -146,9 +146,8 @@ symlink=$(git rev-parse --short $(printf frotz | git hash-object --stdin)) cat >expect.typechange <<EOF --- a/file +++ /dev/null -@@ -1,2 +0,0 @@ --0 --1 +@@ -1 +0,0 @@ +-00 0a 01 0a diff --git a/file b/file new file mode 120000 index 0000000..$symlink
