On Wed, Jan 6, 2016 at 2:52 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> Introduce contents_atom_parser() which will parse the '%(contents)'
>> atom and store information into the 'used_atom' structure based on the
>> modifiers used along with the atom.
>>
>> Helped-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
>> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
>> Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx>
>> ---
>> ref-filter.c | 74 +++++++++++++++++++++++++++++++++++++-----------------------
>> 1 file changed, 46 insertions(+), 28 deletions(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 32b4674..9e61530 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -39,6 +39,10 @@ static struct used_atom {
>> struct align align;
>> enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
>> remote_ref;
>> + struct {
>> + enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
>> + unsigned int nlines;
>> + } contents;
>> } u;
>> } *used_atom;
>> static int used_atom_cnt, need_tagged, need_symref;
>> @@ -96,6 +100,35 @@ static void remote_ref_atom_parser(struct used_atom *atom)
>> die(_("unrecognized format: %%(%s)"), atom->name);
>> }
>>
>> +static void contents_atom_parser(struct used_atom *atom)
>> +{
>> + 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 also wonder if the "&& !buf" in the first two are correct. What
> should happen to "%(subject:foo)", which gives you a non-empty buf?
> It may or may not be an error, but it should not fall thru and cause
> "BUG:parsing non-contents", should it?
>
I think It would be better to add specific messages there
if (match_atom_name(atom->name, "subject", &buf)) {
if (buf)
die("%%(subject) atom does not take arguments");
atom->u.contents.option = C_SUB;
return;
} else if (match_atom_name(atom->name, "body", &buf)) {
if (buf)
die("%%(body) atom does not take arguments");
atom->u.contents.option = C_BODY_DEP;
return;
} else if (!match_atom_name(atom->name, "contents", &buf))
die("BUG: parsing non-'contents'");
--
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html