Subject: Re: [PATCH v4 8/8] t5520: check reflog action in fast-forward merge

From: Junio C Hamano <[email-masked]>
Date: Thu, 21 May 2015 10:29:13 -0700

Paul Tan <[email-masked]> writes:

> On Mon, May 18, 2015 at 11:20 PM, Johannes Schindelin
> ...
>>> +     sed "s/$_x05[0-9a-f]*/OBJID/g" reflog.actual >reflog.fuzzy &&
>>
>> Actually, let's use "s/^[0-9a-f]*/OBJID/" instead: you only want to
>> replace the first few characters.
>
> Did you mean "s/^$_x05[0-9a-f]*/OBJID/"? (with "$_x05" expanding to
> '[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' from test-lib.sh). If not,
> then it would match even if there was no SHA1 hash.
>
> But yes, without the "^" there will very likely be false positives.
> Thanks for catching.

I think the suggestion was more about "do we guarantee that there
would always be at least five?"  It might happen to be the case with
our current default, but these tests do not fundamentally rely on
that default staying the same.  s/^[0-9a-f][0-9a-f]*/OBJECTNAME/g is
probably the right balance between cautiousness (you do not want to
match an empty string) and future-proofing (you do not want to rely
on us having at least five).

Thanks.