Re: [RFH] xdiff shows trivially redundant diff.

Davide Libenzi [off-list ref] writes:

No problem. That's only an eye-issue though, since the diff is still a
valid diff according to its definition where D=A-B => B+D==A && A-D==B
From the day I released 0.18, xregression is continuosly running w/out
any issue. I'll check it out though ...

There is another to report, when ctxlen == 0.

Between the attached files "diff -u0 8f352aa dd40a03", the 
header for a hunk with only inserted lines misidentify the
original location.

For example, the first hunk says:

	@@ -0,0 +6 @@
        +#include "diff.h"

Which is inconsistent with what GNU diff says:

	@@ -5,0 +6 @@
        +#include "diff.h"

I've tried this patch but it is not right; the diff between the
attached two files show a 47-line hunk that inserts at line 400,
then the next 6-line hunk inserts at line 401 which is obviously
bogus.

diff --git a/xdiff/xutils.c b/xdiff/xutils.c
index afaada1..3e7f999 100644
--- a/xdiff/xutils.c
+++ b/xdiff/xutils.c

@@ -244,7 +244,7 @@ int xdl_emit_hunk_hdr(long s1, long c1, 
 	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);

On Mon, 3 Apr 2006, Junio C Hamano wrote:

The fix is fine, but you should do the same even in the s2 case. The 
correct hunk should have been:

@@ -6,0 +6 @@

because the lines are actually inserted at the 6th position, but patch 
handle its own special 0 count case by adding 1 to the position, so I had 
to change even the xpatchi.c code. The 0-context diffs are pretty 
dangerous though, since in case of purely added hunks, patch has no way to 
verify the orig-file position by matching contexts.
Now I'll take a look at the pre-diff optimization issue ...



- Davide

--- xdiff/xpatchi.c
+++ xdiff/xpatchi.c

@@ -162,9 +162,9 @@
  	 * We start from zero, so decrement by one unless it's the special position
  	 * '0' inside the unified diff (new or deleted file).
  	 */
-	if (hki->s1 > 0)
+	if (hki->s1 > 0 && hki->c1 > 0)
  		hki->s1--;
-	if (hki->s2 > 0)
+	if (hki->s2 > 0 && hki->c2 > 0)
  		hki->s2--;

  	return 0;


--- xdiff/xutils.c
+++ xdiff/xutils.c

@@ -537,7 +537,7 @@
  	memcpy(buf, "@@ -", 4);
  	nb += 4;

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

  	memcpy(buf + nb, ",", 1);
  	nb += 1;

@@ -547,7 +547,7 @@
  	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);

  	memcpy(buf + nb, ",", 1);
  	nb += 1;

On Mon, 3 Apr 2006, Davide Libenzi wrote:

On Mon, 3 Apr 2006, Junio C Hamano wrote:

quoted

I've tried this patch but it is not right; the diff between the
attached two files show a 47-line hunk that inserts at line 400,
then the next 6-line hunk inserts at line 401 which is obviously
bogus.

That's fine, since that's orig file position *before* the insert. Try the 
same with GNU diff and you should see the same. Also try to create a diff 
with libxdiff (with the fix) and feed it to GNU patch.



- Davide