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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 3, 2012, 20:30 UTC
Message-ID: <7vk3waetnf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <50450C8D.7010301@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 22 quoted lines

> Am 03.09.2012 21:26, schrieb Junio C Hamano:
>> 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.
>
> If I'm reading POSIX correctly, printf measures the width in %*s in bytes.

Yes, that is the problem.

When we give allocated_width columns to display str, and call

        printf("%.*s", allocated_width, str)

we end up getting (allocated_width - strlen(str)) SPs as filler. If the str occupies less than strlen(str) columns (and it often is the case for a string with non-ascii characters), insufficient number of filler SPs will be given, and an item on the same line that comes after this printf() will appear too close to str. By measuring the required number of columns to display str, you can compensate. i.e.

        printf("%.*s", allocated_width +
                (strlen(str) - display_width(str)), str)

to give it enough filler.

I think the patch uses utf8_strwidth() which measures the display columns needed for the string, not number of unicode codepoints in the string. I was merely pointing out that the wording of the proposed log message "bytes, not letters" and the function name "foo_length" was misleading, when we mean "width".