From: Bert Wesarg <bert.wesarg@googlemail.com>
Date: Mon, 22 Sep 2008 18:00:50 GMT
Subject: Re: [PATCH 1/3] for-each-ref: utilize core.warnambiguousrefs for strict refname:short format
Message-ID: <36ca99e90809221100q4db71514re448f5fa36a19ffc@mail.gmail.com>
In-Reply-To: <7vk5d4xi1m.fsf@gitster.siamese.dyndns.org>

On Mon, Sep 22, 2008 at 18:27, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
>> index 9b44092..e7b7712 100644
>> --- a/builtin-for-each-ref.c
>> +++ b/builtin-for-each-ref.c
>> @@ -571,7 +571,7 @@ static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
>>  /*
>>   * Shorten the refname to an non-ambiguous form
>>   */
>> -static char *get_short_ref(struct refinfo *ref)
>> +static void get_short_ref(struct refinfo *ref, int strict, char **short_ref)
>>  {
>>       int i;
>>       static char **scanf_fmts;
>
> As far as I can tell this changing of function signature does not help its
> existing caller nor the function's implementation.
>
> Why?  And especially why do this in a patch that does something else?
Good point.  I think it had something to do that I first want a
reverse of dwim_ref().  And this has this 'char **' argument.  Than I
changed it back to the old signature which makes this line:

-                                       refname = get_short_ref(ref);
+                                       refname = get_short_ref(ref,
warn_ambiguous_refs);

unbreakable or screams for an out factoring of all inside the 'if
(!prefixcmp(name, "refname")) {'

I would prefer the out factory.

Bert