{"thread":{"id":"5088","subject":"kompare won't parse git diffs","startedAt":"2006-08-02T10:07:39Z","lastAt":"2006-08-06T03:14:33Z","messageCount":9,"participants":["Andy Parkins","Linus Torvalds","Jakub Narebski","Junio C Hamano","Paul Eggert"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"24556","messageId":"200608021107.43485.andyparkins@gmail.com","threadId":"5088","inReplyTo":null,"subject":"kompare won't parse git diffs","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2006-08-02T10:07:39Z","receivedAt":"2006-08-02T10:07:39Z","isPatch":false,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"Hello,\n\nKompare just shows blank for diffs redirected from git. This makes it \n\nI've tracked down the cause and it seems to be on the file declaration lines. \nHere is a sample diff that kompare will not show.\n\ndiff --git a/file b/file\nindex a8b2ec6..a1e65fd 100644\n--- a/file\n+++ b/file\n@@ -1,2 +1,3 @@\n initial contents goes in master branch\n additional content added in repo1#master\n+blah\n\nAs you can see, it's a simple addition of the line \"blah\". Kompare shows this \ndiff as blank. Now if I modify the file so that the \"---\" and \"+++\" lines \nboth have \"(something)\" added:\n\ndiff --git a/file b/file\nindex a8b2ec6..a1e65fd 100644\n--- a/file (anything can go here)\n+++ b/file (anything can go here)\n@@ -1,2 +1,3 @@\n initial contents goes in master branch\n additional content added in repo1#master\n+blah\n\nhttp://bugs.kde.org/show_bug.cgi?id=131717\n\nI've posted a bug for kompare, but thought that maybe it would be handy if git \noutputted those bracketed comments anyway - I notice that \"diff -u\" from the \ncommand line does (although it uses the timestamp of the file). Maybe the \nobject hash of the files being compared could go here?\n\n\nAndy\n\n-- \nDr Andy Parkins, M Eng (hons), MIEE\nandyparkins@gmail.com\n"},{"id":"24572","messageId":"Pine.LNX.4.64.0608021006150.4168@g5.osdl.org","threadId":"5088","inReplyTo":"200608021107.43485.andyparkins@gmail.com","subject":"Re: kompare won't parse git diffs","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-08-02T17:19:50Z","receivedAt":"2006-08-02T17:19:50Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 2 Aug 2006, Andy Parkins wrote:\n>\n> Kompare just shows blank for diffs redirected from git. [ snip ]\n> \n> As you can see, it's a simple addition of the line \"blah\". Kompare shows this \n> diff as blank. Now if I modify the file so that the \"---\" and \"+++\" lines \n> both have \"(something)\" added:\n\nI'd definitely call this a pure kompare bug.\n\nNot only is the git patch format perfectly standard and accepted by other \ntools, it's much better designed than the brain-damaged syntax that GNU \npatch uses (which adds a tab and a timestamp after the filenames). In \nparticular, with git patches it is easy to get filenames that have spaces \nand tabs in them right. \n\nNow, if the kompare people can show that every single other patch \ngenerator adds the stupid tab + date format, I guess we could do it too, \nbut\n (a) there is no valid date in general to use, so it's a fundamentally \n broken notion and\n (b) I'm pretty sure that the kompare people only ever actually tested \n with GNU patch or other very modern patches, because when I did the \n patch apply logic (and designed the extended git format), I looked \n around at things like \"diffstat\" that have been around a long time, \n to see what they accept, and the whole thing is an unholy mess wrt \n filenames.\n\nThe git format really is the best patch format I've seen by _far_, partly \nbecause it's designed to be totally unambiguous even in the presense of \nfile renames (and new/deleted files, and file modes), but partly because \nit's also designed to be both extensible and detectable (ie the marker \n\"diff --git \" is there, so that you can _know_ and _depend_ on the git \nformat, unlike, for example, the GNU patches that don't have a good fixed \nformat).\n\nI'm hoping that people will some day just wake up and notice that the git \nextended patches are really worth doing even for other projects. I was \ngoing to send in patches to GNU patch to try to make it at least \nunderstand them, but I got lazy.\n\n\t\tLinus\n"},{"id":"24575","messageId":"eaqpt2$ots$1@sea.gmane.org","threadId":"5088","inReplyTo":"Pine.LNX.4.64.0608021006150.4168@g5.osdl.org","subject":"Re: kompare won't parse git diffs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-08-02T18:12:07Z","receivedAt":"2006-08-02T18:12:07Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Linus Torvalds wrote:\n\n> On Wed, 2 Aug 2006, Andy Parkins wrote:\n>>\n>> Kompare just shows blank for diffs redirected from git. [ snip ]\n>> \n>> As you can see, it's a simple addition of the line \"blah\". Kompare shows this \n>> diff as blank. Now if I modify the file so that the \"---\" and \"+++\" lines \n>> both have \"(something)\" added:\n> \n> I'd definitely call this a pure kompare bug.\n> \n> Not only is the git patch format perfectly standard and accepted by other \n> tools, it's much better designed than the brain-damaged syntax that GNU \n> patch uses (which adds a tab and a timestamp after the filenames). In \n> particular, with git patches it is easy to get filenames that have spaces \n> and tabs in them right. \n\nWhat about filenames with end-of-line character in them? Is it quoted?\n\nBTW. It should be not that hard to get filename with spaces and tabs even\nin GNU diff format: everything up to last is filename.\n\n> Now, if the kompare people can show that every single other patch \n> generator adds the stupid tab + date format, I guess we could do it too, \n> but\n> (a) there is no valid date in general to use, so it's a fundamentally \n> broken notion and\n\nMeaning we don't save timestamp in git ;-) Well, we could use date of the \ncommit which created given file contents (first commit from root, or last\nfrom head which contains given version)... but the same contents might be\nintroduced independently in different commits. And different clones of the\nsame repository might have different commit dates...\n\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"24577","messageId":"200608021919.02511.andyparkins@gmail.com","threadId":"5088","inReplyTo":"Pine.LNX.4.64.0608021006150.4168@g5.osdl.org","subject":"Re: kompare won't parse git diffs","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2006-08-02T18:18:58Z","receivedAt":"2006-08-02T18:18:58Z","isPatch":false,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Wednesday 2006, August 02 18:19, Linus Torvalds wrote:\n\n> I'd definitely call this a pure kompare bug.\n\nMe too, but it seemed such a small thing change to git that I thought I'd \nmention it.\n\n> Not only is the git patch format perfectly standard and accepted by other\n> tools, it's much better designed than the brain-damaged syntax that GNU\n> patch uses (which adds a tab and a timestamp after the filenames). In\n> particular, with git patches it is easy to get filenames that have spaces\n> and tabs in them right.\n\nThe only three things I checked were diff, git and subversion. git is the \nonly one of the three that didn't include anything after the filenames. \nSubversion uses the space to write (working copy) and (revision XXX), so it's \nclear that these fields are simply being treated as commentary. That only \nadds weight to the argument that it is a kompare bug, as surely is a \nperfectly acceptable comment?\n\n> Now, if the kompare people can show that every single other patch\n> generator adds the stupid tab + date format, I guess we could do it too,\n> but\n\nAs it seems to be acceptable to put something other than a date, perhaps the \nhash of the object being compared would be more useful than the unavailable \ndate?\n\n> (b) I'm pretty sure that the kompare people only ever actually tested\n> with GNU patch or other very modern patches, because when I did the\n\nI'm sure that that is true. I certainly don't think that there is a fight \nhere, I've not seen any feedback on the kompare bug yet, but I'd be very \nsurprised if the response is \"git is stupid - WONTFIX\".\n\n> I'm hoping that people will some day just wake up and notice that the git\n> extended patches are really worth doing even for other projects. I was\n\nI think in the cases I've been testing, the extended format hasn't shown its \nface. I used git-svnimport to duplicate an svn repository then copied the \nsame changes into both an svn working directory and a git working directory. \nI then ran \"git diff\" and \"svn diff\", and diffed the two outputs. Apart from \nsome differences in how many lines where added and deleted (and the \ndifference that started this thread of course) the two diffs were equivalent.\n\n\nAndy\n\n-- \nDr Andrew Parkins, M Eng (Hons), AMIEE\nandyparkins@gmail.com\n"},{"id":"24581","messageId":"7vy7u7t0ap.fsf@assigned-by-dhcp.cox.net","threadId":"5088","inReplyTo":"eaqpt2$ots$1@sea.gmane.org","subject":"Re: kompare won't parse git diffs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-02T19:17:18Z","receivedAt":"2006-08-02T19:17:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski writes:\n\n>> Not only is the git patch format perfectly standard and accepted by other \n>> tools, it's much better designed than the brain-damaged syntax that GNU \n>> patch uses (which adds a tab and a timestamp after the filenames). In \n>> particular, with git patches it is easy to get filenames that have spaces \n>> and tabs in them right. \n>\n> What about filenames with end-of-line character in them? Is it quoted?\n\nYou could have done a bit of homework yourself to find that out\neasily ;-). Anyway, the answer is yes.\n\n\thttp://marc.theaimsgroup.com/?l=git&m=112927316408690\n\nI wonder what happened to the plan to update GNU diff/patch to\nalso emit/understand the c-style quoted paths. Paul?\n"},{"id":"24582","messageId":"Pine.LNX.4.64.0608021207360.4168@g5.osdl.org","threadId":"5088","inReplyTo":"eaqpt2$ots$1@sea.gmane.org","subject":"Re: kompare won't parse git diffs","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-08-02T19:19:31Z","receivedAt":"2006-08-02T19:19:31Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 2 Aug 2006, Jakub Narebski wrote:\n> \n> What about filenames with end-of-line character in them? Is it quoted?\n\nI would _not_ guarantee that git handles filenames with newlines in them \ncorrectly in all cases (I won't guarantee tabs either, but the likelihood \nis much higher ;). But yes, \"git diff\" does handle it right.\n\nThat said, yes, it should be quoted. A quoted filename in \"git diff\" not \nonly changes a newline into the normal quoted sequence (ie \"\\n\"), but will \nalso put the whole filename in quotes.\n\nSo if you have a file called \"hello\", the diff headers will say\n\n\t--- a/hello\n\t+++ b/hello\n\nbut if you have a file with an embedded newline, it would look something \nlike\n\n\t--- \"a/hello\\nthere\"\n\t+++ \"b/hello\\nthere\"\n\n(where a '\"' in a filename causes the same quoting to take effect, so if \nyou haev embedded double-quotes in a filename, it would be shown as \nsomething like\n\n\t--- \"a/embedded\\\"quote\"\n\t+++ \"b/embedded\\\"quote\"\n\ninstead).\n\n> BTW. It should be not that hard to get filename with spaces and tabs even\n> in GNU diff format: everything up to last is filename.\n\nThat's true _if_ you know that the patch was generated by GNU diff, but \nsince you can't know that (since GNU diff doesn't add any markers of its \nown), you cannot know if it's a patch generated by an old version of diff \n(for a filename with a tab inside of it) or if it's a filename with the \nGNU date extensions.\n\n> > Now, if the kompare people can show that every single other patch \n> > generator adds the stupid tab + date format, I guess we could do it too, \n> > but\n> > (a) there is no valid date in general to use, so it's a fundamentally \n> > broken notion and\n> \n> Meaning we don't save timestamp in git ;-) Well, we could use date of the \n> commit which created given file contents (first commit from root, or last\n> from head which contains given version)... but the same contents might be\n> introduced independently in different commits. And different clones of the\n> same repository might have different commit dates...\n\nExactly, a \"date\" in _any_ distributed SCM makes no sense what-so-ever. \nWhat happens across a merge? Which date do you take? Do you follow the \nthing down and basically do a full \"annotate\" on the file?\n\nThe fact is, dates in SCM diffs are insane and stupid. They do not make \nsense in the presense of an SCM, and they only make sense on individual \n_files_ (and quite limited there too, but at least it has _some_ meaning).\n\nIn the SCM, the _changes_ may have timestamps, but the files sure as hell \nshould not. Of course, a file-based SCM easily gets confused about these \nthings (eg in CVS, there is very much a 1:1 relationship between files and \nchanges, so in the CVS mindset it can make sense - but that's because CVS \nis stupid to begin with, and the problem really goes much deeper than \ntimestamps on the diffs).\n\n\t\t\tLinus\n"},{"id":"24587","messageId":"ear19d$j9p$1@sea.gmane.org","threadId":"5088","inReplyTo":"Pine.LNX.4.64.0608021207360.4168@g5.osdl.org","subject":"Re: kompare won't parse git diffs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-08-02T20:18:19Z","receivedAt":"2006-08-02T20:18:19Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Linus Torvalds wrote:\n\n> On Wed, 2 Aug 2006, Jakub Narebski wrote:\n>\n>> > Now, if the kompare people can show that every single other patch \n>> > generator adds the stupid tab + date format, I guess we could do it too, \n>> > but\n>> > (a) there is no valid date in general to use, so it's a fundamentally \n>> > broken notion and\n>> \n>> Meaning we don't save timestamp in git ;-) Well, we could use date of the \n>> commit which created given file contents (first commit from root, or last\n>> from head which contains given version)... but the same contents might be\n>> introduced independently in different commits. And different clones of the\n>> same repository might have different commit dates...\n> \n> Exactly, a \"date\" in _any_ distributed SCM makes no sense what-so-ever. \n> What happens across a merge? Which date do you take? Do you follow the \n> thing down and basically do a full \"annotate\" on the file?\n> \n> The fact is, dates in SCM diffs are insane and stupid. They do not make \n> sense in the presense of an SCM, and they only make sense on individual \n> _files_ (and quite limited there too, but at least it has _some_ meaning).\n\nWhat about putting e.g. sha1 (or abbreviated sha1) of blob if file exist \nin repository, \"(in index)\" or \"(working area)\" in two other cases, instead\nof date or file version?\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"24598","messageId":"Pine.LNX.4.64.0608021349580.4168@g5.osdl.org","threadId":"5088","inReplyTo":"ear19d$j9p$1@sea.gmane.org","subject":"Re: kompare won't parse git diffs","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-08-02T20:51:34Z","receivedAt":"2006-08-02T20:51:34Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 2 Aug 2006, Jakub Narebski wrote:\n> \n> What about putting e.g. sha1 (or abbreviated sha1) of blob if file exist \n> in repository, \"(in index)\" or \"(working area)\" in two other cases, instead\n> of date or file version?\n\nQuite frankly, it will just break the current git header standard, and \nmake diffs look uglier.\n\nJust get kompare fixed instead. If you want to use it with git, you would \nreally want it to know about all the other git diff extensions too anyway, \nand right now it's _buggy_ for requiring a tab where none needs to be.\n\nIf \"patch\" doesn't want the tab, then dammit, neither should kompare.\n\n\t\t\tLinus\n"},{"id":"24868","messageId":"87fygao8rq.fsf@penguin.cs.ucla.edu","threadId":"5088","inReplyTo":"7vy7u7t0ap.fsf@assigned-by-dhcp.cox.net","subject":"Re: kompare won't parse git diffs","fromName":"Paul Eggert","fromEmail":"eggert@cs.ucla.edu","sentAt":"2006-08-06T03:14:33Z","receivedAt":"2006-08-06T03:14:33Z","isPatch":false,"sender":{"key":"eggert@cs.ucla.edu","avatar":"https://avatars.githubusercontent.com/u/572024?v=4"},"body":"Junio C Hamano writes:\n\n> I wonder what happened to the plan to update GNU diff/patch to\n> also emit/understand the c-style quoted paths. Paul?\n\nIt's still intended, but not yet implemented unfortunately.\n"}]}