Re: [PATCH v7 1/6] object-name tests: add tests for ambiguous object blind spots
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 14, 2022, 18:45 UTC
- Message-ID
- <xmqqh7a6cg17.fsf@gitster.g>
- In-Reply-To
- <220114.861r1att7j.gmgdl@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 18 quoted lines
> On Thu, Jan 13 2022, Junio C Hamano wrote:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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.
Show 11 quoted lines
>> 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.