Re: [PATCH v12 10/13] tag.c: use 'ref-filter' data structures

From: Karthik Nayak <karthik.188@gmail.com>
Date: Aug 19, 2015, 15:51 UTC
Message-ID: <CAOLa=ZTL-3A1t9qM=Qa=A47qB6TekquKOuS_v58=rZ6-9=L9bw@mail.gmail.com>
In-Reply-To: <vpqzj1n2upc.fsf@anie.imag.fr>

On Wed, Aug 19, 2015 at 8:26 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 9 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> This is a temporary step before porting 'tag.c' to use 'ref-filter'
>> completely. As this is a temporary step, most of the code
>> introduced here will be removed when 'tag.c' is ported over to use
>> 'ref-filter' APIs
>
> If you resend: missing '.' at the end of sentence.
>

Ok will add.

Show 17 quoted lines

>> -     if (lines != -1)
>> +     if (filter.lines != -1)
>>               die(_("-n option is only allowed with -l."));
>> -     if (with_commit)
>> +     if (filter.with_commit)
>>               die(_("--contains option is only allowed with -l."));
>> -     if (points_at.nr)
>> +     if (filter.points_at.nr)
>>               die(_("--points-at option is only allowed with -l."));
>
> It may make sense to factor these checks into a function like
>
>   void check_filter_consistancy(struct ref_filter *filter)
>
> in ref-filter.c, since for-each-ref, branch and tag will eventually have
> the same set of constraints on the options.
>

Ah! this is needed only in branch and tag where we need to see if the options are used with tag.c and both have a different sort of check for this. Might need more of a cleanup in branch.c and tag.c before we could do something like this so that we could have a similar check.

for reference branch.c uses:

    if (!!delete + !!rename + !!new_upstream +
        list + unset_upstream > 1)
        usage_with_options(builtin_branch_usage, options);

whereas tag.c uses what you stated above.

-- 
Regards,
Karthik Nayak