{"thread":{"id":"634","subject":"[PATCH 2/4] Tweak diff output further to make it a bit less distracting.","startedAt":"2005-05-15T21:19:50Z","lastAt":"2005-05-18T16:10:47Z","messageCount":19,"participants":["Junio C Hamano","Petr Baudis","Linus Torvalds","Daniel Barkalow","Matthias Urlichs"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"3390","messageId":"7vvf5kqj9l.fsf@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":null,"subject":"[PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-15T21:19:50Z","receivedAt":"2005-05-15T21:19:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adds an newline between each diff. Also change \"#mode : \"\nstring, which was misleading in that we are not showing just\nmode when we talk about a file changing into a symlink.\n\nSigned-off-by: Junio C Hamano \n---\n\ndiff.c | 18 ++++++++++--------\nt/t4000-diff-format.sh | 6 ++++--\n2 files changed, 14 insertions(+), 10 deletions(-)\n\n--- a/diff.c\n+++ b/diff.c\n@@ -83,7 +83,7 @@\n \t\t\t struct diff_tempfile *temp)\n {\n \tint i, next_at;\n-\tconst char *git_prefix = \"# mode: \";\n+\tconst char *git_prefix = \"\\n@. \";\n \tconst char *diff_cmd = \"diff -L'%s%s' -L'%s%s'\";\n \tconst char *diff_arg = \"'%s' '%s'||:\"; /* \"||:\" is to return 0 */\n \tconst char *input_name_sq[2];\n@@ -128,15 +128,17 @@\n \telse if (!path1[1][0])\n \t\tprintf(\"%s%s . %s\\n\", git_prefix, temp[0].mode, name);\n \telse {\n-\t\tif (strcmp(temp[0].mode, temp[1].mode))\n+\t\tif (strcmp(temp[0].mode, temp[1].mode)) {\n \t\t\tprintf(\"%s%s %s %s\\n\", git_prefix,\n \t\t\t temp[0].mode, temp[1].mode, name);\n-\n-\t\tif (strncmp(temp[0].mode, temp[1].mode, 3))\n-\t\t\t/* we do not run diff between different kind\n-\t\t\t * of objects.\n-\t\t\t */\n-\t\t\texit(0);\n+\t\t\tif (strncmp(temp[0].mode, temp[1].mode, 3))\n+\t\t\t\t/* we do not run diff between different kind\n+\t\t\t\t * of objects.\n+\t\t\t\t */\n+\t\t\t\texit(0);\n+\t\t}\n+\t\telse\n+\t\t\tputchar('\\n');\n \t}\n \tfflush(NULL);\n \texeclp(\"/bin/sh\",\"sh\", \"-c\", cmd, NULL);\n--- a/t/t4000-diff-format.sh\n+++ b/t/t4000-diff-format.sh\n@@ -26,7 +26,8 @@\n 'git-diff-files -p after editing work tree.' \\\n 'git-diff-files -p >current'\n cat >expected <<\\EOF\n-# mode: 100644 100755 path0\n+\n+@. 100644 100755 path0\n --- a/path0\n +++ b/path0\n @@ -1,3 +1,3 @@\n@@ -34,7 +35,8 @@\n Line 2\n -line 3\n +Line 3\n-# mode: 100755 . path1\n+\n+@. 100755 . path1\n --- a/path1\n +++ /dev/null\n @@ -1,3 +0,0 @@\n\n"},{"id":"3420","messageId":"20050516220559.GC8609@pasky.ji.cz","threadId":"634","inReplyTo":"7vvf5kqj9l.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-16T22:05:59Z","receivedAt":"2005-05-16T22:05:59Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sun, May 15, 2005 at 11:19:50PM CEST, I got a letter\nwhere Junio C Hamano told me that...\n> Adds an newline between each diff. Also change \"#mode : \"\n> string, which was misleading in that we are not showing just\n> mode when we talk about a file changing into a symlink.\n> \n> Signed-off-by: Junio C Hamano \n\nSo, I've been looking at the output, and I have to admit that I'm still\nnot too happy with it (I know I'm horrible). It turned out to be rather\nconfusing, since there are normally no blank lines in the middle of the\ndiffs, so it looked as the blank lines were actually part of the diffed\nfiles.\n\nWhat about just throwing away the newlines and just passing '@.'?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"3427","messageId":"7vsm0mn5s1.fsf@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":"20050516220559.GC8609@pasky.ji.cz","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-16T22:51:42Z","receivedAt":"2005-05-16T22:51:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis writes:\n\nPB> What about just throwing away the newlines and just passing '@.'?\n\nLet's just drop the patch altogether unless anybody else has\nbetter justification and pressing needs.\n\nThe current one is tolerable, except I do not like the word\n\"mode\" very much.\n\n --- a/frotz\n +++ b/frotz\n @@ xxx @@\n + asdfasdf\n # mode: 100644 100755 nitfol\n --- a/nitfol\n +++ b/nitfol\n @@ yyy @@\n - asdfasdf\n + asdfasdfasdf\n --- a/rezrov\n +++ b/rezrov\n @@ zzz @@\n ...\n\nThis is what we would have got with the patch, which as you say\ngives an illusion as if there should exist an empty line in\n\"frotz\" and \"nitfol\", after the lines the hunks are applied to.\nI should not have pushed it to begin with.\n\n --- a/frotz\n +++ b/frotz\n @@ xxx @@\n + asdfasdf\n\n @. 100644 100755 nitfol\n --- a/nitfol\n +++ b/nitfol\n @@ yyy @@\n - asdfasdf\n + asdfasdfasdf\n\n --- a/rezrov\n +++ b/rezrov\n @@ zzz @@\n ...\n\n\n"},{"id":"3431","messageId":"Pine.LNX.4.58.0505161556260.18337@ppc970.osdl.org","threadId":"634","inReplyTo":"7vsm0mn5s1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-16T23:28:31Z","receivedAt":"2005-05-16T23:28:31Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 16 May 2005, Junio C Hamano wrote:\n\n> # mode: 100644 100755 nitfol\n> --- a/nitfol\n> +++ b/nitfol\n\n> @. 100644 100755 nitfol\n> --- a/nitfol\n> +++ b/nitfol\n\nI have to say, I muct prefer the first over the second.\n\nI don't know why people think \"line noise\" means \"computer readable\". To\nme, \"@.\" look slike line noise, and worse, it's clearly _less_\ndisambiguous than spelling out \"mode\". The fact that it (on purpose, I\nassume) looks somewhat like the \"@@\" beginning of a patch hunk for the\n_previous_ file makes it even worse.\n\nTerseness is filne, and thus maybe it could be just\n\n\t# 100644 100755 nitfol\n\nbut on the other hand that doesn't really have any advantages either. \nExcept being better than the inexplicable \"@.\"\n\nMe, I'd prefer making it clear whether the file is \"new\", \"removed\" or\n\"changed\". That's really what matters, and at least the \"new\" case does\nneed the mode (while the \"removed\" case does not). So instead of talking\nabout \"mode\", which is largely irrelevant (the important thing is to\nindicate whether it's new or old), we should talk about how the file has\nchanged.\n\nThere's another issue I noticed with the current default 'diff' output: I \noften (almost always) want to have a way to go to \"next file\", which in \ntraditional diffs I just do with \"/^diff\" when paging with 'less'. The \ncurrent one doesn't have a way to do that - searching for '^--- ' comes \nclosest, but is ambiguous, since it might be a part of a _patch_ that \ncontains a line that got removed and that started with \"-- \".\n\nSo I'd actually prefer some output format that fixed that thing too, by \nhaving a header for each file.\n\nMaking the header be \"diff a/file b/file\" would make the thing look the \nsame as a regular diff, and that also makes it disambiguous (and thus easy \nfor machines to parse) to have a _second_ or third line for things like \nmode change information.\n\nSo my preferred format would actually be\n\n\tdiff -git a/filename b/filename\n\t\n\t--- a/filename\n\t+++ b/filename\n\t@@ -xx ....\n\nwhere the \"optional extended header lines\" would be very plain, like\n\n\told mode 100644\n\tnew mode 100755\n\nor\n\n\tnew file mode 100644\n\nor\n\n\tdeleted file mode 100644\n\nor something like that.\n\nIn fact anything _except_ for something that starts with \"-\" \"+\" \" \" or\n\"@\" which have special meanings inside diffs (whether you want the \"mode\"\nfor the deleted file case or not is up to you - it could be an added\nsanity check that the diff actually matches, but on the other hand there's\nreally nothing you could do anyway except warn if the mode didn't match,\nso..)\n\nThis makes it very easy to parse mechanically: look for a line that starts\nwith \"diff -git \" (which cannot be part of the \"meat\" of the patch), and\nthen you know that you've found the start of an extended patch.\n\nWhy the \"-git\"? A normal patch header already looks something like the \nfollowing (head of the pre-git 2.6.12-rc1 patch):\n\n\tdiff -Nru a/CREDITS b/CREDITS\n\t--- a/CREDITS 2005-03-17 17:35:10 -08:00\n\t+++ b/CREDITS 2005-03-17 17:35:10 -08:00\n\t@@ -34,8 +34,9 @@\n\t E: airlied@linux.ie\n\t W: http://www.csn.ul.ie/~airlied\n\t D: NFS over TCP patches\n\t-S: University of Limerick\n\t-S: Ireland\n\t+D: in-kernel DRM Maintainer\n\nso the \"-git\" thing there is equivalent to the \"-Nru\" part, telling how\nthe diff was generated, and being an added hint that we may have extended\nheaders before the actual patch (which wouldn't be sensible in a non-git\npatch).\n\nOne final note: I actually think that \"rename patches\" make a ton of \nsense, even if git itself doesn't track renames. If we ever have a \"smart \ndiff\" thing that can generate inter-file diffs, I'd like to eventually see\n\n\tdiff -git a/kernel/sched.c b/kernel/sched.c.old\n\trename kernel/sched.c kernel/sched.c.old\n\told mode 100644\n\tnew mode 100755\n\t--- a/kernel/sched.c\n\t+++ b/kernel/sched.c.old\n\t@@ -1,5 +1,5 @@\n\t /*\n\t- * kernel/sched.c\n\t+ * kernel/sched.c.old\n\t *\n\t * Kernel scheduler and related syscalls\n\t *\n\nNotice? We could have a mode change, a rename _and_ a content change, all\nat the same time under the same header. That's obviously a totally idiotic\nexample, but the point is that if we have a nice \"extended diff header\"\nsetup, the format is very easily able to accomodate things like this.\n\nAnd it's both human-readable _and_ automatically parseable. And my old \n\"/^diff \" thing would still work, and still find each new entry, so I'd \nnot have to teach my old fingers new tricks.\n\n(This is, btw, the reason for the format of \"git-diff-tree -v --stdin\", in \ncase anybody wondered. Take a look, and notice how piping the output to \n\"less\" and then doing \"/^diff-tree \" gives the expected results. Same \ndeal. Human-readable _and_ machine-parseable at the same time!).\n\n\t\tLinus\n"},{"id":"3434","messageId":"7vsm0mlosf.fsf@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505161556260.18337@ppc970.osdl.org","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-16T23:44:00Z","receivedAt":"2005-05-16T23:44:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> I have to say, I muct prefer the first over the second.\n\nLikewise.\n\nLT> ... (whether you want the \"mode\"\nLT> for the deleted file case or not is up to you - it could be an added\nLT> sanity check that the diff actually matches, but on the other hand there's\nLT> really nothing you could do anyway except warn if the mode didn't match,\nLT> so..)\n\nApplying patch in reverse comes to mind...\n\nI'd agree what you said about \"diff -git\" in the rest of your\nmessage makes the most sense.\n\n"},{"id":"3436","messageId":"Pine.LNX.4.21.0505161955340.30848-100000@iabervon.org","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505161556260.18337@ppc970.osdl.org","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-05-17T00:10:35Z","receivedAt":"2005-05-17T00:10:35Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 16 May 2005, Linus Torvalds wrote:\n\n> One final note: I actually think that \"rename patches\" make a ton of \n> sense, even if git itself doesn't track renames. If we ever have a \"smart \n> diff\" thing that can generate inter-file diffs, I'd like to eventually see\n> \n> \tdiff -git a/kernel/sched.c b/kernel/sched.c.old\n> \trename kernel/sched.c kernel/sched.c.old\n> \told mode 100644\n> \tnew mode 100755\n\nI'd like something like:\n\ndiff -git a/kernel/sched.c b/kernel/sched.c.old\nfilename -- kernel/sched.c\nfilename ++ kernel/sched.c.old\nmode -- 100644\nmode ++ 100755\n--- a/kernel/sched.c\n+++ b/kernel/sched.c.old\n@@ -1,5 +1,5 @@\n(etc.)\n\nbecause I actually start thinking of the two sides as \"-\" and \"+\", and I'd\nactually have to think about which is \"old\" and which is \"new\", and which\nway the \"rename\" line goes, and so forth. I'd actually be happier with\njust a \"mode -- 100644\" line for a deleted file, also. If I'm looking at a\npatch, and I read Makefile with '-' and '+' versions of the lists of\nobjects, and then get to a \"new file\" line, I have to think about it to\nassociate the '+' side with having the file and the '-' side with not\nhaving it.\n\n\t-Daniel\n*This .sig left intentionally blank*\n\n"},{"id":"3437","messageId":"7voebalniw.fsf_-_@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505161556260.18337@ppc970.osdl.org","subject":"[PATCH] Fix diff output take #3.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-17T00:11:19Z","receivedAt":"2005-05-17T00:11:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This implements the output format suggested by Linus in\n\n\nSigned-off-by: Junio C Hamano \n---\n\ndiff.c | 14 +++++++-------\nt/t4000-diff-format.sh | 7 +++++--\n2 files changed, 12 insertions(+), 9 deletions(-)\n\ndiff -git a/diff.c b/diff.c\n--- a/diff.c\n+++ b/diff.c\n@@ -83,7 +83,6 @@\n \t\t\t struct diff_tempfile *temp)\n {\n \tint i, next_at;\n-\tconst char *git_prefix = \"# mode: \";\n \tconst char *diff_cmd = \"diff -L'%s%s' -L'%s%s'\";\n \tconst char *diff_arg = \"'%s' '%s'||:\"; /* \"||:\" is to return 0 */\n \tconst char *input_name_sq[2];\n@@ -123,15 +122,16 @@\n \tnext_at += snprintf(cmd+next_at, cmd_size-next_at,\n \t\t\t diff_arg, input_name_sq[0], input_name_sq[1]);\n \n+\tprintf(\"diff -git a/%s b/%s\\n\", name, name);\n \tif (!path1[0][0])\n-\t\tprintf(\"%s. %s %s\\n\", git_prefix, temp[1].mode, name);\n+\t\tprintf(\"new file mode %s\\n\", temp[1].mode);\n \telse if (!path1[1][0])\n-\t\tprintf(\"%s%s . %s\\n\", git_prefix, temp[0].mode, name);\n+\t\tprintf(\"deleted file mode %s\\n\", temp[0].mode);\n \telse {\n-\t\tif (strcmp(temp[0].mode, temp[1].mode))\n-\t\t\tprintf(\"%s%s %s %s\\n\", git_prefix,\n-\t\t\t temp[0].mode, temp[1].mode, name);\n-\n+\t\tif (strcmp(temp[0].mode, temp[1].mode)) {\n+\t\t\tprintf(\"old mode %s\\n\", temp[0].mode);\n+\t\t\tprintf(\"new mode %s\\n\", temp[1].mode);\n+\t\t}\n \t\tif (strncmp(temp[0].mode, temp[1].mode, 3))\n \t\t\t/* we do not run diff between different kind\n \t\t\t * of objects.\ndiff -git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh\n--- a/t/t4000-diff-format.sh\n+++ b/t/t4000-diff-format.sh\n@@ -26,7 +26,9 @@\n 'git-diff-files -p after editing work tree.' \\\n 'git-diff-files -p >current'\n cat >expected <<\\EOF\n-# mode: 100644 100755 path0\n+diff -git a/path0 b/path0\n+old mode 100644\n+new mode 100755\n --- a/path0\n +++ b/path0\n @@ -1,3 +1,3 @@\n@@ -34,7 +36,8 @@\n Line 2\n -line 3\n +Line 3\n-# mode: 100755 . path1\n+diff -git a/path1 b/path1\n+deleted file mode 100755\n --- a/path1\n +++ /dev/null\n @@ -1,3 +0,0 @@\n------------------------------------------------\n\n"},{"id":"3442","messageId":"20050517070158.GA10031@pasky.ji.cz","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505161556260.18337@ppc970.osdl.org","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-17T07:01:58Z","receivedAt":"2005-05-17T07:01:58Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, May 17, 2005 at 01:28:31AM CEST, I got a letter\nwhere Linus Torvalds told me that...\n> \n> \n> On Mon, 16 May 2005, Junio C Hamano wrote:\n> \n> > # mode: 100644 100755 nitfol\n> > --- a/nitfol\n> > +++ b/nitfol\n> \n> > @. 100644 100755 nitfol\n> > --- a/nitfol\n> > +++ b/nitfol\n> \n> I have to say, I muct prefer the first over the second.\n\nGlad. :-)\n\n> One final note: I actually think that \"rename patches\" make a ton of \n> sense, even if git itself doesn't track renames. If we ever have a \"smart \n> diff\" thing that can generate inter-file diffs, I'd like to eventually see\n> \n> \tdiff -git a/kernel/sched.c b/kernel/sched.c.old\n> \trename kernel/sched.c kernel/sched.c.old\n> \told mode 100644\n> \tnew mode 100755\n> \t--- a/kernel/sched.c\n> \t+++ b/kernel/sched.c.old\n> \t@@ -1,5 +1,5 @@\n> \t /*\n> \t- * kernel/sched.c\n> \t+ * kernel/sched.c.old\n> \t *\n> \t * Kernel scheduler and related syscalls\n> \t *\n> \n> Notice? We could have a mode change, a rename _and_ a content change, all\n> at the same time under the same header. That's obviously a totally idiotic\n> example, but the point is that if we have a nice \"extended diff header\"\n> setup, the format is very easily able to accomodate things like this.\n\nActually, if the git diff format is fixed, do we even need the explicit\nrename line? It could be enough if the filenames on the diff line would\nbe just different. Or you want it because of clarity?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"3446","messageId":"Pine.LNX.4.58.0505170812060.18337@ppc970.osdl.org","threadId":"634","inReplyTo":"20050517070158.GA10031@pasky.ji.cz","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-17T15:20:26Z","receivedAt":"2005-05-17T15:20:26Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 May 2005, Petr Baudis wrote:\n> > \n> > \tdiff -git a/kernel/sched.c b/kernel/sched.c.old\n> > \trename kernel/sched.c kernel/sched.c.old\n> \n> Actually, if the git diff format is fixed, do we even need the explicit\n> rename line? It could be enough if the filenames on the diff line would\n> be just different. Or you want it because of clarity?\n\nYes, it's something we can glean from the header itself (or the ---/+++\nlines), but I'd prefer it just to make things really obvious. Especially\nas all the other pathnames involved (both on the \"diff\" header line and on\nthe ---/+++ lines) are in non-canonical -p1 format. So the \"rename\" line\nwould be the only one that is actually in canonical form.\n\nThere's also a real technical reason for this: since the rename format\nwould not be a valid patch for a traditional \"patch\" program, and if we\never want to actually teach \"patch\" to handle it, we really need to be\nexplicit. There are tons of traditional patches around that say\n\n\tdiff -Nur a/kernel/sched.c.old b/kernel/sched.c\n\t--- a/kernel/sched.c.old\n\t+++ b/kernel/sched.c\n\t...\n\nand clearly the above is _not_ a rename from \"sched.c.old\" to \"sched.c\",\nso if we want to teach \"patch\" about the magic git rules, we'd have to\nhave something unambiguous that a GNU patch maintainer might be willing to\ntrigger on. The combination of the \"diff -git \" and \"rename\" markers might\nbe such a thing.\n\nSo it's a combination of clarity, canonical names, and \"patch\" issues.\n\n\t\tLinus\n"},{"id":"3458","messageId":"7vu0l1fz6p.fsf@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505170812060.18337@ppc970.osdl.org","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-17T19:08:14Z","receivedAt":"2005-05-17T19:08:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> There's also a real technical reason for this: since the rename format\nLT> would not be a valid patch for a traditional \"patch\" program, and if we\nLT> ever want to actually teach \"patch\" to handle it, we really need to be\nLT> explicit. There are tons of traditional patches around that say\n\nLT> \tdiff -Nur a/kernel/sched.c.old b/kernel/sched.c\nLT> \t--- a/kernel/sched.c.old\nLT> \t+++ b/kernel/sched.c\nLT> \t...\n\nLT> and clearly the above is _not_ a rename from \"sched.c.old\" to \"sched.c\",\nLT> so if we want to teach \"patch\" about the magic git rules, we'd have to\nLT> have something unambiguous that a GNU patch maintainer might be willing to\nLT> trigger on. The combination of the \"diff -git \" and \"rename\" markers might\nLT> be such a thing.\n\nLT> So it's a combination of clarity, canonical names, and \"patch\" issues.\n\nI've been thinking about doing some rename detection in\ndiff-helper for some time. Here is what that would produce in\nyour proposed file format (BTW, wouldn't the earlier patch ready\nfor merge already?), if you move file frotz to file nitfol and\nat the same time do some edits:\n\n diff -git a/frotz b/frotz\n rename old frotz\n rename new nitfol\n delete file mode 100644\n --- a/frotz\n +++ /dev/null\n @@ -1,2 +0,0 @@\n -xyzzy\n -rezrov\n diff -git a/nitfol b/nitfol\n rename old frotz\n rename new nitfol\n new file mode 100644\n --- /dev/null\n +++ b/nitfol\n @@ -0,0 +1,2 @@\n +xyzzy\n +rezrov\n diff -git a/nitfol b/nitfol\n rename old frotz\n rename new nitfol\n --- a/nitfol\n +++ b/nitfol\n @@ -1,2 +1,3 @@\n xyzzy\n rezrov\n +gnusto\n\nThe basic idea is to express the pure rename with traditional\ntwo patches against /dev/null, plus optionally contents patch on\ntop after pure rename patches.\n\nI am still debating myself where rename lines should be, though.\nI cannot decide so I placed them in all three in the above\nexample.\n\n\n"},{"id":"3461","messageId":"Pine.LNX.4.58.0505171227260.18337@ppc970.osdl.org","threadId":"634","inReplyTo":"7vu0l1fz6p.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-17T19:32:52Z","receivedAt":"2005-05-17T19:32:52Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 May 2005, Junio C Hamano wrote:\n> \n> I've been thinking about doing some rename detection in\n> diff-helper for some time. Here is what that would produce in\n> your proposed file format (BTW, wouldn't the earlier patch ready\n> for merge already?), if you move file frotz to file nitfol and\n> at the same time do some edits:\n\nThis has the advantage of working with any old \"patch\" version, but it has \nthe disadvantage of being human-unreadable, and big. \n\nTo me, there really are only two reasons to do rename diffs:\n - smaller diffs\n - human readability (you can actually see what changed)\n\nand if you want to have compatibility with a \"patch\" program that doesn't\nsupport the feature (like your example), you basically lose both of those\nadvantages. You have _some_ human-readability, but it basically boils down\nto \"ignore all those deletes/creates\".\n\nSo I'd really suggest having just a flag that says \"pure old diff format\" \nor \"new diff format with renames\", and if the latter is selected, then do\n_just_ the changes, ie the rename+change case would really boil down to\ngetting just\n\n> diff -git a/nitfol b/nitfol\n> rename old frotz\n> rename new nitfol\n> --- a/nitfol\n> +++ b/nitfol\n> @@ -1,2 +1,3 @@\n> xyzzy\n> rezrov\n> +gnusto\n\n(except I think it would be nice to have the renamed names show up in the \n\"diff\" and \"---/+++\" lines too)\n\n\t\tLinus\n"},{"id":"3469","messageId":"20050517211132.GK7136@pasky.ji.cz","threadId":"634","inReplyTo":"Pine.LNX.4.21.0505161955340.30848-100000@iabervon.org","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-17T21:11:32Z","receivedAt":"2005-05-17T21:11:32Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Tue, May 17, 2005 at 02:10:35AM CEST, I got a letter\nwhere Daniel Barkalow told me that...\n> On Mon, 16 May 2005, Linus Torvalds wrote:\n> \n> > One final note: I actually think that \"rename patches\" make a ton of \n> > sense, even if git itself doesn't track renames. If we ever have a \"smart \n> > diff\" thing that can generate inter-file diffs, I'd like to eventually see\n> > \n> > \tdiff -git a/kernel/sched.c b/kernel/sched.c.old\n> > \trename kernel/sched.c kernel/sched.c.old\n> > \told mode 100644\n> > \tnew mode 100755\n> \n> I'd like something like:\n> \n> diff -git a/kernel/sched.c b/kernel/sched.c.old\n> filename -- kernel/sched.c\n> filename ++ kernel/sched.c.old\n> mode -- 100644\n> mode ++ 100755\n> --- a/kernel/sched.c\n> +++ b/kernel/sched.c.old\n> @@ -1,5 +1,5 @@\n> (etc.)\n> \n> because I actually start thinking of the two sides as \"-\" and \"+\", and I'd\n> actually have to think about which is \"old\" and which is \"new\", and which\n> way the \"rename\" line goes, and so forth. I'd actually be happier with\n> just a \"mode -- 100644\" line for a deleted file, also. If I'm looking at a\n> patch, and I read Makefile with '-' and '+' versions of the lists of\n> objects, and then get to a \"new file\" line, I have to think about it to\n> associate the '+' side with having the file and the '-' side with not\n> having it.\n\nOops, I've somehow completely missed this mail, but I like this idea a\nlot. What do you think, Linus and Junio?\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nC++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor\n"},{"id":"3472","messageId":"7vy8adsg77.fsf@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":"20050517211132.GK7136@pasky.ji.cz","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Junio C Hamano","fromEmail":"junio@siamese.dyndns.org","sentAt":"2005-05-17T21:19:56Z","receivedAt":"2005-05-17T21:19:56Z","isPatch":true,"sender":{"key":"junio@siamese.dyndns.org","avatar":null},"body":">>>>> \"PB\" == Petr Baudis writes:\n\nPB> Oops, I've somehow completely missed this mail, but I like this idea a\nPB> lot. What do you think, Linus and Junio?\n\nI find the original Linus one easier to read.\n\n"},{"id":"3482","messageId":"7vsm0lqym3.fsf@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505171227260.18337@ppc970.osdl.org","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-17T22:25:08Z","receivedAt":"2005-05-17T22:25:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> So I'd really suggest having just a flag that says \"pure old diff format\" \nLT> or \"new diff format with renames\", and if the latter is selected, then do\nLT> _just_ the changes, ie the rename+change case would really boil down to\nLT> getting just\n\nThat is sensible. So the with --detect-rename flag, we do\nrename detection and show only the changes, otherwise we do not\ndo rename detection and give pure old diff (two diffs against\n/dev/null, that is). I do not personally think --detect-rename\nwith --output-old-style-diff is useful.\n\nNow, in the new diff format, if the rename is really a pure\nrename, then we would have:\n\n diff -git a/nitfol b/nitfol\n rename old frotz\n rename new nitfol\n diff -git a/rezrov b/rezrov\n --- a/rezrov\n +++ b/rezrov\n @@ ...\n\nthat is, nothing until the patch for the next file or EOF. Is\nthis acceptable?\n\n"},{"id":"3485","messageId":"Pine.LNX.4.58.0505171630130.18337@ppc970.osdl.org","threadId":"634","inReplyTo":"7vsm0lqym3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-17T23:32:47Z","receivedAt":"2005-05-17T23:32:47Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 May 2005, Junio C Hamano wrote:\n> \n> Now, in the new diff format, if the rename is really a pure\n> rename, then we would have:\n> \n> diff -git a/nitfol b/nitfol\n> rename old frotz\n> rename new nitfol\n> diff -git a/rezrov b/rezrov\n> --- a/rezrov\n> +++ b/rezrov\n> @@ ...\n> \n> that is, nothing until the patch for the next file or EOF. Is\n> this acceptable?\n\nI think that's exactly what we want. At least it does exactly the right \nthing for me, when I do '/^diff ' in less, with nice highlighting of the \nheaders.\n\nWith people inevitably adding some nice coloration support in gitweb etc,\nand it will be outstanding.\n\n\t\tLinus\n"},{"id":"3494","messageId":"pan.2005.05.18.13.40.32.907488@smurf.noris.de","threadId":"634","inReplyTo":"7vsm0mlosf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Matthias Urlichs","fromEmail":"smurf@smurf.noris.de","sentAt":"2005-05-18T13:40:33Z","receivedAt":"2005-05-18T13:40:33Z","isPatch":true,"sender":{"key":"matthias@urlichs.de","avatar":"https://gravatar.com/avatar/2708905af227313eba6f2b2ae0f7d0259b5ac5d71baef58fe5a13c699ce0bbf0?d=mp&s=160"},"body":"Hi, Junio C Hamano wrote:\n> \n> I'd agree what you said about \"diff -git\" in the rest of your\n> message makes the most sense.\n> \n... except, please use \"diff --git\".\n\n-- \nMatthias Urlichs | {M:U} IT Design @ m-u-it.de | smurf@smurf.noris.de\n\n\n"},{"id":"3496","messageId":"Pine.LNX.4.58.0505180819190.18337@ppc970.osdl.org","threadId":"634","inReplyTo":"pan.2005.05.18.13.40.32.907488@smurf.noris.de","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-18T15:20:35Z","receivedAt":"2005-05-18T15:20:35Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 18 May 2005, Matthias Urlichs wrote:\n\n> Hi, Junio C Hamano wrote:\n> > \n> > I'd agree what you said about \"diff -git\" in the rest of your\n> > message makes the most sense.\n> > \n> ... except, please use \"diff --git\".\n\nYes, that makes sense. It's not three flags \"g\" \"i\" and \"t\", it's the \n\"git\" flag.\n\n\t\tLinus\n"},{"id":"3499","messageId":"20050518160710.GA19264@kiste.smurf.noris.de","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505180819190.18337@ppc970.osdl.org","subject":"Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.","fromName":"Matthias Urlichs","fromEmail":"smurf@smurf.noris.de","sentAt":"2005-05-18T16:07:10Z","receivedAt":"2005-05-18T16:07:10Z","isPatch":true,"sender":{"key":"matthias@urlichs.de","avatar":"https://gravatar.com/avatar/2708905af227313eba6f2b2ae0f7d0259b5ac5d71baef58fe5a13c699ce0bbf0?d=mp&s=160"},"body":"Hi,\n\nLinus Torvalds:\n> It's not three flags \"g\" \"i\" and \"t\",\n\nOr the (hitherto nonexisting, at least in GNU diff) '-g' flag with an\n\"it\" argument. ;-)\n\n-- \nMatthias Urlichs | {M:U} IT Design @ m-u-it.de | smurf@smurf.noris.de\n"},{"id":"3500","messageId":"7vpsvopla0.fsf_-_@assigned-by-dhcp.cox.net","threadId":"634","inReplyTo":"Pine.LNX.4.58.0505180819190.18337@ppc970.osdl.org","subject":"[PATCH] Fix diff output take #4.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-18T16:10:47Z","receivedAt":"2005-05-18T16:10:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> Yes, that makes sense. It's not three flags \"g\" \"i\" and \"t\", it's the \nLT> \"git\" flag.\n\nConcurred. This is against the tip of your tree. Pasky already\nhas a version with '-git' in his tree but I trust he can deal\nwith that single byte change locally.\n\n------------\n[PATCH] Fix diff output take #4.\n\nThis implements the output format suggested by Linus in\n, except the\nimaginary diff option is spelled \"diff --git\" with double\ndashes.\n\nSigned-off-by: Junio C Hamano \n---\n\ndiff.c | 14 +++++++-------\nt/t4000-diff-format.sh | 7 +++++--\n2 files changed, 12 insertions(+), 9 deletions(-)\n\ndiff -git a/diff.c b/diff.c\n--- a/diff.c\n+++ b/diff.c\n@@ -83,7 +83,6 @@\n \t\t\t struct diff_tempfile *temp)\n {\n \tint i, next_at;\n-\tconst char *git_prefix = \"# mode: \";\n \tconst char *diff_cmd = \"diff -L'%s%s' -L'%s%s'\";\n \tconst char *diff_arg = \"'%s' '%s'||:\"; /* \"||:\" is to return 0 */\n \tconst char *input_name_sq[2];\n@@ -123,15 +122,16 @@\n \tnext_at += snprintf(cmd+next_at, cmd_size-next_at,\n \t\t\t diff_arg, input_name_sq[0], input_name_sq[1]);\n \n+\tprintf(\"diff --git a/%s b/%s\\n\", name, name);\n \tif (!path1[0][0])\n-\t\tprintf(\"%s. %s %s\\n\", git_prefix, temp[1].mode, name);\n+\t\tprintf(\"new file mode %s\\n\", temp[1].mode);\n \telse if (!path1[1][0])\n-\t\tprintf(\"%s%s . %s\\n\", git_prefix, temp[0].mode, name);\n+\t\tprintf(\"deleted file mode %s\\n\", temp[0].mode);\n \telse {\n-\t\tif (strcmp(temp[0].mode, temp[1].mode))\n-\t\t\tprintf(\"%s%s %s %s\\n\", git_prefix,\n-\t\t\t temp[0].mode, temp[1].mode, name);\n-\n+\t\tif (strcmp(temp[0].mode, temp[1].mode)) {\n+\t\t\tprintf(\"old mode %s\\n\", temp[0].mode);\n+\t\t\tprintf(\"new mode %s\\n\", temp[1].mode);\n+\t\t}\n \t\tif (strncmp(temp[0].mode, temp[1].mode, 3))\n \t\t\t/* we do not run diff between different kind\n \t\t\t * of objects.\ndiff -git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh\n--- a/t/t4000-diff-format.sh\n+++ b/t/t4000-diff-format.sh\n@@ -26,7 +26,9 @@\n 'git-diff-files -p after editing work tree.' \\\n 'git-diff-files -p >current'\n cat >expected <<\\EOF\n-# mode: 100644 100755 path0\n+diff --git a/path0 b/path0\n+old mode 100644\n+new mode 100755\n --- a/path0\n +++ b/path0\n @@ -1,3 +1,3 @@\n@@ -34,7 +36,8 @@\n Line 2\n -line 3\n +Line 3\n-# mode: 100755 . path1\n+diff --git a/path1 b/path1\n+deleted file mode 100755\n --- a/path1\n +++ /dev/null\n @@ -1,3 +0,0 @@\n------------------------------------------------\n\n\n"}]}