Subject: Re: [PATCH v7 1/6] object-name tests: add tests for ambiguous object blind spots

From: Junio C Hamano <[email-masked]>
Date: Fri, 14 Jan 2022 10:45:40 -0800

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> On Thu, Jan 13 2022, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason  <[email-masked]> writes:
>>
>>> +test_cmp_failed_rev_parse () {
>>> +	cat >expect &&
>>> +	test_must_fail git -C "$1" rev-parse "$2" 2>actual.raw &&
>>> +	sed "s/\($2\)[0-9a-f]*/\1.../" <actual.raw >actual &&
>>> +	test_cmp expect actual
>>> +}
>>
>> That's dense, especially without a comment (or named variable) that
>> hints readers what the arguments to this helper (and its standard
>> input) ought to be.
>
> I got rid of the named variables from v6 in response to a "shift" that
> shifted the wrong number, but perhaps I should have just removed the
> "shift"?

I agree that is a more sensible thing you could have done.

>> As long as messages from rev-parse on the error stream never has
>> more than one abbreviated object name on a single line, the above
>> should give us a copy of the message with expected object name
>> abbreviated to $2; otherwise we might be missing a /g in the sed
>> script.
>
> In the v6 you rightly commented on the /g that was there previously not
> being needed :)
>
> So I dropped it, in this case we can rely on only getting the
> abbreviated output.

I do not care either way, as long as it is clearly stated why /g is
there (or why /g is missing) for the future developers.