{"thread":{"id":"47827","subject":"make git diff output easier to read - use better diff heuristics","startedAt":"2018-02-13T15:06:22Z","lastAt":"2018-02-13T22:41:19Z","messageCount":5,"participants":["Σπύρος Βαζαίος","Jeff King","Stefan Beller","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"339199","messageId":"CAOmC-AncOZfAwowXLjwDbL9Bdx1+=RbXsrd72LzEBd2W-=LAQg@mail.gmail.com","threadId":"47827","inReplyTo":null,"subject":"make git diff output easier to read - use better diff heuristics","fromName":"Σπύρος Βαζαίος","fromEmail":"sbazaios@gmail.com","sentAt":"2018-02-13T15:06:15Z","receivedAt":"2018-02-13T15:06:22Z","isPatch":false,"sender":{"key":"sbazaios@gmail.com","avatar":null},"body":"Hi, I've came across an issue when using the git diff command. In\nparticular the diff is different to what the svn diff produces. While\nboth being correct the output of the svn diff is easier to understand\nthan the git diff one. See the following issue on github where I\ninitially reported the issue:\n\nhttps://github.com/git-for-windows/git/issues/1494\n\nI have Included a picture to better illustrate the problem. What do\nyou think? Is it possible to make git diff output similar to svn diff\nregarding this issue?\n"},{"id":"339201","messageId":"20180213160824.GA5203@sigill.intra.peff.net","threadId":"47827","inReplyTo":"CAOmC-AncOZfAwowXLjwDbL9Bdx1+=RbXsrd72LzEBd2W-=LAQg@mail.gmail.com","subject":"Re: make git diff output easier to read - use better diff heuristics","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-02-13T16:08:24Z","receivedAt":"2018-02-13T16:08:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 13, 2018 at 05:06:15PM +0200, Σπύρος Βαζαίος wrote:\n\n> Hi, I've came across an issue when using the git diff command. In\n> particular the diff is different to what the svn diff produces. While\n> both being correct the output of the svn diff is easier to understand\n> than the git diff one. See the following issue on github where I\n> initially reported the issue:\n> \n> https://github.com/git-for-windows/git/issues/1494\n> \n> I have Included a picture to better illustrate the problem. What do\n> you think? Is it possible to make git diff output similar to svn diff\n> regarding this issue?\n\nTry \"git diff --no-indent-heuristic\", which makes your example look\nbetter. Here's a quick reproduction:\n\n-- >8 --\ncat >foo.c <<\\EOF\nstatic struct foo bar[] = {\n#ifdef SOMETHING\n\t{ \"stats.info\", MNU_GBX_FSTAINF, etc },\n\t{ \"expired.info\", MNU_GBX_FSTAINF, etc },\n\t{ \"info.log\", MNU_GBX_INFOLOG, etc },\n#endif\n\t{ NULL, 0, 0 },\n};\nEOF\n\nsed '6a\\\n#ifdef WITH_EMU\\\n\t{ \"SoftCam.Key\", MNU_CFG_FSOFTCAMKEY, etc },\\\n#endif\n' <foo.c >bar.c\n-- 8< --\n\nNow this looks ugly:\n\n  git diff --no-index foo.c bar.c\n\nbut this does not:\n\n  git diff --no-index --no-indent-heuristic foo.c bar.c\n\nThat heuristic is described in 433860f3d0 (diff: improve positioning of\nadd/delete blocks in diffs, 2016-09-05). I'm not sure exactly why it\ndoes the wrong thing here, or if it would be possible to tweak the\nweighting factors to make it work.\n\n-Peff\n"},{"id":"339211","messageId":"CAGZ79kYXStMQCxnVjpV7n7miZEDAw4moR+0JksgTaRqHJwgSqw@mail.gmail.com","threadId":"47827","inReplyTo":"20180213160824.GA5203@sigill.intra.peff.net","subject":"Re: make git diff output easier to read - use better diff heuristics","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-02-13T18:11:49Z","receivedAt":"2018-02-13T18:11:56Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Feb 13, 2018 at 8:08 AM, Jeff King <peff@peff.net> wrote:\n> On Tue, Feb 13, 2018 at 05:06:15PM +0200, Σπύρος Βαζαίος wrote:\n>\n>> Hi, I've came across an issue when using the git diff command. In\n>> particular the diff is different to what the svn diff produces. While\n>> both being correct the output of the svn diff is easier to understand\n>> than the git diff one. See the following issue on github where I\n>> initially reported the issue:\n>>\n>> https://github.com/git-for-windows/git/issues/1494\n>>\n>> I have Included a picture to better illustrate the problem. What do\n>> you think? Is it possible to make git diff output similar to svn diff\n>> regarding this issue?\n>\n> Try \"git diff --no-indent-heuristic\", which makes your example look\n> better. Here's a quick reproduction:\n>\n> -- >8 --\n> cat >foo.c <<\\EOF\n> static struct foo bar[] = {\n> #ifdef SOMETHING\n>         { \"stats.info\", MNU_GBX_FSTAINF, etc },\n>         { \"expired.info\", MNU_GBX_FSTAINF, etc },\n>         { \"info.log\", MNU_GBX_INFOLOG, etc },\n> #endif\n>         { NULL, 0, 0 },\n> };\n> EOF\n>\n> sed '6a\\\n> #ifdef WITH_EMU\\\n>         { \"SoftCam.Key\", MNU_CFG_FSOFTCAMKEY, etc },\\\n> #endif\n> ' <foo.c >bar.c\n> -- 8< --\n>\n> Now this looks ugly:\n>\n>   git diff --no-index foo.c bar.c\n>\n> but this does not:\n>\n>   git diff --no-index --no-indent-heuristic foo.c bar.c\n>\n> That heuristic is described in 433860f3d0 (diff: improve positioning of\n> add/delete blocks in diffs, 2016-09-05). I'm not sure exactly why it\n> does the wrong thing here, or if it would be possible to tweak the\n> weighting factors to make it work.\n\nhttps://github.com/git/git/commit/433860f3d0\n\nI would think that the long lines at the shift boundaries[1] have\nbad impact on the algorithm as they are of different length\nand influence the backstepping value. (vague theory)\n\nI wonder if we want to add language specifics to the heuristic,\nsuch that '}' or 'end' in a given line have a strong signal that the\ncut should be right after that line? i.e. that would decrease the\nbadness score.\n\nWhile this is not a general solution (but quite language specific),\nI would think this is still a good idea to look at.\n(When coming up with the heuristic, most people thought the bad\nexamples would come from exotic languages, but not C, which we\nare all so familiar with such that we thought we had a good grasp\nhow to make a heuristic that works with C. Note that the example\nlooks like C, though).\n\n[1]\n{ \"info.log\", MNU_GBX_INFOLOG, etc },\nvs\n{ \"SoftCam.Key\", MNU_CFG_FSOFTCAMKEY, etc },\\\n\n\n(slightly unrelated tangent on better diffs in general:)\nA couple days ago I had another discussion on how\ndiffs are best presented to users. That discussion was\nfocused on a slightly higher layer, the diff algorithm itself,\ninstead of a post-algorithm boost-fixup.\n\nIt is usually easier to both write as well as review code that\nadds completely new features or projects instead of squashing\nbugs or integrating subtle new features in an existing code base.\n\nThis observation lead to the conclusion that reviewing large\nlocks of adjacent new lines is easier than reviewing blocks\nwhere deletions as well as additions are found. So we theorized\nthat deletions should have more impact than additions when\ncomputing the diff itself. The case presented here has no deletions\nso this is not applicable, but most examples that came up once\n2477ab2ea8 (diff: support anchoring line(s), 2017-11-27)\nwas discussed would be likely to have better diffs.\n\nStefan\n"},{"id":"339213","messageId":"CAOmC-AnXD=eSphS=TK4v72tSYDYEV2cgu_EsP4kgNGR=7yre8w@mail.gmail.com","threadId":"47827","inReplyTo":"CAGZ79kYXStMQCxnVjpV7n7miZEDAw4moR+0JksgTaRqHJwgSqw@mail.gmail.com","subject":"Re: make git diff output easier to read - use better diff heuristics","fromName":"Σπύρος Βαζαίος","fromEmail":"sbazaios@gmail.com","sentAt":"2018-02-13T18:25:55Z","receivedAt":"2018-02-13T18:26:02Z","isPatch":false,"sender":{"key":"sbazaios@gmail.com","avatar":null},"body":"Hi guys,\n\nWhile I din't have the experience to express an opinion on this\nmatter, I have to say that the --no-indent-heuristic that Jeff\nsuggested worked great.\nThere were more than a handful of cases that this issue happened in my\ndiff file (all were the same: #endif followed by #ifdef).\nOh, and the language is C indeed.\n\nThanks for the great support!\n\nOn Tue, Feb 13, 2018 at 8:11 PM, Stefan Beller <sbeller@google.com> wrote:\n> On Tue, Feb 13, 2018 at 8:08 AM, Jeff King <peff@peff.net> wrote:\n>> On Tue, Feb 13, 2018 at 05:06:15PM +0200, Σπύρος Βαζαίος wrote:\n>>\n>>> Hi, I've came across an issue when using the git diff command. In\n>>> particular the diff is different to what the svn diff produces. While\n>>> both being correct the output of the svn diff is easier to understand\n>>> than the git diff one. See the following issue on github where I\n>>> initially reported the issue:\n>>>\n>>> https://github.com/git-for-windows/git/issues/1494\n>>>\n>>> I have Included a picture to better illustrate the problem. What do\n>>> you think? Is it possible to make git diff output similar to svn diff\n>>> regarding this issue?\n>>\n>> Try \"git diff --no-indent-heuristic\", which makes your example look\n>> better. Here's a quick reproduction:\n>>\n>> -- >8 --\n>> cat >foo.c <<\\EOF\n>> static struct foo bar[] = {\n>> #ifdef SOMETHING\n>>         { \"stats.info\", MNU_GBX_FSTAINF, etc },\n>>         { \"expired.info\", MNU_GBX_FSTAINF, etc },\n>>         { \"info.log\", MNU_GBX_INFOLOG, etc },\n>> #endif\n>>         { NULL, 0, 0 },\n>> };\n>> EOF\n>>\n>> sed '6a\\\n>> #ifdef WITH_EMU\\\n>>         { \"SoftCam.Key\", MNU_CFG_FSOFTCAMKEY, etc },\\\n>> #endif\n>> ' <foo.c >bar.c\n>> -- 8< --\n>>\n>> Now this looks ugly:\n>>\n>>   git diff --no-index foo.c bar.c\n>>\n>> but this does not:\n>>\n>>   git diff --no-index --no-indent-heuristic foo.c bar.c\n>>\n>> That heuristic is described in 433860f3d0 (diff: improve positioning of\n>> add/delete blocks in diffs, 2016-09-05). I'm not sure exactly why it\n>> does the wrong thing here, or if it would be possible to tweak the\n>> weighting factors to make it work.\n>\n> https://github.com/git/git/commit/433860f3d0\n>\n> I would think that the long lines at the shift boundaries[1] have\n> bad impact on the algorithm as they are of different length\n> and influence the backstepping value. (vague theory)\n>\n> I wonder if we want to add language specifics to the heuristic,\n> such that '}' or 'end' in a given line have a strong signal that the\n> cut should be right after that line? i.e. that would decrease the\n> badness score.\n>\n> While this is not a general solution (but quite language specific),\n> I would think this is still a good idea to look at.\n> (When coming up with the heuristic, most people thought the bad\n> examples would come from exotic languages, but not C, which we\n> are all so familiar with such that we thought we had a good grasp\n> how to make a heuristic that works with C. Note that the example\n> looks like C, though).\n>\n> [1]\n> { \"info.log\", MNU_GBX_INFOLOG, etc },\n> vs\n> { \"SoftCam.Key\", MNU_CFG_FSOFTCAMKEY, etc },\\\n>\n>\n> (slightly unrelated tangent on better diffs in general:)\n> A couple days ago I had another discussion on how\n> diffs are best presented to users. That discussion was\n> focused on a slightly higher layer, the diff algorithm itself,\n> instead of a post-algorithm boost-fixup.\n>\n> It is usually easier to both write as well as review code that\n> adds completely new features or projects instead of squashing\n> bugs or integrating subtle new features in an existing code base.\n>\n> This observation lead to the conclusion that reviewing large\n> locks of adjacent new lines is easier than reviewing blocks\n> where deletions as well as additions are found. So we theorized\n> that deletions should have more impact than additions when\n> computing the diff itself. The case presented here has no deletions\n> so this is not applicable, but most examples that came up once\n> 2477ab2ea8 (diff: support anchoring line(s), 2017-11-27)\n> was discussed would be likely to have better diffs.\n>\n> Stefan\n"},{"id":"339238","messageId":"CAMy9T_FHhXMJTTHR3jg-SGab-1Rgy3FScMxeCUcUjnPfCZEJiw@mail.gmail.com","threadId":"47827","inReplyTo":"CAOmC-AnXD=eSphS=TK4v72tSYDYEV2cgu_EsP4kgNGR=7yre8w@mail.gmail.com","subject":"Re: make git diff output easier to read - use better diff heuristics","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2018-02-13T22:41:10Z","receivedAt":"2018-02-13T22:41:19Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On Tue, Feb 13, 2018 at 7:25 PM, Σπύρος Βαζαίος <sbazaios@gmail.com> wrote:\n> While I din't have the experience to express an opinion on this\n> matter, I have to say that the --no-indent-heuristic that Jeff\n> suggested worked great.\n> There were more than a handful of cases that this issue happened in my\n> diff file (all were the same: #endif followed by #ifdef).\n> Oh, and the language is C indeed.\n\nThe \"indent heuristic\" algorithm that Git now uses by default is\nnothing more than that—a heuristic—so it can be fooled. It bases its\ndecision on the locations of blank lines and the indentations of\nnon-blank lines. In the vast majority of cases it gives the same or\nbetter results than the old algorithm, but there are some cases, like\nyours, where it gives aesthetically less pleasing (though still\ncorrect) results.\n\nThe algorithm usually handles C code well, but it tends to be confused\nby preprocessor directives, because they are not indented like typical\ncode. It might be possible to tweak the weights to get it to handle\npreprocessor directives better, but that causes it to do worse on\nother, more common things like Python code (where blocks are preceded\nbut not followed by a line with lesser indentation).\n\nDoing significantly better probably would require some amount of\nlanguage-awareness, but that's a bigger job than I was willing to take\non.\n\nMichael\n"}]}