Re: Looking for a review (pretty-formats, hard truncation), was What's cooking in git.git (Nov 2022, #04; Fri, 18))

[Philip Oakley <philipoakley@iee.email>]

Hi Junio,

On 21/11/2022 00:37, Junio C Hamano wrote:
> Philip Oakley <philipoakley@iee.email> writes:
>
>> Ping, Hopefully an easy single patch review for someone on-list.
>>
>> Potential review points:
>>
>> Is the commit message sufficient?
>> Are the tests: Sufficient, Complete, Correct ?
>> Is `qz_to_tab_space` conversion applied correctly?
> Is the feature and the design sensible?
>
> Are the tests checking interesting cases?  The underlying mechanism
> uses strbuf_utf8_replace() because there are character strings whose
> display columns do not match their byte length (otherwise you can
> just use strlen() and chomp at byte boundary), so a test whose
> result would be different if strbuf_utf8_replace() were replaced
> with a more naive strbuf_splice() would be valuable and meaningful.

The tests do include those  utf8 cases. They are in the existing t/txxxx
tests that specifically cover the utf8 non-English characters. I did
check how they were constructed and what they were doing to confirm
expectations.

Were you thinking that an extra comment would be useful in the commit
message to confirm the completeness of the existing tests?

--
Philip