Re: [PATCH v2 07/11] ref-filter: introduce align_atom_parser()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 16, 2015 at 10:29 AM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> Introduce align_atom_parser() which will parse an "align" atom and
> store the required alignment position and width in the "use_atom"
> structure for further usage in 'populate_value()'.
>
> Helped-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -63,6 +69,49 @@ static void color_atom_parser(struct used_atom *atom)
> +static void align_atom_parser(struct used_atom *atom)
> +{
> +       [...]
> +       match_atom_name(atom->str, "align", &buf);
> +       if (!buf)
> +               die(_("expected format: %%(align:<width>,<position>)"));
> +       [...]
> +}
> @@ -880,35 +924,7 @@ static void populate_value(struct ref_array_item *ref)
>                                 v->s = " ";
>                         continue;
>                 } else if (match_atom_name(name, "align", &valp)) {

Hmm, align_atom_parser() has already been called before we get here,
right? And it has already invoked match_atom_name() and died if the
argument to "align:" was missing, correct? If so, then this invocation
of match_atom_name() in populate_value() is unnecessary and should be
replaced with a simpler starts_with("align:").

Plus, 'valp' is not used, aside from this unnecessary
match_atom_name(), thus it can be removed as well.

> -                       struct align *align = &v->u.align;
> -                       struct strbuf **s, **to_free;
> -                       int width = -1;
> -
> -                       if (!valp)
> -                               die(_("expected format: %%(align:<width>,<position>)"));
> -
> -                       s = to_free = strbuf_split_str_omit_term(valp, ',', 0);
> -
> -                       align->position = ALIGN_LEFT;
> -
> -                       while (*s) {
> -                               if (!strtoul_ui(s[0]->buf, 10, (unsigned int *)&width))
> -                                       ;
> -                               else if (!strcmp(s[0]->buf, "left"))
> -                                       align->position = ALIGN_LEFT;
> -                               else if (!strcmp(s[0]->buf, "right"))
> -                                       align->position = ALIGN_RIGHT;
> -                               else if (!strcmp(s[0]->buf, "middle"))
> -                                       align->position = ALIGN_MIDDLE;
> -                               else
> -                                       die(_("improper format entered align:%s"), s[0]->buf);
> -                               s++;
> -                       }
> -
> -                       if (width < 0)
> -                               die(_("positive width expected with the %%(align) atom"));
> -                       align->width = width;
> -                       strbuf_list_free(to_free);
> +                       v->u.align = atom->u.align;
>                         v->handler = align_atom_handler;
>                         continue;
>                 } else if (!strcmp(name, "end")) {
> --
> 2.6.4
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]