Re: [PATCH v4 08/12] ref-filter: introduce align_atom_parser()
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Feb 4, 2016, 23:48 UTC
- Message-ID
- <CAPig+cQXK6xENN864XuxYFQEKtw59FAO7qh=wEGv4maomKpCzA@mail.gmail.com>
- In-Reply-To
- <1454262176-6594-9-git-send-email-Karthik.188@gmail.com>
On Sun, Jan 31, 2016 at 12:42 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
Show 26 quoted lines
> Introduce align_atom_parser() which will parse an 'align' atom and
> store the required alignment position and width in the 'used_atom'
> structure for further usage in populate_value().
>
> Since this patch removes the last usage of match_atom_name(), remove
> the function from ref-filter.c.
>
> Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> @@ -55,6 +61,37 @@ static align_type parse_align_position(const char *s)
> +static void align_atom_parser(struct used_atom *atom, const char *arg)
> +{
> + struct align *align = &atom->u.align;
> + struct strbuf **s, **to_free;
> + unsigned int width = ~0U;
> +
> + if (!arg)
> + die(_("expected format: %%(align:<width>,<position>)"));
> + s = to_free = strbuf_split_str_omit_term(arg, ',', 0);
> +
> + align->position = ALIGN_LEFT;
> +
> + while (*s) {
> + int position;
> + arg = s[0]->buf;It's confusing to see 'arg' being re-used here for a different purpose, and leads the reader to wonder if this is done because the s[0]->buf might be needed outside the loop (when, in fact, it isn't). It would be better to declare a new variable here in the scope of the 'while' loop to hold this value.
(I might have named the result of the strbuf split 'tokens' or even short-and-sweet 'v' -- for vector -- and then used 's' for the name of the new variable here in the 'while' loop, but these name suggestions aren't particularly important; it is important to declare a new variable here -- whatever you name it -- rather than re-using 'arg'.)
Show 15 quoted lines
> +
> + if (!strtoul_ui(arg, 10, &width))
> + ;
> + else if ((position = parse_align_position(arg)) >= 0)
> + align->position = position;
> + else
> + die(_("unrecognized %%(align) argument: %s"), arg);
> + s++;
> + }
> +
> + if (width == ~0U)
> + die(_("positive width expected with the %%(align) atom"));
> + align->width = width;
> + strbuf_list_free(to_free);
> +}