Re: [PATCH] fetch: align new ref summary printout in UTF-8 locales

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 3, 2012, 19:26 UTC
Message-ID: <7vwr0aewlf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1346670609-19986-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> fetch does printf("%-*s", width, "foo") where "foo" can be an utf-8
> string, but width is bytes, not letters. This results in misaligned
> ref summary table.

"but width is bytes, not letters" is a misleading statement.

Be careful about three different quantities when talking about aligning on terminal output with monospaced fonts:

 - How many bytes does the string occupy in memory?
 - How many unicode codepoints are in the string?
 - How many display columns does the string occupy on terminal?

Note that some "letters" (e.g. Han) occupy two display columns, and you want to measure the "width" and compensate that for "bytes". Letter count do not come into the picture for the purpose of aligning the terminal output.

> Introduce gettext_length() function that returns the string length in
> letters. Make the code use TRANSPORT_SUMMARY(x) where the length is

Again, are you measuring "string length in letters"? Or are you trying to measure in terms of git_wcwidth()? If the latter, please name this after "width", not "length" to make it more clear.

> compensated properly in utf-8 locales.
> ---

Is it in vogue to omit Signed-off-by line these days or something?

Show 6 quoted lines

>  gettext_length() can be made to support other charsets too. But I'm
>  on utf-8, it's not my itch.
>
>  Grepping through '%-*s' does not reveal any other places that obviously
>  need adjustment like this (apply and remote might, but pathnames and
>  remote names are usually in ascii)

Thanks.