[bug] git diff --word-diff gives wrong result for utf-8 chinese

Result of "git diff"

-  为1
+  为2

or (if chinese can not be displayed correctly)

-  <E4><B8><BA>1
+  <E4><B8><BA>2

Actual result of "git diff --color-words"

<E4><B8>[-<BA>1-]{+<BA>2+}

Expected result of "git diff --color-words"

为[-1-]{+2+}

or (if chinese can not be displayed correctly)

<E4><B8><BA>[-1-]{+2+}

Ping Yin

sorry, typo, s/--color-words/--word-diff/g

Ping Yin

On Tue, Nov 29, 2022 at 11:46 AM Ping Yin <pkufranky@gmail.com> wrote:

On 11/29/22 10:49, Ping Yin wrote:

Hi, welcome to Git mailing list!

Please remind yourself:

  * Do not send HTML mails, send plain-text ones instead. Many mailing
    lists (including vger.kernel.org that powers Git ML) reject HTML
    emails for these are likely spam. Make sure your email isn't mangled
    (tabs and spaces as-is, no line wrapping).

  * Do not top-post, reply inline with appropriate context instead. I
    have to cut the reply context as a result.

  * When you submit a patch and people reply with their reviews, engage
    with them (either sending revised patch addressing the reviews or
    reply with justification). They will ignore you if you ignore them.

Thanks.

On Tue, Nov 29 2022, Ping Yin wrote:

I think we could provide new ways to do per-language diffs, right now you can use --word-diff-regex, but it would be handy to e.g. have a built-in collection of those (or other non-regex boundary algorithms) for Chinese etc.

But as for considering this a bug, or changing the existing behavior I think we'd need to deal with:

 * We (approximately) split on space now, which is certainly
   ASCII-biased, and outside of CJK fairly somewhat universal.

 * If we're going to split on "real words" in some cross-language aware
   way, are we going to run into conflicts between what different
   languages would consider sensible rules?

 * We probably don't want to make the "diff" dependent on the user's
   locale, but e.g. saying "I want a Chinese diff" via a CLI option
   would be OK.

 * Even for say Chinese, there's probably interesting edge cases when
   it's combined with other languages or character sets (e.g. Chinese +
   HTML).

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

I think you are thinking it with unnecessaarily complexity.  

The only thing that needs noticing in the above example, I think is, that the three-byte sequence E4-B8-BA in the example is supposed to be a single unicode character, and the actual result depicted can happen only if we (incorrectly) chomp that single character in the middle.

No matter what language we are using, we shouldn't do that.

I suspect that "--word-diff" internal is not even aware what a character is, but if you assume UTF-8 (precomposed), then you should be able to tell where the character boundary is by only looking at the high-bit patterns to avoid producing such an output.

On Tue, Nov 29, 2022 at 08:32:58PM +0900, Junio C Hamano wrote:

Agreed that we should probably avoid breaking characters. But what puzzles me more is that we break it between B8 and BA, and not elsewhere. Why not between E4 and B8? Why not between BA and "1"?

If the rule is "break on ascii whitespace", then I'd have expected the whole four-character sequence to be taken as a unit. In other words, it does should not have to care that a character is, as long as the bytes for space characters cannot appear inside other characters (which is true of utf8).

-Peff

On Tue, Nov 29, 2022 at 01:23:27PM -0500, Jeff King wrote:

Even more puzzling is that it produces the expected output for me:

  [note that \x is a bash-ism]
  $ printf '\xe4\xb8\xba1' >one
  $ printf '\xe4\xb8\xba2' >two
  $ git diff --no-index --word-diff one two
  diff --git a/one b/two
  index 9ae469fc41..576e6e32d8 100644
  --- a/one
  +++ b/two
  @@ -1 +1 @@
  [-为1-]{+为2+}

I wonder if OP has diff.wordRegex config (or attributes triggering a diff.*.wordRegex) that is doing something else.

-Peff

Wow, you are right, sorry for the noise.

$ git config -l | grep word diff.wordregex=[[:alnum:]_]+|[^[:space:]]

Is there a way to achieve this: break english by word, and break chinese by utf-8 character

Hi Ping

On 01/12/2022 07:33, Ping Yin wrote:

You could extend your current regex so that it matches whole utf-8 codepoints which is what git does for the builtin userdiff regexes. I've not tested it but I think

git config --global diff.wordregex "[[:alnum:]_]+|[^[:space:]]|$(printf '[\xc0-\xff][\x80-\xbf]+')"

should work. The downside is that you end up with a .gitconfig that is not valid utf-8. Perhaps someone else has a clever idea to get around that.

Best Wishes

Phillip

Ping Yin

On Thu, Dec 1, 2022 at 10:51 PM Phillip Wood <phillip.wood123@gmail.com> wrote:

Wow, it works. Thanks very much.

On Thu, Dec 01, 2022 at 02:51:29PM +0000, Phillip Wood wrote:

I think in more advanced regular expression engines you can do stuff like matching "[\x{4e00}-\x{9fcc}]", or even "\p{Han}". But I don't know that the stock libc regex is capable of anything like this, even with EREs. That's the only option Git provides for matching word regexes, but in theory we could support libpcre. We already can optionally build against it; we would just need config/plumbing to get it into diff.c:find_word_boundaries().

-Peff