Re: [PATCH v3 14/15] ref-filter: introduce contents_atom_parser()

[Karthik Nayak <karthik.188@gmail.com>]

On Thu, Jan 7, 2016 at 11:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>>>> +static void contents_atom_parser(struct used_atom *atom)
>>>> +{
>>>> +     const char * buf;
>
> const char *buf;
>

will change that.

>>>> +
>>>> +     if (match_atom_name(atom->name, "subject", &buf) && !buf) {
>>>> +             atom->u.contents.option = C_SUB;
>>>> +             return;
>>>> +     } else if (match_atom_name(atom->name, "body", &buf) && !buf) {
>>>> +             atom->u.contents.option = C_BODY_DEP;
>>>> +             return;
>>>> +     } if (!match_atom_name(atom->name, "contents", &buf))
>>>> +               die("BUG: parsing non-'contents'");
>>>
>>> Did you really intend to say "if" here, not "else if"?
>>
>> Not that it makes a difference here since both the previous
>> condition return. I think "else if" would be better.
>
> I am not sure if it is "Y would be better even though X and Y both
> would work".  It looks to me "X and Y behave differently, X is a bug
> and Y is correct".
>
> The above would behave differently between "if" and "else if" (and
> by the way, the code layout suggests it is "else if"; otherwise you
> would be starting "if" on its own line) when you feed "subject:foo",
> no?

It is indeed an "else if". What I was referring to was that since its like

if (cond_a) {
...
return;
} else if (cond_b) {
...
return;
} if (cond_c) {
...
}

cond_c is only checked if cond_a and cond_b don't hold good. Similar to
how 'else if' would work, because cond_a and cond_b return. Sorry for the
confusion.

-- 
Regards,
Karthik Nayak