git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* git-diff bug?
@ 2020-11-02  6:53 Eli Barzilay
  2020-11-02 17:45 ` René Scharfe
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Barzilay @ 2020-11-02  6:53 UTC (permalink / raw)
  To: git

Is the following a bug?

    $ printf "aaa\nbbb\nccc\n\n" > 1
    $ printf "aaa\nbbb\n\nccc\n" > 2
    $ git diff --ignore-blank-lines 1 2

This shows a weird output, as if `ccc` was removed and then re-added.
Flipping the 1 & 2 names makes it show no difference at all.  I tried
a bunch of variants, including --minimal, and the four algorithms, and
all show the same results.  (Similar brokenness happens with an empty
line at the beginning on one side and after the first line on the
other.)

I'm really not sure that the following is a bug, because I see the
same behavior from `diff` (which is what made me try git-diff, hoping
that it would be more consistent).  (But I can't think of any rational
that would make it not a bug.)

-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git-diff bug?
  2020-11-02  6:53 git-diff bug? Eli Barzilay
@ 2020-11-02 17:45 ` René Scharfe
  2020-11-02 21:06   ` Eli Barzilay
  2020-11-02 22:00   ` Junio C Hamano
  0 siblings, 2 replies; 7+ messages in thread
From: René Scharfe @ 2020-11-02 17:45 UTC (permalink / raw)
  To: Eli Barzilay, git

Am 02.11.20 um 07:53 schrieb Eli Barzilay:
> Is the following a bug?
>
>     $ printf "aaa\nbbb\nccc\n\n" > 1
>     $ printf "aaa\nbbb\n\nccc\n" > 2
>     $ git diff --ignore-blank-lines 1 2
>
> This shows a weird output, as if `ccc` was removed and then re-added.
> Flipping the 1 & 2 names makes it show no difference at all.  I tried
> a bunch of variants, including --minimal, and the four algorithms, and
> all show the same results.  (Similar brokenness happens with an empty
> line at the beginning on one side and after the first line on the
> other.)
>
> I'm really not sure that the following is a bug, because I see the
> same behavior from `diff` (which is what made me try git-diff, hoping
> that it would be more consistent).  (But I can't think of any rational
> that would make it not a bug.)

    $ printf "aaa\nbbb\nccc\n\n" > 1
    $ printf "aaa\nbbb\n\nccc\n" > 2

    $ diff --ignore-blank-lines -u 1 2
    --- 1	2020-11-02 18:11:04.618133008 +0100
    +++ 2	2020-11-02 18:11:04.618133008 +0100
    @@ -1,4 +1,4 @@
     aaa
     bbb
    -ccc

    +ccc
    $ diff --ignore-blank-lines -u 2 1

This matches your results.  That the order makes a difference is a bit
odd.  Both are valid diffs of the inputs and neither one changes blank
lines, though, so it doesn't look like a bug.

    $ git diff --ignore-blank-lines 1 2
    $ git diff --ignore-blank-lines 2 1
    $ git --version
    git version 2.29.2

This matches your expectation, but not your results.  Which version do
you use?

René

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git-diff bug?
  2020-11-02 17:45 ` René Scharfe
@ 2020-11-02 21:06   ` Eli Barzilay
  2020-11-02 22:14     ` Junio C Hamano
  2020-11-02 22:00   ` Junio C Hamano
  1 sibling, 1 reply; 7+ messages in thread
From: Eli Barzilay @ 2020-11-02 21:06 UTC (permalink / raw)
  To: René Scharfe; +Cc: git

On Mon, Nov 2, 2020 at 12:45 PM René Scharfe <l.s.r@web.de> wrote:
>
>     $ printf "aaa\nbbb\nccc\n\n" > 1
>     $ printf "aaa\nbbb\n\nccc\n" > 2
>
>     $ diff --ignore-blank-lines -u 1 2
>     --- 1       2020-11-02 18:11:04.618133008 +0100
>     +++ 2       2020-11-02 18:11:04.618133008 +0100
>     @@ -1,4 +1,4 @@
>      aaa
>      bbb
>     -ccc
>
>     +ccc
>     $ diff --ignore-blank-lines -u 2 1

Yes, this is what I'm getting, also without a -u.  (Also on 2.29.2)


> This matches your results.  That the order makes a difference is a bit
> odd.  Both are valid diffs of the inputs and neither one changes blank
> lines, though, so it doesn't look like a bug.

How is it valid?  Isn't the whole point of `--ignore-blank-lines` to
do the same thing as comparing a version of the files that drops all
empty lines?

>
>     $ git diff --ignore-blank-lines 1 2
>     $ git diff --ignore-blank-lines 2 1
>     $ git --version
>     git version 2.29.2
>
> This matches your expectation, but not your results.  Which version do
> you use?

$ git diff --ignore-blank-lines 1 2
diff --git a/1 b/2
index fc13a35..bd05737 100644
--- a/1
+++ b/2
@@ -1,4 +1,4 @@
 aaa
 bbb
-ccc

+ccc
$ git --version
git version 2.29.2


-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git-diff bug?
  2020-11-02 17:45 ` René Scharfe
  2020-11-02 21:06   ` Eli Barzilay
@ 2020-11-02 22:00   ` Junio C Hamano
  1 sibling, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2020-11-02 22:00 UTC (permalink / raw)
  To: René Scharfe; +Cc: Eli Barzilay, git

René Scharfe <l.s.r@web.de> writes:

>     $ diff --ignore-blank-lines -u 1 2
>     --- 1	2020-11-02 18:11:04.618133008 +0100
>     +++ 2	2020-11-02 18:11:04.618133008 +0100
>     @@ -1,4 +1,4 @@
>      aaa
>      bbb
>     -ccc
>
>     +ccc
>     $ diff --ignore-blank-lines -u 2 1
>
> This matches your results.  That the order makes a difference is a bit
> odd.  Both are valid diffs of the inputs and neither one changes blank
> lines, though, so it doesn't look like a bug.

Interesting.  If "diff" happens to pick the line with "ccc" on it as
the unchanging pair of lines between the preimage and the postimage,
then another "valid diff of the inputs" would look like this:

     aaa
     bbb
    +
     ccc
    -

What such a patch would change consists only of blank lines.  It is
reasonable to expect "--ignore-blank-lines" would turn it into a
no-op, provided if "diff" picks "ccc" as the matching line.

But if "diff" picks that the blank line at the end of the original
file as unchanged line, then we'll see the diff quoted in the first
part of this message.  And that patch does not change any blank
lines, so it is unreasonable to expect "--ignore-blank-lines" to
turn it into a no-op.

So it all depends on which matching pair "diff" first picks, before
the "are all the lines changed by the hunk blank ones?" kicks in.

One could argue that "diff" should work hard to enumerate all the
possible combinations (we just saw two possible combinations above)
to find one that allows "--ignore-blank-lines" to produce an empty
patch, but I am not sure it is a sensible thing to do.


>     $ git diff --ignore-blank-lines 1 2
>     $ git diff --ignore-blank-lines 2 1
>     $ git --version
>     git version 2.29.2
>
> This matches your expectation, but not your results.  Which version do
> you use?
>
> René

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git-diff bug?
  2020-11-02 21:06   ` Eli Barzilay
@ 2020-11-02 22:14     ` Junio C Hamano
  2020-11-03  3:14       ` Eli Barzilay
  0 siblings, 1 reply; 7+ messages in thread
From: Junio C Hamano @ 2020-11-02 22:14 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: René Scharfe, git

Eli Barzilay <eli@barzilay.org> writes:

>> This matches your results.  That the order makes a difference is a bit
>> odd.  Both are valid diffs of the inputs and neither one changes blank
>> lines, though, so it doesn't look like a bug.
>
> How is it valid?

Just this part.  Any patch output that correctly explains how the
preimage text changed to the postimage text is a "valid" diff, and
that is how René used the word.  There are multiple "valid" diff
to bring the preimage to the postimage:

    (preimage)          (postimage)
    aaa                 aaa
    bbb                 bbb
    ccc
                        ccc

In an extreme case, this diff is even valid.

    @@ -1,4 +1,4 @@
    -aaa
    -bbb
    -ccc
    -
    +aaa
    +bbb
    +
    +ccc

It's just that it is not as _useful_ as other valid diff that
explains how the preimage changed to the postimage.

HTH.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git-diff bug?
  2020-11-02 22:14     ` Junio C Hamano
@ 2020-11-03  3:14       ` Eli Barzilay
  2020-11-03 16:58         ` Junio C Hamano
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Barzilay @ 2020-11-03  3:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: René Scharfe, git

On Mon, Nov 2, 2020 at 5:15 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Eli Barzilay <eli@barzilay.org> writes:
>
> > How is it valid?
>
> Just this part.  Any patch output that correctly explains how the
> preimage text changed to the postimage text is a "valid" diff, and
> that is how René used the word.

To be clear, the "valid" in my question is about the correctness of
the expected behavior, which (as I disclaimed) is likely a problem of
the text that explains these expectations.  If validity is only about
the correctness of the resulting transform, then it is obviously
valid, as well as the other alternatives that you included (and
therefore this is not the meaning that I used, otherwise I wouldn't
have sent this).

In any case, I think that I now see the problem: the (sparse)
explanation says "Ignore changes whose lines are all blank.".  It
would have been helpful to clarify with "(but blank likes that are
*part of* a change are still shown)".

Thanks,
-- 
                   ((x=>x(x))(x=>x(x)))                  Eli Barzilay:
                   http://barzilay.org/                  Maze is Life!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: git-diff bug?
  2020-11-03  3:14       ` Eli Barzilay
@ 2020-11-03 16:58         ` Junio C Hamano
  0 siblings, 0 replies; 7+ messages in thread
From: Junio C Hamano @ 2020-11-03 16:58 UTC (permalink / raw)
  To: Eli Barzilay; +Cc: René Scharfe, git

Eli Barzilay <eli@barzilay.org> writes:

> On Mon, Nov 2, 2020 at 5:15 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Eli Barzilay <eli@barzilay.org> writes:
>>
>> > How is it valid?
>>
>> Just this part.  Any patch output that correctly explains how the
>> preimage text changed to the postimage text is a "valid" diff, and
>> that is how René used the word.
>
> To be clear, the "valid" in my question is about the correctness of
> the expected behavior,...

I know that, and that is why I clarified that you two are using the
same word differently.

> In any case, I think that I now see the problem: the (sparse)
> explanation says "Ignore changes whose lines are all blank.".  It
> would have been helpful to clarify with "(but blank likes that are
> *part of* a change are still shown)".

Looks sensible ;-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-03 16:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02  6:53 git-diff bug? Eli Barzilay
2020-11-02 17:45 ` René Scharfe
2020-11-02 21:06   ` Eli Barzilay
2020-11-02 22:14     ` Junio C Hamano
2020-11-03  3:14       ` Eli Barzilay
2020-11-03 16:58         ` Junio C Hamano
2020-11-02 22:00   ` Junio C Hamano

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git