Hi Patrick, On Wed, 2 Apr 2025, Patrick Steinhardt wrote: > 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. Or I am too slow, that's also a possibility. > 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. That makes sense. Not only would it be a chattier diff, it would be even harder to validate. I would probably have written an entire paragraph in the commit message just about this decision, if only to get frustration about the state of Git's tests off of my chest. Your decision to avoid spending more energy on this than you already did sounds like a smart one to me, and I am sorry that I forced you to explain this one more time. > Let me adapt the commit message once again and make it a bit more > concrete compared to the current fuzzy description. Your explanation is sufficient for me, therefore you do not need to send another iteration merely for my sake. Thank you, Johannes
