From: Karthik Nayak <karthik.188@gmail.com>
Date: Sat, 10 Oct 2015 16:22:11 GMT
Subject: Re: [PATCH v2 01/10] ref-filter: implement %(if), %(then), and %(else) atoms
Message-ID: <CAOLa=ZSVJnZu2vuhR8breWkiYTQWb2ZxygGOv3QB4kqJ5nBDiA@mail.gmail.com>
In-Reply-To: <vpqvbahmc1i.fsf@grenoble-inp.fr>

On Fri, Oct 9, 2015 at 12:18 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> +static void if_then_else_handler(struct ref_formatting_stack **stack)
>> +{
>> +     struct ref_formatting_stack *cur = *stack;
>> +     struct ref_formatting_stack *prev = cur->prev;
>> +     struct if_then_else *if_then_else = (struct if_then_else *)cur->at_end_data;
>> +
>
> You should add
>
>         if (!if_then_else->then_atom)
>                 die(_("format: %%(if) atom used without a %%(then) atom"));
>
> here ...
>

Will do.

>> +static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
>> +{
>> +     struct ref_formatting_stack *cur = state->stack;
>> +     struct if_then_else *if_then_else = NULL;
>> +
>> +     if (cur->at_end == if_then_else_handler)
>> +             if_then_else = (struct if_then_else *)cur->at_end_data;
>> +     if (!if_then_else)
>> +             die(_("format: %%(then) atom used without an %%(if) atom"));
>> +     if (if_then_else->then_atom)
>> +             die(_("format: %%(then) atom used more than once"));
>> +     if_then_else->then_atom = 1;
>
> ... and
>
>         if (if_then_else->else_atom)
>                 die(_("format: %%(then) atom used after %%(else)"));
>
> here, just in case (adding the two corresponding test_must_fail wouldn't
> harm of course).
>

Will do, thanks!

-- 
Regards,
Karthik Nayak