Re: [PATCH v3] diff-highlight: do not split multibyte characters

From: Yi, EungJun <semtlenori@gmail.com>
Date: Apr 4, 2015, 14:47 UTC
Message-ID: <CAFT+Tg_dFvpxauPJgRi86qTFo4k6dXa6WST+UPTguisA9ma83Q@mail.gmail.com>
In-Reply-To: <20150404140902.GA25455@peff.net>

On Fri, Apr 3, 2015 at 10:24 AM, Jeff King <peff@peff.net> wrote:

>
> EungJun, does this version meet your needs?
>
> -Peff

Yes, this patch is enough to meet my needs because it works well on UTF-8, the only encoding I use. And this patch looks better than my one because it is smaller, doesn't depend on String::Multibyte and seems to have no side-effect.

I hope someone who use another multibyte encoding will send a patch to support the encoding in future... :)

On Sat, Apr 4, 2015 at 11:09 PM, Jeff King <peff@peff.net> wrote:

Show 37 quoted lines

> On Fri, Apr 03, 2015 at 03:15:14PM -0700, Kyle J. McKay wrote:
>
>> When the input is UTF-8 and Perl is operating on bytes instead of
>> characters, a diff that changes one multibyte character to another
>> that shares an initial byte sequence will result in a broken diff
>> display as the common byte sequence prefix will be separated from
>> the rest of the bytes in the multibyte character.
>>
>> For example, if a single line contains only the unicode character
>> U+C9C4 (encoded as UTF-8 0xEC, 0xA7, 0x84) and that line is then
>> changed to the unicode character U+C9C0 (encoded as UTF-8 0xEC,
>> 0xA7, 0x80), when operating on bytes diff-highlight will show only
>> the single byte change from 0x84 to 0x80 thus creating invalid UTF-8
>> and a broken diff display.
>>
>> Fix this by putting Perl into character mode when splitting the line
>> and then back into byte mode after the split is finished.
>>
>> The utf8::xxx functions require Perl 5.8 so we require that as well.
>>
>> Also, since we are mucking with code in the split_line function, we
>> change a '*' quantifier to a '+' quantifier when matching the $COLOR
>> expression which has the side effect of speeding everything up while
>> eliminating useless '' elements in the returned array.
>>
>> Reported-by: Yi EungJun <semtlenori@gmail.com>
>> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
>
> This version looks good to me. I looked over the diff of running "git
> log -p --color" on git.git through diff-highlight before and after this
> patch, and everything looks like an improvement.
>
>   Acked-by: Jeff King <peff@peff.net>
>
> Thanks both of you for working on this.
>
> -Peff