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@xxxxxxxxx>
Signed-off-by: Kyle J. McKay <mackyle@xxxxxxxxx>
---
On Apr 2, 2015, at 19:19, Yi, EungJun wrote:
>> I timed this one versus the existing diff-highlight. It's about 7%
>> slower. That's not great, but is acceptable to me. The
>> String::Multibyte
>> version was a lot faster, which was nice (but I'm still unclear on
>> _why_).
>
> I think the reason is here:
>
>> sub split_line {
>> local $_ = shift;
>> return map { /$COLOR/ ? $_ : ($mbcs ? $mbcs->strsplit('', $_) :
>> split //) }
>> split /($COLOR)/;
>> }
>
> I removed "*" from "split /($COLOR*)/". Actually I don't know why "*"
> was required but I need to remove it to make my patch works correctly.
>
> On Fri, Apr 3, 2015 at 10:24 AM, Jeff King <peff@xxxxxxxx> wrote:
>> EungJun, does this version meet your needs?
This version differs from the former as follows:
1) Slightly faster code that eliminates the need for Encode::is_utf8.
2) The '*' quantifier is changed to '+' in the split_line regexs which
was probably the original intent anyway as using '*' generates useless
empty elements. This has the side effect of greatly increasing the
speed so the tiny speed penalty for the UTF-8 checking is vastly
overwhelmed by the overall speed up. :)
3) The 'use 5.008;' line has been added since the utf8::xxx functions
require Perl 5.8
-Kyle
contrib/diff-highlight/diff-highlight | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/contrib/diff-highlight/diff-highlight b/contrib/diff-highlight/diff-highlight
index 08c88bbc..ffefc31a 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -1,5 +1,6 @@
#!/usr/bin/perl
+use 5.008;
use warnings FATAL => 'all';
use strict;
@@ -164,8 +165,12 @@ sub highlight_pair {
sub split_line {
local $_ = shift;
- return map { /$COLOR/ ? $_ : (split //) }
- split /($COLOR*)/;
+ return utf8::decode($_) ?
+ map { utf8::encode($_); $_ }
+ map { /$COLOR/ ? $_ : (split //) }
+ split /($COLOR+)/ :
+ map { /$COLOR/ ? $_ : (split //) }
+ split /($COLOR+)/;
}
sub highlight_line {
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html