Re: [RFH] xdiff shows trivially redundant diff.
- From
- Davide Libenzi <davidel@xmailserver.org>
- Date
- Apr 3, 2006, 19:33 UTC
- Message-ID
- <Pine.LNX.4.64.0604031222360.30048@alien.or.mcafeemobile.com>
- In-Reply-To
- <7v4q1bglkp.fsf@assigned-by-dhcp.cox.net>
On Mon, 3 Apr 2006, Junio C Hamano wrote:
Show 41 quoted lines
> Davide Libenzi <davidel@xmailserver.org> 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);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;