Re: [RFC 2/4] ref-filter: add return value && strbuf to handlers

["Martin Ågren" <martin.agren@gmail.com>]

On 13 March 2018 at 11:16, Olga Telezhnaya <olyatelezhnaya@gmail.com> wrote:
> -static void append_atom(struct atom_value *v, struct ref_formatting_state *state)
> +static int append_atom(struct atom_value *v, struct ref_formatting_state *state,
> +                      struct strbuf *err)
>  {
>         /*
>          * Quote formatting is only done when the stack has a single
> @@ -493,6 +495,7 @@ static void append_atom(struct atom_value *v, struct ref_formatting_state *state
>                 quote_formatting(&state->stack->output, v->s, state->quote_style);
>         else
>                 strbuf_addstr(&state->stack->output, v->s);
> +       return 0;
>  }

Maybe "unused_err" instead of "err", to document that we are aware that
"err" is not being used and that it is ok. Something similar is done in
builtin/difftool.c.

> -static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state)
> +static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state,
> +                            struct strbuf *err)
>  {
>         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_seen)
> -               die(_("format: %%(then) atom used more than once"));
> -       if (if_then_else->else_atom_seen)
> -               die(_("format: %%(then) atom used after %%(else)"));
> +       if (!if_then_else) {
> +               strbuf_addstr(err, _("format: %(then) atom used without an %(if) atom"));
> +               return -1;
> +       } else if (if_then_else->then_atom_seen) {
> +               strbuf_addstr(err, _("format: %(then) atom used more than once"));
> +               return -1;
> +       } else if (if_then_else->else_atom_seen) {
> +               strbuf_addstr(err, _("format: %(then) atom used after %(else)"));
> +               return -1;
> +       }

I slowly start to wonder if we want a function (or macro)
error_strbuf_addstr(), which returns -1. That could probably wait,
though.

Martin