{"thread":{"id":"54551","subject":"git-diff bug?","startedAt":"2020-11-02T06:53:31Z","lastAt":"2020-11-03T16:58:14Z","messageCount":7,"participants":["Eli Barzilay","René Scharfe","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"408853","messageId":"CALO-guviA4xKjUi0HfA+RLkTPPaQw7KArj__A9fKz0oP3m5MGw@mail.gmail.com","threadId":"54551","inReplyTo":null,"subject":"git-diff bug?","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2020-11-02T06:53:33Z","receivedAt":"2020-11-02T06:53:31Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"Is the following a bug?\n\n $ printf \"aaa\\nbbb\\nccc\\n\\n\" > 1\n $ printf \"aaa\\nbbb\\n\\nccc\\n\" > 2\n $ git diff --ignore-blank-lines 1 2\n\nThis shows a weird output, as if `ccc` was removed and then re-added.\nFlipping the 1 & 2 names makes it show no difference at all. I tried\na bunch of variants, including --minimal, and the four algorithms, and\nall show the same results. (Similar brokenness happens with an empty\nline at the beginning on one side and after the first line on the\nother.)\n\nI'm really not sure that the following is a bug, because I see the\nsame behavior from `diff` (which is what made me try git-diff, hoping\nthat it would be more consistent). (But I can't think of any rational\nthat would make it not a bug.)\n\n-- \n ((x=>x(x))(x=>x(x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"408870","messageId":"72cfef26-e986-d34c-eea4-46ec0fda2688@web.de","threadId":"54551","inReplyTo":"CALO-guviA4xKjUi0HfA+RLkTPPaQw7KArj__A9fKz0oP3m5MGw@mail.gmail.com","subject":"Re: git-diff bug?","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-11-02T17:45:03Z","receivedAt":"2020-11-02T17:45:26Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 02.11.20 um 07:53 schrieb Eli Barzilay:\n> Is the following a bug?\n>\n> $ printf \"aaa\\nbbb\\nccc\\n\\n\" > 1\n> $ printf \"aaa\\nbbb\\n\\nccc\\n\" > 2\n> $ git diff --ignore-blank-lines 1 2\n>\n> This shows a weird output, as if `ccc` was removed and then re-added.\n> Flipping the 1 & 2 names makes it show no difference at all. I tried\n> a bunch of variants, including --minimal, and the four algorithms, and\n> all show the same results. (Similar brokenness happens with an empty\n> line at the beginning on one side and after the first line on the\n> other.)\n>\n> I'm really not sure that the following is a bug, because I see the\n> same behavior from `diff` (which is what made me try git-diff, hoping\n> that it would be more consistent). (But I can't think of any rational\n> that would make it not a bug.)\n\n $ printf \"aaa\\nbbb\\nccc\\n\\n\" > 1\n $ printf \"aaa\\nbbb\\n\\nccc\\n\" > 2\n\n $ diff --ignore-blank-lines -u 1 2\n --- 1\t2020-11-02 18:11:04.618133008 +0100\n +++ 2\t2020-11-02 18:11:04.618133008 +0100\n @@ -1,4 +1,4 @@\n aaa\n bbb\n -ccc\n\n +ccc\n $ diff --ignore-blank-lines -u 2 1\n\nThis matches your results. That the order makes a difference is a bit\nodd. Both are valid diffs of the inputs and neither one changes blank\nlines, though, so it doesn't look like a bug.\n\n $ git diff --ignore-blank-lines 1 2\n $ git diff --ignore-blank-lines 2 1\n $ git --version\n git version 2.29.2\n\nThis matches your expectation, but not your results. Which version do\nyou use?\n\nRené\n"},{"id":"408928","messageId":"CALO-gusRt4J5ar45mo7un-EENyt5cX2SQvcXgyMmaHNZg5bFUg@mail.gmail.com","threadId":"54551","inReplyTo":"72cfef26-e986-d34c-eea4-46ec0fda2688@web.de","subject":"Re: git-diff bug?","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2020-11-02T21:06:49Z","receivedAt":"2020-11-02T21:06:50Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"On Mon, Nov 2, 2020 at 12:45 PM René Scharfe wrote:\n>\n> $ printf \"aaa\\nbbb\\nccc\\n\\n\" > 1\n> $ printf \"aaa\\nbbb\\n\\nccc\\n\" > 2\n>\n> $ diff --ignore-blank-lines -u 1 2\n> --- 1 2020-11-02 18:11:04.618133008 +0100\n> +++ 2 2020-11-02 18:11:04.618133008 +0100\n> @@ -1,4 +1,4 @@\n> aaa\n> bbb\n> -ccc\n>\n> +ccc\n> $ diff --ignore-blank-lines -u 2 1\n\nYes, this is what I'm getting, also without a -u. (Also on 2.29.2)\n\n\n> This matches your results. That the order makes a difference is a bit\n> odd. Both are valid diffs of the inputs and neither one changes blank\n> lines, though, so it doesn't look like a bug.\n\nHow is it valid? Isn't the whole point of `--ignore-blank-lines` to\ndo the same thing as comparing a version of the files that drops all\nempty lines?\n\n>\n> $ git diff --ignore-blank-lines 1 2\n> $ git diff --ignore-blank-lines 2 1\n> $ git --version\n> git version 2.29.2\n>\n> This matches your expectation, but not your results. Which version do\n> you use?\n\n$ git diff --ignore-blank-lines 1 2\ndiff --git a/1 b/2\nindex fc13a35..bd05737 100644\n--- a/1\n+++ b/2\n@@ -1,4 +1,4 @@\n aaa\n bbb\n-ccc\n\n+ccc\n$ git --version\ngit version 2.29.2\n\n\n-- \n ((x=>x(x))(x=>x(x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"408931","messageId":"xmqqpn4vs9l9.fsf@gitster.c.googlers.com","threadId":"54551","inReplyTo":"72cfef26-e986-d34c-eea4-46ec0fda2688@web.de","subject":"Re: git-diff bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-02T22:00:18Z","receivedAt":"2020-11-02T22:00:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> $ diff --ignore-blank-lines -u 1 2\n> --- 1\t2020-11-02 18:11:04.618133008 +0100\n> +++ 2\t2020-11-02 18:11:04.618133008 +0100\n> @@ -1,4 +1,4 @@\n> aaa\n> bbb\n> -ccc\n>\n> +ccc\n> $ diff --ignore-blank-lines -u 2 1\n>\n> This matches your results. That the order makes a difference is a bit\n> odd. Both are valid diffs of the inputs and neither one changes blank\n> lines, though, so it doesn't look like a bug.\n\nInteresting. If \"diff\" happens to pick the line with \"ccc\" on it as\nthe unchanging pair of lines between the preimage and the postimage,\nthen another \"valid diff of the inputs\" would look like this:\n\n aaa\n bbb\n +\n ccc\n -\n\nWhat such a patch would change consists only of blank lines. It is\nreasonable to expect \"--ignore-blank-lines\" would turn it into a\nno-op, provided if \"diff\" picks \"ccc\" as the matching line.\n\nBut if \"diff\" picks that the blank line at the end of the original\nfile as unchanged line, then we'll see the diff quoted in the first\npart of this message. And that patch does not change any blank\nlines, so it is unreasonable to expect \"--ignore-blank-lines\" to\nturn it into a no-op.\n\nSo it all depends on which matching pair \"diff\" first picks, before\nthe \"are all the lines changed by the hunk blank ones?\" kicks in.\n\nOne could argue that \"diff\" should work hard to enumerate all the\npossible combinations (we just saw two possible combinations above)\nto find one that allows \"--ignore-blank-lines\" to produce an empty\npatch, but I am not sure it is a sensible thing to do.\n\n\n> $ git diff --ignore-blank-lines 1 2\n> $ git diff --ignore-blank-lines 2 1\n> $ git --version\n> git version 2.29.2\n>\n> This matches your expectation, but not your results. Which version do\n> you use?\n>\n> René\n"},{"id":"408932","messageId":"xmqqlffjs8ws.fsf@gitster.c.googlers.com","threadId":"54551","inReplyTo":"CALO-gusRt4J5ar45mo7un-EENyt5cX2SQvcXgyMmaHNZg5bFUg@mail.gmail.com","subject":"Re: git-diff bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-02T22:14:59Z","receivedAt":"2020-11-02T22:15:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eli Barzilay writes:\n\n>> This matches your results. That the order makes a difference is a bit\n>> odd. Both are valid diffs of the inputs and neither one changes blank\n>> lines, though, so it doesn't look like a bug.\n>\n> How is it valid?\n\nJust this part. Any patch output that correctly explains how the\npreimage text changed to the postimage text is a \"valid\" diff, and\nthat is how René used the word. There are multiple \"valid\" diff\nto bring the preimage to the postimage:\n\n (preimage) (postimage)\n aaa aaa\n bbb bbb\n ccc\n ccc\n\nIn an extreme case, this diff is even valid.\n\n @@ -1,4 +1,4 @@\n -aaa\n -bbb\n -ccc\n -\n +aaa\n +bbb\n +\n +ccc\n\nIt's just that it is not as _useful_ as other valid diff that\nexplains how the preimage changed to the postimage.\n\nHTH.\n"},{"id":"408980","messageId":"CALO-gusFac+GNrB9Rcbqteyv+gs5h0A-PTxnwRswZMhTnNBFyA@mail.gmail.com","threadId":"54551","inReplyTo":"xmqqlffjs8ws.fsf@gitster.c.googlers.com","subject":"Re: git-diff bug?","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2020-11-03T03:14:07Z","receivedAt":"2020-11-03T03:14:03Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"On Mon, Nov 2, 2020 at 5:15 PM Junio C Hamano wrote:\n>\n> Eli Barzilay writes:\n>\n> > How is it valid?\n>\n> Just this part. Any patch output that correctly explains how the\n> preimage text changed to the postimage text is a \"valid\" diff, and\n> that is how René used the word.\n\nTo be clear, the \"valid\" in my question is about the correctness of\nthe expected behavior, which (as I disclaimed) is likely a problem of\nthe text that explains these expectations. If validity is only about\nthe correctness of the resulting transform, then it is obviously\nvalid, as well as the other alternatives that you included (and\ntherefore this is not the meaning that I used, otherwise I wouldn't\nhave sent this).\n\nIn any case, I think that I now see the problem: the (sparse)\nexplanation says \"Ignore changes whose lines are all blank.\". It\nwould have been helpful to clarify with \"(but blank likes that are\n*part of* a change are still shown)\".\n\nThanks,\n-- \n ((x=>x(x))(x=>x(x))) Eli Barzilay:\n http://barzilay.org/ Maze is Life!\n"},{"id":"409018","messageId":"xmqqpn4us7hf.fsf@gitster.c.googlers.com","threadId":"54551","inReplyTo":"CALO-gusFac+GNrB9Rcbqteyv+gs5h0A-PTxnwRswZMhTnNBFyA@mail.gmail.com","subject":"Re: git-diff bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-03T16:58:04Z","receivedAt":"2020-11-03T16:58:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eli Barzilay writes:\n\n> On Mon, Nov 2, 2020 at 5:15 PM Junio C Hamano wrote:\n>>\n>> Eli Barzilay writes:\n>>\n>> > How is it valid?\n>>\n>> Just this part. Any patch output that correctly explains how the\n>> preimage text changed to the postimage text is a \"valid\" diff, and\n>> that is how René used the word.\n>\n> To be clear, the \"valid\" in my question is about the correctness of\n> the expected behavior,...\n\nI know that, and that is why I clarified that you two are using the\nsame word differently.\n\n> In any case, I think that I now see the problem: the (sparse)\n> explanation says \"Ignore changes whose lines are all blank.\". It\n> would have been helpful to clarify with \"(but blank likes that are\n> *part of* a change are still shown)\".\n\nLooks sensible ;-)\n"}]}