On Tue, Aug 18, 2015 at 2:37 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 now have a `handler()` for each atom_value which will be called
> when that atom_value is being parsed, and similarly an `at_end`
> function for each state which is to be called when the `end` atom is
> encountered. Using this implement the `align` atom which aligns the
> given strbuf by calling `strbuf_utf8_align()` from utf8.c
>
> This is done by assigning a 'handler' function to each atom_value and
> a related 'at_end' function for each state. The align_handler()
> defined uses strbuf_utf8_align() from utf8.c to perform the alignment.
>
> Add documentation and tests for the same.
>
> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> ---
> diff --git a/ref-filter.c b/ref-filter.c
> index 74532d3..ecbcc5a 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -626,6 +638,36 @@ static inline char *copy_advance(char *dst, const char *src)
> return dst;
> }
>
> +static void align_handler(struct ref_formatting_state *state)
> +{
> + struct align *align = (struct align *)state->cb_data;
> + struct strbuf s = STRBUF_INIT;
> +
> + strbuf_utf8_align(&s, align->position, align->width, state->output.buf);
> + strbuf_reset(&state->output);
> + strbuf_addbuf(&state->output, &s);
> + free(align);
> +}
Leaking 'strbuf s' here.
Also, this operation can be expressed more concisely as:
strbuf_utf8_align(&s, ...);
strbuf_swap(&state->output, &s);
strbuf_release(&s);
Seeing this is also making me question my earlier suggestion of making
pop_state() responsible for appending the current state's output to
the previous state's output. The reason is that if align_handler() is
responsible for appending to the previous state's output, then all the
above string handling collapses to the one line:
strbuf_utf8_align(&state->prev->output, ..., state->output.buf);
which is even simpler, and doesn't involve a temporary strbuf or
swapping of strbuf contents.
On the other hand, it won't always be the case that all handlers can
get by with such simple code, and they might end up creating temporary
strbuf(s) and such anyhow, so I don't feel too strongly about it, and
it can always be changed at a later date (by someone) if that approach
ends up being better.
--
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