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

From: Junio C Hamano <[email-masked]>
Date: Thu, 07 Jan 2016 10:04:46 -0800

Karthik Nayak <[email-masked]> writes:

>>> +static void contents_atom_parser(struct used_atom *atom)
>>> +{
>>> +     const char * buf;

const char *buf;

>>> +
>>> +     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?