Re: [PATCH 10/15] for-each-ref: introduce format specifier %>(*) and %<(*)

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Jun 4, 2013 at 7:35 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> +static unsigned int get_atom_width(struct format_one_atom_context *ctx,
> +                                  const char *start, const char *end)
> +{
> +       struct strbuf sb = STRBUF_INIT;
> +       int i, atom = parse_atom(start, end);
> +       unsigned int len = 0, sb_len;
> +       for (i = 0; i < ctx->maxcount; i++) {
> +               print_value(&sb, ctx->refs[i], atom, ctx->quote_style);
> +               sb_len = utf8_strnwidth(sb.buf, sb.len, 1);
> +               if (sb_len > len)
> +                       len = sb_len;
> +               strbuf_reset(&sb);
> +       }
> +       strbuf_release(&sb);
> +       return len;
> +}
> +

I mentioned it before and I do it again. This is not optimal. We
should cache the result of get_atom_width() after the first
calculation. I haven't done it yet because I'm still not sure if it's
worth supporting %<(*)%non-atom at this stage. Caching for atoms only
is much easier because atom is indexed. But I guess it's ok in this
shape unless you run this over hundreds of refs.
--
Duy