bug in git diff --unified=0 ?

bug in git diff --unified=0 ?

[Shawn O. Pearce]

I can't decide if this is a bug in JGit, or a bug in CGit, or just
an area where the patch format is very unclear.

The following 2 patches are identical, but use different context:

git diff --unified=1:

    diff --git a/X b/X
    index a3648a1..2d44096 100644
    --- a/X
    +++ b/X
    @@ -2,2 +2,3 @@
     b
    +c
     d
    @@ -16,4 +17,2 @@
     q
    -r
    -s
     t

git diff --unified=0:

    diff --git a/X b/X
    index a3648a1..2d44096 100644
    --- a/X
    +++ b/X
    @@ -2,0 +3 @@
    +c
    @@ -17,2 +17,0 @@
    -r
    -s
    
Now lets look at JGit's output of that last patch, unified=0:

    diff --git a/X b/X
    index a3648a1..2d44096 100644
    --- a/X
    +++ b/X
    @@ -3,0 +3,1 @@
    +c
    @@ -17,2 +18,0 @@
    -r
    -s

Notice that `git diff --unified=1` said the first hunk started on
line 2 of the pre-image, as the 2nd line of the file is the context
line "b\n".  I call that correct.

However, `git diff --unified=0` said the first hunk started on line
2 of the pre-image, but there is no pre-image mentioned.  JGit says
it starts on line 3 of the pre-image, because the script is adding
a "c\n" on line 3.

The exact opposite behavior can be seen in the 2nd hunk, where we
delete lines.  Again, `git diff --unified=0` seems to behave as
though we had 1 line of context, but we don't.

-- 
Shawn.

Re: bug in git diff --unified=0 ?

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> I can't decide if this is a bug in JGit, or a bug in CGit, or just
> an area where the patch format is very unclear.
>
> The following 2 patches are identical, but use different context:
>
> git diff --unified=1:
>
>     diff --git a/X b/X
>     index a3648a1..2d44096 100644
>     --- a/X
>     +++ b/X
>     @@ -2,2 +2,3 @@
>      b
>     +c
>      d
>     @@ -16,4 +17,2 @@
>      q
>     -r
>     -s
>      t
>
> git diff --unified=0:
>
>     diff --git a/X b/X
>     index a3648a1..2d44096 100644
>     --- a/X
>     +++ b/X
>     @@ -2,0 +3 @@
>     +c
>     @@ -17,2 +17,0 @@
>     -r
>     -s
>     
> Now lets look at JGit's output of that last patch, unified=0:
>
>     diff --git a/X b/X
>     index a3648a1..2d44096 100644
>     --- a/X
>     +++ b/X
>     @@ -3,0 +3,1 @@
>     +c
>     @@ -17,2 +18,0 @@
>     -r
>     -s

I do not know what the current status of the POSIX draft for spliting
"context diff" format into "copied context" and "unified context", but
see http://www.opengroup.org/austin/aardvark/latest/xcubug2.txt for the
definition that was on track the last time I checked.

    First the range of lines in each file shall be written in the
    following format:

    "@@^-%s^+%s^@@", <file1 range>, <file2 range>

    Each <range> field shall be of the form:

   "%1d", <beginning line number>

    if the range contains exactly one line, and:

   "%1d,%1d", <beginning line number>, <number of lines>

    otherwise.  If a range is empty, its beginning line number shall be
    the number of the line just before the range, or 0 if the empty
    range starts the file.

So, JGit is wrong.

Re: bug in git diff --unified=0 ?

[Shawn O. Pearce]

Junio C Hamano <gitster@pobox.com> wrote:
> I do not know what the current status of the POSIX draft for spliting
> "context diff" format into "copied context" and "unified context", but
> see http://www.opengroup.org/austin/aardvark/latest/xcubug2.txt for the
> definition that was on track the last time I checked.

Doh.  Thank you for the clarification Junio.
 
> So, JGit is wrong.

Yup, I agree.  /me works up a replacement patch.

-- 
Shawn.

Re: bug in git diff --unified=0 ?

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> I do not know what the current status of the POSIX draft for spliting
> "context diff" format into "copied context" and "unified context"...

POSIX 1003.1-2008 (base specifications issue 7) was published in December
2008.

    http://www.opengroup.org/onlinepubs/9699919799/toc.htm

The text in the part I quoted in the previous message seems to be
unchanged.

    http://www.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10_07