Re: [PATCH/RFC 1/1] diff.c: When appropriate, use utf8_strwidth()
- From
Torsten Bögershausen <tboegi@web.de>
- Date
- Aug 15, 2022, 06:34 UTC
- Message-ID
- <20220815063441.uxrtdqsggmrqxxl2@tb-raspi4>
- In-Reply-To
- <xmqqfshy773n.fsf@gitster.g>
On Sun, Aug 14, 2022 at 04:12:12PM -0700, Junio C Hamano wrote:
Show 29 quoted lines
> tboegi@web.de writes:
>
> > The choosen solution is to split code in diff.c like this
> >
> > strbuf_addf(&out, "%-*s", len, name);
> >
> > into 2 calls, like this:
> >
> > strbuf_addf(&out, "%s", name);
> > if (len > utf8_strwidth(name))
> > strbuf_addchars(&out, ' ', len - utf8_strwidth(name));
>
> Makes sense. Is utf8_strwidth(name) cheap enough that we can call
> it twice in a row on the same string casually, or should we avoid it
> with a new variable?
>
> It might be worth doing a helper function, even?
>
> static inline strbuf_pad(struct strbuf *out, const char *s, size_t width)
> {
> size_t w = utf8_strwidth(s);
>
> strbuf_addstr(out, s);
> if (w < width)
> strbuf_addchars(out, ' ', width - w);
> }
>
> Other than that, sounds very sensible.
>Thanks for the review.
Actually, the commit message is wrong - after writing it, the code was changed into
if (len > utf8_strwidth(name))
num_padding_spaces = len - utf8_strwidth(name);and later
if (num_padding_spaces) strbuf_addchars(&out, ' ', num_padding_spaces);
(And having written this, there is probably room for test cases, IOW: a V2 will come the next days)