Re: [PATCH replacement for take 3 4/4] color-words: take an optional regular expression describing words

[Thomas Rast <trast@student.ethz.ch>]

[-- Attachment #1: Type: text/plain, Size: 3984 bytes --]

Thomas Rast wrote:
> Johannes Schindelin wrote:
> > If a hunk header '@@ -2 +1,0 @@' is found that logically should be
> > '@@ -2 +2,0 @@', diff_words got confused.
> [...]
> > This might be a libxdiff issue, though.
> 
> Looks like it's just bug-for-bug compatible with diff.  At least my
> GNU diffutils 2.8.7 show the same behaviour.

I think the culprit is in

  commit ca557afff9f7dad7a8739cd193ac0730d872e282
  Author: Davide Libenzi <davidel@xmailserver.org>
  Date:   Mon Apr 3 18:47:55 2006 -0700

      Clean-up trivially redundant diff.

      Also corrects the line numbers in unified output when using
      zero lines context.
[...]
diff --git a/xdiff/xutils.c b/xdiff/xutils.c
[...]
  @@ -244,7 +257,7 @@ int xdl_emit_hunk_hdr(long s1, long c1, long s2, long c2,
          memcpy(buf, "@@ -", 4);
          nb += 4;

  -       nb += xdl_num_out(buf + nb, c1 ? s1: 0);
  +       nb += xdl_num_out(buf + nb, c1 ? s1: s1 - 1);

          if (c1 != 1) {
                  memcpy(buf + nb, ",", 1);
  @@ -256,7 +269,7 @@ int xdl_emit_hunk_hdr(long s1, long c1, long s2, long c2,
          memcpy(buf + nb, " +", 2);
          nb += 2;

  -       nb += xdl_num_out(buf + nb, c2 ? s2: 0);
  +       nb += xdl_num_out(buf + nb, c2 ? s2: s2 - 1);

          if (c2 != 1) {
                  memcpy(buf + nb, ",", 1);


Note how (for some reason I don't quite understand yet) "correcting"
the offsets involves subtracting 1 if there were no changes on that
side.

But skipping ahead to the end doesn't work if there are several such
instances where nothing was added.  So I think it must be fixed as
follows.

---- 8< ----
diff --git a/diff.c b/diff.c
index 4174d88..d7bbf74 100644
--- a/diff.c
+++ b/diff.c
@@ -361,8 +361,9 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 		diff_words->plus.orig[plus_first + plus_len - 1].end;
 
 	/*
-	 * since this is a --unified=0 diff, it can result in a single hunk
-	 * with a header like this: @@ -2 +1,0 @@
+	 * libxdiff subtracts one from the offset if the corresponding
+	 * length is 0.	 (This can only happen because we use
+	 * --unified=0.)
 	 *
 	 * This breaks the assumption that minus_first == plus_first.
 	 *
@@ -373,10 +374,9 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
 	 * It is only necessary for the plus part, as we show the common
 	 * words from that buffer.
 	 */
-	if (plus_len == 0 && minus_first + minus_len
-			== diff_words->minus.orig_nr)
+	if (plus_len == 0)
 		plus_begin = plus_end =
-			diff_words->plus.orig[diff_words->plus.orig_nr - 1].end;
+			diff_words->plus.orig[plus_first + plus_len].end;
 
 	if (diff_words->current_plus != plus_begin)
 		fwrite(diff_words->current_plus,
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 744221b..875b464 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -156,4 +156,40 @@ test_expect_success 'test when words are only removed at the end' '
 
 '
 
+echo 'abcd(Xefghijklmn(YZopqrst' > pre
+echo 'abcd(efghijklmn(opqrst' > post
+
+cat > expect <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index 434ff54..c4bb9f1 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
+<BROWN>@@ -1 +1 @@<RESET>
+abcd(<RED>X<RESET>efghijklmn(<RED>YZ<RESET>opqrst
+EOF
+
+test_expect_success 'no added words' '
+
+	word_diff --color-words=.
+
+'
+
+echo 'abcd(efghijklmn(opqrst' > pre
+echo 'abcd(Xefghijklmn(YZopqrst' > post
+
+cat > expect <<\EOF
+<WHITE>diff --git a/pre b/post<RESET>
+<WHITE>index c4bb9f1..434ff54 100644<RESET>
+<WHITE>--- a/pre<RESET>
+<WHITE>+++ b/post<RESET>
+<BROWN>@@ -1 +1 @@<RESET>
+abcd(<GREEN>X<RESET>efghijklmn(<GREEN>YZ<RESET>opqrst
+EOF
+
+test_expect_success 'no removed words' '
+
+	word_diff --color-words=.
+
+'
+
 test_done
-- 
1.6.1.283.g653b2

-- 
Thomas Rast
trast@{inf,student}.ethz.ch


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]