On Sat, Sep 5, 2015 at 2:52 PM, Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
> Implement an `align` atom which left-, middle-, or right-aligns the
> content between %(align:...) and %(end).
>
> It is followed by `:<width>,<position>`, where the `<position>` is
> either left, right or middle and `<width>` is the size of the area
> into which the content will be placed. If the content between
> %(align:...) and %(end) is more than the width then no alignment is
> performed. e.g. to align a refname atom to the middle with a total
> width of 40 we can do: --format="%(align:middle,40)%(refname)%(end)".
>
> We introduce an `at_end` function for each element of the stack which
> is to be called when the `end` atom is encountered. Using this we
> implement end_align_handler() for the `align` atom, this aligns the
> final strbuf by calling `strbuf_utf8_align()` from utf8.c.
>
> Ensure that quote formatting is performed on the whole of
> %(align:...)...%(end) rather than individual atoms inside. We skip
> quote formatting for individual atoms when the current stack element
> is handling an %(align:...) atom and perform quote formatting at the
> end when we encounter the %(end) atom of the second element of then
> stack.
>
> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> ---
> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
> index e49d578..b23412d 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -127,6 +127,16 @@ color::
> +align::
> + Left-, middle-, or right-align the content between %(align:...)
> + and %(end). Followed by `:<width>,<position>`, where the
> + `<position>` is either left, right or middle and `<width>` is
> + the total length of the content with alignment. If the
This should mention that <position> is optional and default to "left"
if omitted.
> + contents length is more than the width then no alignment is
> + performed. If used with '--quote' everything in between
> + %(align:...) and %(end) is quoted, but if nested then only the
> + topmost level performs quoting.
> diff --git a/ref-filter.c b/ref-filter.c
> index e99c342..6c9ef08 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -773,6 +837,50 @@ static void populate_value(struct ref_array_item *ref)
> else
> v->s = " ";
> continue;
> + } else if (match_atom_name(name, "align", &valp)) {
> + struct align *align = &v->u.align;
> + struct strbuf **s;
> +
> + if (!valp)
> + die(_("expected format: %%(align:<width>, <position>)"));
I'm pretty sure this parsing code won't deal well with a space after
the comma, so the space should be dropped from the diagnostic message
advising the user of the correct format: s/, /,/
> + /*
> + * TODO: Implement a function similar to strbuf_split_str()
> + * which would strip the terminator at the end.
"...which would omit the separator from the end of each value."
> + */
> + s = strbuf_split_str(valp, ',', 0);
> +
> + /* If the position is given trim the ',' from the first strbuf */
> + if (s[1])
> + strbuf_setlen(s[0], s[0]->len - 1);
> + if (s[2])
> + die(_("align:<width>,<position> followed by garbage: %s"), s[2]->buf);
If <position> is omitted, strbuf_split_str("42", ...) will return the
array ["42", NULL] which won't have an element [2], which means this
code will access beyond end-of-array. (This is another good argument
for looping over s[] as Junio suggested rather than assuming these
fixed yet optional positions. It can be hard to get it right.)
> + if (strtoul_ui(s[0]->buf, 10, &align->width))
> + die(_("positive width expected align:%s"), s[0]->buf);
> +
> + /*
> + * TODO: Implement a more general check, so that the values
> + * do not always have to be in a specific order.
> + */
> + if (!s[1])
> + align->position = ALIGN_LEFT;
> + else if (!strcmp(s[1]->buf, "left"))
> + align->position = ALIGN_LEFT;
> + else if (!strcmp(s[1]->buf, "right"))
> + align->position = ALIGN_RIGHT;
> + else if (!strcmp(s[1]->buf, "middle"))
> + align->position = ALIGN_MIDDLE;
> + else
> + die(_("improper format entered align:%s"), s[1]->buf);
> +
> + strbuf_list_free(s);
> +
> + v->handler = align_atom_handler;
> + continue;
> + } else if (!strcmp(name, "end")) {
> + v->handler = end_atom_handler;
> + continue;
> } else
> continue;
--
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