{"thread":{"id":"412","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","startedAt":"2005-05-01T00:34:10Z","lastAt":"2005-05-15T18:10:12Z","messageCount":25,"participants":["Daniel Jacobowitz","Linus Torvalds","Junio C Hamano","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"2272","messageId":"7v7jij3htp.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":null,"subject":"[PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-01T00:34:10Z","receivedAt":"2005-05-01T00:34:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Diff-tree-helper take two patch inadvertently dropped the\nsupport of -R option, which is necessary to produce reverse diff\nbased on diff-cache and diff-files output (diff-tree does not\nmatter since you can feed two trees in reverse order). This\npatch restores it.\n\nSigned-off-by: Junio C Hamano \n---\ndiff-tree-helper.c | 17 +++++++++++------\n1 files changed, 11 insertions(+), 6 deletions(-)\n\njit-diff 0 diff-tree-helper.c\n# - Fix up d_type handling - we need to include before\n# + working-tree\n--- k/diff-tree-helper.c (mode:100644)\n+++ l/diff-tree-helper.c (mode:100644)\n@@ -44,7 +44,8 @@ static int parse_oneside_change(const ch\n \treturn 0;\n }\n \n-static int parse_diff_tree_output(const char *buf, const char **spec, int cnt)\n+static int parse_diff_tree_output(const char *buf,\n+\t\t\t\t const char **spec, int cnt, int reverse)\n {\n \tstruct diff_spec old, new;\n \tchar path[PATH_MAX];\n@@ -98,8 +99,12 @@ static int parse_diff_tree_output(const \n \tdefault:\n \t\treturn -1;\n \t}\n-\tif (!cnt || matches_pathspec(path, spec, cnt))\n-\t\trun_external_diff(path, &old, &new);\n+\tif (!cnt || matches_pathspec(path, spec, cnt)) {\n+\t\tif (reverse)\n+\t\t\trun_external_diff(path, &new, &old);\n+\t\telse\n+\t\t\trun_external_diff(path, &old, &new);\n+\t}\n \treturn 0;\n }\n \n@@ -108,14 +113,14 @@ static const char *diff_tree_helper_usag\n \n int main(int ac, const char **av) {\n \tstruct strbuf sb;\n-\tint reverse_diff = 0;\n+\tint reverse = 0;\n \tint line_termination = '\\n';\n \n \tstrbuf_init(&sb);\n \n \twhile (1 < ac && av[1][0] == '-') {\n \t\tif (av[1][1] == 'R')\n-\t\t\treverse_diff = 1;\n+\t\t\treverse = 1;\n \t\telse if (av[1][1] == 'z')\n \t\t\tline_termination = 0;\n \t\telse\n@@ -129,7 +134,7 @@ int main(int ac, const char **av) {\n \t\tread_line(&sb, stdin, line_termination);\n \t\tif (sb.eof)\n \t\t\tbreak;\n-\t\tstatus = parse_diff_tree_output(sb.buf, av+1, ac-1);\n+\t\tstatus = parse_diff_tree_output(sb.buf, av+1, ac-1, reverse);\n \t\tif (status)\n \t\t\tfprintf(stderr, \"cannot parse %s\\n\", sb.buf);\n \t}\n\n\n\n"},{"id":"2269","messageId":"Pine.LNX.4.58.0504301805300.2296@ppc970.osdl.org","threadId":"412","inReplyTo":"7v7jij3htp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-01T01:09:53Z","receivedAt":"2005-05-01T01:09:53Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 30 Apr 2005, Junio C Hamano wrote:\n>\n> Diff-tree-helper take two patch inadvertently dropped the\n> support of -R option\n\nTalking about the diffs, I'm beginning to hate those \"mode\" things.\n\nNot only do they screw up diffstat (big deal), but they are pointless, \nsince 99.9% of the time the mode stays the same.\n\nSo it would be much nicer (I think) if mode changes are handled \nseparately, with a simple separate line before the diff saying\n\n\t\"Mode change: %o->%o %s\", oldmode, newmode, path\n\nand not mess up the diff header. That way, you only see it when it\nactually makes any difference, and it's more readable both for humans\n_and_ machines as a result.\n\nNormal \"patch\" will just ignore the extra lines before the diff anyway, so \nit won't matter there.\n\nComments?\n\n\t\tLinus\n"},{"id":"2267","messageId":"20050501014726.GA15220@nevyn.them.org","threadId":"412","inReplyTo":"Pine.LNX.4.58.0504301805300.2296@ppc970.osdl.org","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Daniel Jacobowitz","fromEmail":"dan@debian.org","sentAt":"2005-05-01T01:47:26Z","receivedAt":"2005-05-01T01:47:26Z","isPatch":true,"sender":{"key":"dan@debian.org","avatar":null},"body":"On Sat, Apr 30, 2005 at 06:09:53PM -0700, Linus Torvalds wrote:\n> So it would be much nicer (I think) if mode changes are handled \n> separately, with a simple separate line before the diff saying\n> \n> \t\"Mode change: %o->%o %s\", oldmode, newmode, path\n> \n> and not mess up the diff header. That way, you only see it when it\n> actually makes any difference, and it's more readable both for humans\n> _and_ machines as a result.\n> \n> Normal \"patch\" will just ignore the extra lines before the diff anyway, so \n> it won't matter there.\n> \n> Comments?\n\nIt sounds good - but could you efficiently collect them before any diff\noutput? If you have something like this, it'll be easy to read:\n\nMode change: 644->755 foo.sh\nMode change: 644->755 bar.sh\n\n--- ChangeLog\n+++ ChangeLog\n@@ -1,0 +1,1 @@\n+New line\n--- copyright\n+++ copyright\n@@ -1,0 +1,1 @@\n+New line\n\n\nBut if you generate this then you might as well not generate the mode\nlines at all, for all a human looking at the diff is going to notice\nthem:\n\n--- ChangeLog\n+++ ChangeLog\n@@ -1,0 +1,1 @@\n+New line\nMode change: 644->755 foo.sh\n--- copyright\n+++ copyright\n@@ -1,0 +1,1 @@\n+New line\nMode change: 644->755 bar.sh\n\n\nThe latter is how diff does its \"Only in\" messages. I never see them\nwhen I'm looking through a diff of any size; only via diffstat, where\nthey're clearly disambiguated.\n\n-- \nDaniel Jacobowitz\nCodeSourcery, LLC\n"},{"id":"2274","messageId":"7vis231y7y.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"Pine.LNX.4.58.0504301805300.2296@ppc970.osdl.org","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-01T02:22:57Z","receivedAt":"2005-05-01T02:22:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> Talking about the diffs, I'm beginning to hate those \"mode\" things.\n\nLikewise.\n\nLT> Not only do they screw up diffstat (big deal), but they are pointless, \nLT> since 99.9% of the time the mode stays the same.\n\nPointless, yes. mode is not what screwing up diffstat but\ncomparing against /dev/null is, so it is not a reason to hate\nmode, and my fingers learned to say diffstat -p1 already so it\nis not a big deal anymore.\n\nLT> Normal \"patch\" will just ignore the extra lines before the\nLT> diff anyway, so it won't matter there.\n\nLT> Comments?\n\nI am 100% in agreement with you here. The only reason I added\nit was to match what Pasky does so that his cg-patch can eat its\noutput. To me, pleasing cg-patch is far lower priority than\npleasing l-k developers, so your veto counts.\n\nMy JIT tools do not use that mode thing in the patch. I apply a\npatch between two commits (or trees) to the work tree by doing\nsomething like this:\n\n GIT_EXTERNAL_DIFF=jit-diff-extract \\\n jit-diff \"$@\" | {\n cd \"${GIT_PROJECT_TOP}\"\n sh\n }\n\nHere jit-diff-extract is the gem that creates a small shell\nscript that patches the file and runs \"chmod +x\" or \"chmod -x\"\nwhen necessary, and does git-update-cache for added or removed\nfiles. Its output would look something like this:\n\n patch -p1 <<\\EOF\n --- /dev/null\n +++ fs/ext9/Makefile\n @@ ....\n EOF\n chmod -x 'fs/ext9/Makefile'\n git-update-cache --add --remove -- 'fs/ext9/Makefile'\n\nMaybe I can make the default diff output just like the above?\nAs you say, normal patch would not look at those shell script\npart at all anyway.\n\n"},{"id":"2275","messageId":"Pine.LNX.4.58.0504302224510.2296@ppc970.osdl.org","threadId":"412","inReplyTo":"7vis231y7y.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-01T05:27:50Z","receivedAt":"2005-05-01T05:27:50Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 30 Apr 2005, Junio C Hamano wrote:\n> Its output would look something like this:\n> \n> patch -p1 <<\\EOF\n> --- /dev/null\n> +++ fs/ext9/Makefile\n> @@ ....\n> EOF\n> chmod -x 'fs/ext9/Makefile'\n> git-update-cache --add --remove -- 'fs/ext9/Makefile'\n> \n> Maybe I can make the default diff output just like the above?\n> As you say, normal patch would not look at those shell script\n> part at all anyway.\n\nI actually do end up looking at diffs, and I'd hate it. I'd much rather\nhave as little extra fluff as possible, and putting shell scipt fragments\nin it definitely counts as distraction.\n\nThe fewer lines there are that don't usually tell a human anything, the \nbetter. Dense is good. \n\n\t\tLinus\n"},{"id":"2276","messageId":"Pine.LNX.4.58.0504302228230.2296@ppc970.osdl.org","threadId":"412","inReplyTo":"20050501014726.GA15220@nevyn.them.org","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-05-01T05:33:05Z","receivedAt":"2005-05-01T05:33:05Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 30 Apr 2005, Daniel Jacobowitz wrote:\n> \n> It sounds good - but could you efficiently collect them before any diff\n> output? If you have something like this, it'll be easy to read:\n> \n> Mode change: 644->755 foo.sh\n> Mode change: 644->755 bar.sh\n> \n> --- ChangeLog\n> +++ ChangeLog\n\nThat may sound like a good idea, but it's horrid.\n\nYou'd only have to gather them back later anyway, since you can only apply \nthe mode change _after_ you've done the diff. Why? The diff may be the \nthing that creates the file in the first place. Sp you should consider the \nmode changes as part of the \"stream\", not as something separate from the \nstream.\n\nSo I'd really much rather see it more as an \"Index\" line, which gets\nprepended as part of the patch for that file (of course, either patch or\nmodeline can be missing).\n\n\t\tLinus\n"},{"id":"2277","messageId":"7vr7grzcqu.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"Pine.LNX.4.58.0504302224510.2296@ppc970.osdl.org","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-01T06:22:49Z","receivedAt":"2005-05-01T06:22:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> I actually do end up looking at diffs, and I'd hate it. I'd much rather\nLT> have as little extra fluff as possible, and putting shell scipt fragments\nLT> in it definitely counts as distraction.\n\nWell I was half joking when I suggested that and I am glad to\nsee that you have a good aesthetics ;-). How about:\n\n - Stop attempting to be compatible with cg-patch, and drop\n (mode:XXXXXX) bits from the diff.\n\n - Do keep the /dev/null change for created and deleted case.\n\n - No \"Index:\" line, no \"Mode change:\" line, anywhere in the\n output. Anything that wants the mode bits and sha1 hash can\n do things from GIT_EXTERNAL_DIFF mechanism. Maybe document\n suggested usage mechanism better.\n\nI'll whip something up along the above lines and submit it.\n\n"},{"id":"2278","messageId":"7vll6zza43.fsf_-_@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"Pine.LNX.4.58.0504302224510.2296@ppc970.osdl.org","subject":"[PATCH] Rework built-in diff to make its output more dense.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-01T07:19:40Z","receivedAt":"2005-05-01T07:19:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus says,\n\n The fewer lines there are that don't usually tell a human\n anything, the better. Dense is good.\n\nThis patch makes the default diff output more dense. This\nremoves the previous misguided attempt to be cg-patch\ncompatible.\n\nSigned-off-by: Junio C Hamano \n---\n\ndiff.c | 26 +++++++++++++++-----------\n1 files changed, 15 insertions(+), 11 deletions(-)\n\n# - [PATCH] Resurrect diff-tree-helper -R\n# + [PATCH] Rework built-in diff to make its output more dense.\n--- k/diff.c\n+++ l/diff.c\n@@ -82,35 +82,32 @@ static void builtin_diff(const char *nam\n \t\t\t struct diff_tempfile *temp)\n {\n \tint i, next_at;\n-\tconst char *diff_cmd = \"diff -L'%s%s%s' -L'%s%s%s'\";\n+\tconst char *diff_cmd = \"diff -L'%s%s' -L'%s%s'\";\n \tconst char *diff_arg = \"'%s' '%s'\";\n \tconst char *input_name_sq[2];\n \tconst char *path0[2];\n \tconst char *path1[2];\n-\tchar mode[2][20];\n \tconst char *name_sq = sq_expand(name);\n \tchar *cmd;\n \t\n-\t/* diff_cmd and diff_arg have 8 %s in total which makes\n-\t * the sum of these strings 16 bytes larger than required.\n+\t/* diff_cmd and diff_arg have 6 %s in total which makes\n+\t * the sum of these strings 12 bytes larger than required.\n \t * we use 2 spaces around diff-opts, and we need to count\n-\t * terminating NUL, so we subtract 13 here.\n+\t * terminating NUL, so we subtract 9 here.\n \t */\n \tint cmd_size = (strlen(diff_cmd) + strlen(diff_opts) +\n-\t\t\tstrlen(diff_arg) - 13);\n+\t\t\tstrlen(diff_arg) - 9);\n \tfor (i = 0; i < 2; i++) {\n \t\tinput_name_sq[i] = sq_expand(temp[i].name);\n \t\tif (!strcmp(temp[i].name, \"/dev/null\")) {\n \t\t\tpath0[i] = \"/dev/null\";\n \t\t\tpath1[i] = \"\";\n-\t\t\tmode[i][0] = 0;\n \t\t} else {\n \t\t\tpath0[i] = i ? \"l/\" : \"k/\";\n \t\t\tpath1[i] = name_sq;\n-\t\t\tsprintf(mode[i], \" (mode:%s)\", temp[i].mode);\n \t\t}\n \t\tcmd_size += (strlen(path0[i]) + strlen(path1[i]) +\n-\t\t\t strlen(mode[i]) + strlen(input_name_sq[i]));\n+\t\t\t strlen(input_name_sq[i]));\n \t}\n \n \tcmd = xmalloc(cmd_size);\n@@ -118,13 +115,20 @@ static void builtin_diff(const char *nam\n \tnext_at = 0;\n \tnext_at += snprintf(cmd+next_at, cmd_size-next_at,\n \t\t\t diff_cmd,\n-\t\t\t path0[0], path1[0], mode[0],\n-\t\t\t path0[1], path1[1], mode[1]);\n+\t\t\t path0[0], path1[0], path0[1], path1[1]);\n \tnext_at += snprintf(cmd+next_at, cmd_size-next_at,\n \t\t\t \" %s \", diff_opts);\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+\tif (!path1[0][0])\n+\t\tprintf(\"Created: %s (mode:%s)\\n\", name, temp[1].mode);\n+\telse if (!path1[1][0])\n+\t\tprintf(\"Deleted: %s\\n\", name);\n+\telse if (strcmp(temp[0].mode, temp[1].mode))\n+\t\tprintf(\"Mode changed: %s (%s->%s)\\n\", name,\n+\t\t temp[0].mode, temp[1].mode);\n+\tfflush(NULL);\n \texeclp(\"/bin/sh\",\"sh\", \"-c\", cmd, NULL);\n }\n \n\n"},{"id":"2279","messageId":"7vfyx7za09.fsf_-_@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"7vr7grzcqu.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Add git-apply-patch-script.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-01T07:21:58Z","receivedAt":"2005-05-01T07:21:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I said:\n\n - Stop attempting to be compatible with cg-patch, and drop\n (mode:XXXXXX) bits from the diff.\n\n - Do keep the /dev/null change for created and deleted case.\n\n - No \"Index:\" line, no \"Mode change:\" line, anywhere in the\n output. Anything that wants the mode bits and sha1 hash can\n do things from GIT_EXTERNAL_DIFF mechanism. Maybe document\n suggested usage better.\n\nThis adds an example script git-apply-patch-script, that can be\nused as the GIT_EXTERNAL_DIFF to apply changes between two trees\ndirectly on the current work tree, like this:\n\n GIT_EXTERNAL_DIFF=git-apply-patch-script git-diff-tree -p \n\nSigned-off-by: Junio C Hamano \n---\n\nMakefile | 4 +--\ngit-apply-patch-script | 60 +++++++++++++++++++++++++++++++++++++++++++++++++\n2 files changed, 62 insertions(+), 2 deletions(-)\n\n# - [PATCH] Rework built-in diff to make its output more dense.\n# + [PATCH] Add git-apply-patch-script.\n--- k/Makefile\n+++ l/Makefile\n@@ -12,8 +12,8 @@ CFLAGS=-g -O2 -Wall\n CC=gcc\n AR=ar\n \n-SCRIPTS=git-merge-one-file-script git-prune-script git-pull-script \\\n-\tgit-tag-script\n+SCRIPTS=git-apply-patch-script git-merge-one-file-script git-prune-script \\\n+\tgit-pull-script git-tag-script\n \n PROG= git-update-cache git-diff-files git-init-db git-write-tree \\\n \tgit-read-tree git-commit-tree git-cat-file git-fsck-cache \\\nCreated: git-apply-patch-script (mode:100755)\n--- /dev/null\n+++ l/git-apply-patch-script\n@@ -0,0 +1,60 @@\n+#!/bin/sh\n+# Copyright (C) 2005 Junio C Hamano\n+#\n+# Applying diff between two trees to the work tree can be\n+# done with the following single command:\n+#\n+# GIT_EXTERNAL_DIFF=git-apply-patch-script git-diff-tree -p $tree1 $tree2\n+#\n+\n+case \"$#\" in\n+2) exit 1 ;; # do not feed unmerged diff to me!\n+esac\n+name=\"$1\" tmp1=\"$2\" hex1=\"$3\" mode1=\"$4\" tmp2=\"$5\" hex2=\"$6\" mode2=\"$7\"\n+case \"$mode1\" in *7??) mode1=+x ;; *6??) mode1=-x ;; esac\n+case \"$mode2\" in *7??) mode2=+x ;; *6??) mode2=-x ;; esac\n+\n+if test -f \"$name.orig\" || test -f \"$name.rej\"\n+then\n+ echo >&2 \"Unresolved patch conflicts in the previous run found.\"\n+ exit 1\n+fi\n+# This will say \"patching ...\" so we do not say anything outselves.\n+\n+diff -u -L \"a/$name\" -L \"b/$name\" \"$tmp1\" \"$tmp2\" | patch -p1\n+test -f \"$name.rej\" || {\n+ case \"$mode1,$mode2\" in\n+ .,?x)\n+\t# newly created\n+\tcase \"$mode2\" in\n+\t+x)\n+\t echo >&2 \"created $name with mode +x.\"\n+\t chmod \"$mode2\" \"$name\"\n+\t ;;\n+\t-)\n+\t echo >&2 \"created $name.\"\n+\t ;;\n+\tesac\n+\tgit-update-cache --add -- \"$name\"\n+\t;;\n+ ?x,.)\n+\t# deleted\n+\techo >&2 \"deleted $name.\"\n+\trm -f \"$name\"\n+\tgit-update-cache --remove -- \"$name\"\n+\t;;\n+ *)\n+\t# changed\n+\tcase \"$mode1,$mode2\" in\n+\t\"$mode2,$mode1\") ;;\n+\t*)\n+\t echo >&2 \"changing mode from $mode1 to $mode2.\"\n+\t chmod \"$mode2\" \"$name\"\n+\t ;;\n+\tesac\n+ esac\n+ # This bit is debatable---the SCM may not want to keep\n+ # cache in sync with the work tree (JIT does want to).\n+ git-update-cache -- \"$name\"\n+}\n+exit 0\n\n"},{"id":"3258","messageId":"20050513224529.GF32232@pasky.ji.cz","threadId":"412","inReplyTo":"Pine.LNX.4.58.0504301805300.2296@ppc970.osdl.org","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-13T22:45:29Z","receivedAt":"2005-05-13T22:45:29Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sun, May 01, 2005 at 03:09:53AM CEST, I got a letter\nwhere Linus Torvalds told me that...\n> \n> \n> On Sat, 30 Apr 2005, Junio C Hamano wrote:\n> >\n> > Diff-tree-helper take two patch inadvertently dropped the\n> > support of -R option\n> \n> Talking about the diffs, I'm beginning to hate those \"mode\" things.\n> \n> Not only do they screw up diffstat (big deal), but they are pointless, \n> since 99.9% of the time the mode stays the same.\n> \n> So it would be much nicer (I think) if mode changes are handled \n> separately, with a simple separate line before the diff saying\n> \n> \t\"Mode change: %o->%o %s\", oldmode, newmode, path\n> \n> and not mess up the diff header. That way, you only see it when it\n> actually makes any difference, and it's more readable both for humans\n> _and_ machines as a result.\n> \n> Normal \"patch\" will just ignore the extra lines before the diff anyway, so \n> it won't matter there.\n> \n> Comments?\n\nSorry for replying after so much time, it looks like I missed this and\ngot here only after checking what change removed the mode: bits...\n\nI'd personally prefer something like\n\n\t@.Mode change:\n\nthat is, using a '@.' prefix for those. It seems to be unique enough and\n'@' is one of the four magic characters prefixing diff lines. Just using\nthe plain string seems too volatile, and I need to grep all the\ninteresting bits out of the diff file. This is because patch can\notherwise complain \"only garbage found in the patch\" when processing the\ndiff, which confuses my users greatly.\n\nWhat do you think?\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":"3260","messageId":"7vpsvu91w0.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050513224529.GF32232@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-13T22:50:23Z","receivedAt":"2005-05-13T22:50:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Have you checked what the current one does?\n\n"},{"id":"3262","messageId":"7vhdh691gs.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050513224529.GF32232@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-13T22:59:31Z","receivedAt":"2005-05-13T22:59:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"PB> Sorry for replying after so much time, it looks like I missed this and\nPB> got here only after checking what change removed the mode: bits...\n\nPB> What do you think?\n\nFYI, here is a demonsrtation of what you have right now.\nTemporarily slurp in the test framework patch you hate so much\nto get t/test-lib.sh ;-), apply this to get t/t2000-diff.sh, cd\nto t and say sh ./t2000-diff.sh\n\nSigned-off-by: Junio C Hamano \n---\njit-diff 1: t/t2000-diff.sh\n# - HEAD: Fix git-diff-files for symlinks.\n# + (working tree)\nCreated: t/t2000-diff.sh (mode:100755)\n--- /dev/null\n+++ b/t/t2000-diff.sh\n@@ -0,0 +1,41 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2005 Junio C Hamano\n+#\n+\n+test_description='Test built-in diff output engine.\n+\n+'\n+. ./test-lib.sh\n+\n+echo >path0 'Line 1\n+Line 2\n+line 3'\n+cat path0 >path1\n+chmod +x path1\n+git-update-cache --add path0 path1\n+mv path0 path0-\n+sed -e 's/line/Line/' path0\n+chmod +x path0\n+rm -f path1\n+git-diff-files -p >current\n+cat >expected <<\\EOF\n+Mode changed: path0 (100644->100755)\n+--- a/path0\n++++ b/path0\n+@@ -1,3 +1,3 @@\n+ Line 1\n+ Line 2\n+-line 3\n++Line 3\n+Deleted: path1\n+--- a/path1\n++++ /dev/null\n+@@ -1,3 +0,0 @@\n+-Line 1\n+-Line 2\n+-line 3\n+EOF\n+\n+test_expect_success 'cmp -s current expected'\n+test_done\n\n\n"},{"id":"3261","messageId":"7vbr7e9162.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050513224529.GF32232@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-13T23:05:57Z","receivedAt":"2005-05-13T23:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis writes:\n\nPB> that is, using a '@.' prefix for those. It seems to be unique enough and\nPB> '@' is one of the four magic characters prefixing diff lines. Just using\nPB> the plain string seems too volatile, and I need to grep all the\nPB> interesting bits out of the diff file. This is because patch can\nPB> otherwise complain \"only garbage found in the patch\" when processing the\nPB> diff, which confuses my users greatly.\n\nPB> What do you think?\n\nPersonally what I think is that grepping in the diff, especially\nif the diff is something you are generating (I am assuming that\nyou are taling about cg-diff fed to cg-patch to port work tree\nchanges forward), is a wrong way to do things.\n\nThe way JIT does the equivalent is via git-apply-patch-script.\nYou run git-diff-{files,cache,tree}, setting GIT_EXTERNAL_DIFF\nenvironment variable to git-apply-patch-script, and have the\napply-patch-script to take care of the mode changes, creation,\netc. See the implementation of the jit-patch command if you are\ninterested.\n\n"},{"id":"3266","messageId":"20050513233354.GK32232@pasky.ji.cz","threadId":"412","inReplyTo":"7vhdh691gs.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-13T23:33:54Z","receivedAt":"2005-05-13T23:33:54Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, May 14, 2005 at 12:59:31AM CEST, I got a letter\nwhere Junio C Hamano told me that...\n> Created: t/t2000-diff.sh (mode:100755)\n> +Mode changed: path0 (100644->100755)\n\nGreat, so it's even worse than before. :/\n\nMy issues:\n\n* Highly inconsistent format. Created has the \"old-style\" mode stuff\nwhile changed mode looks completely differently. If you look at the\nregular diffs, do you notice any format differences between creating\na file and modifying it?\n\n\tActually, I think it should just always look as\n\n\t\t@ Mode changed: A->B C\n\n\twhere either A or B may be empty (for file creation/deletion).\n\tI can see no point in further Created/Deleted lines.\n\n* Filename not last. That'd be much friendlier to scripting, then you\ncould just split the line by spaces and at a certain point slurp the\nrest and say that that would be a filename.\n\n* No special prefix. Even if you think I shouldn't grep (it makes no\nsense to me to redo half of the stuff when I already have something\nreusable done, for no apparent benefit), I feel quite uncomfortable with\npicking up and interpreting random pieces of surrounding text which\naren't marked as special in any way.\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":"3270","messageId":"7vmzqy7k47.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050513233354.GK32232@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-13T23:59:36Z","receivedAt":"2005-05-13T23:59:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis writes:\n\nPB> Dear diary, on Sat, May 14, 2005 at 12:59:31AM CEST, I got a letter\nPB> where Junio C Hamano told me that...\n>> Created: t/t2000-diff.sh (mode:100755)\n>> +Mode changed: path0 (100644->100755)\n\nPB> Great, so it's even worse than before. :/\n\nDepends on the definition of \"before\". At the beginning, we did\nnot do anything special and always said l/foo k/foo even when\ncreate/delete was involved. Then we did a misguided attempt to\nminimally be cg-diff compatible, which Linus complained that it\nwas too distracting for human consumption. The current one is\nsomething in between, a lot more human side.\n\nYes, it is off course worse than the minimally cg-diff\ncompatible one, from cg-patch'es point of view.\n\nYou have seen what the current \"something in between\" does.\nWhat I think is that in order not to distract human (read:\nLinus) who reads patches, they should not share the same special\ncharacters like \"@\". Which unfortunately completely contradicts\nwhat you are attempting to do. Another thing we did while you\nwere looking other way ;-) was that we say mode changed only\nwhen things change, so in that sense it is \"inconsistent\" from\nthe scripting point of view. These were all done to make the\noutput more readable by and less distracting for humans, per\nrequest from Linus.\n\nI do not think nobody uses that current textual \"comment\"\ninformation in automated tools (I do not), so changing them\nshould not be a problem. How about we do something like this:\n\n 1. Invent an environment variable you can define. Let's say\n GIT_DIFF_SHOW_MODES. It could alternatively a flag you\n pass from git-diff-{files,cache,tree,tree-helper} to the\n internal diff engine but then you need to add the necessary\n command line parameter for all these commands. I can be\n persuaded in either way.\n\n 2. When it is defined, we are not interested in pleasing Linus\n by trying not to be distracting. We are more interested in\n producing patch that is easily script processible.\n\n 3. Keep the current behaviour for human comsumption when we\n are operating without the option we define in 1.\n\n 4. Change the mode stuff when GIT_DIFF_SHOW_MODES is defined.\n It would produce one of the following for _all_ entries;\n\n @. (100644->100755) path/to/a/file/that/changed/mode\n @. (100644->120000) path/to/a/file/that/changed/to/symlink\n @. (100644->100644) path/to/a/file/with/no/mode/change\n @. (.->100644) path/to/a/new/file\n @. (100644->.) path/to/a/deleted/file\n\n I have to stress that these would come immediately before\n the patch for each file. Not upfront, not grouped together\n at the beginning.\n\nBTW, what do you think about renaming git-diff-tree-helper to\njust git-diff-helper? It used to be for grokking diff-tree's\noutput but now the family have the same raw output format it\ndoes not make much sense to keep \"tree\" in its name.\n\n"},{"id":"3271","messageId":"7vhdh67jx6.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050513233354.GK32232@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-14T00:03:49Z","receivedAt":"2005-05-14T00:03:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis writes:\n\nPB> .... Even if you think I shouldn't grep\n\nI do not mean you should never grep. There are cases when there\nis no alternative. One case I did not mention is that you would\nwant to accept patches via e-mail and apply. In such a case,\nGIT_EXTERNAL_DIFF with git-apply-patch-script way would not work\nbecause that approach is essentially to patch while you are\ngenerating diff locally.\n\n"},{"id":"3272","messageId":"7voebe63zs.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"7vmzqy7k47.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-14T00:33:11Z","receivedAt":"2005-05-14T00:33:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Another possibility. How about generating the following _only_\nwhen mode changes (including create and delete), even for human\nconsumption? There will be _no_ such line when mode or type\ndoes not change.\n\n# mode: 100644 100755 path/to/a/file/that/changed/mode\n# mode: 100644 120000 path/to/a/file/that/changed/to/symlink\n# mode: 100644 100644 path/to/a/file/with/no/mode/change\n# mode: . 100644 path/to/a/new/file\n# mode: 100644 . path/to/a/deleted/file\n\nThis is not \"something like this\", but a proposal for the exact\noutput format specification (I am going to code immediately).\nEach token above is separated with exactly one ' ' (ASCII 0x20)\neach, and such a line comes immediately before the patch for the\nfile. Showing both mode bits is to prepare for the case you\nwould want to apply the patch in reverse.\n\nThis is for machine consumption and there is no need to force\nthem to parse out -> and (), so I dropped them. And mode or\ntype change happens so rarely, it would be OK for human\nconsumption if we show these garbage (from human point of view)\nonly when things change. Can you parse this, or do you always\nwant to have them even if nothing changes?\n\nLet's see how this would look like to humans.\n\n # mode: 100644 100755 path0\n --- a/path0\n +++ b/path0\n @@ -1,3 +1,3 @@\n Line 1\n Line 2\n -line 3\n +Line 3\n # mode: 100644 . path1\n --- a/path1\n +++ /dev/null\n @@ -1,3 +0,0 @@\n -Line 1\n -Line 2\n -line 3\n --- a/path2\n +++ b/path2\n @@ -1,3 +1,3 @@\n Line 1\n Line 2\n -line 3\n +Line 3\n # mode: . 100755 t/t2000-diff.sh\n --- /dev/null\n +++ b/t/t2000-diff.sh\n @@ -0,0 +1,41 @@\n ...\n\nDoesn't look too bad, does it?\n\n"},{"id":"3311","messageId":"20050514150200.GJ3905@pasky.ji.cz","threadId":"412","inReplyTo":"7vmzqy7k47.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-14T15:02:00Z","receivedAt":"2005-05-14T15:02:00Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, May 14, 2005 at 01:59:36AM CEST, I got a letter\nwhere Junio C Hamano told me that...\n> >>>>> \"PB\" == Petr Baudis writes:\n> \n> PB> Dear diary, on Sat, May 14, 2005 at 12:59:31AM CEST, I got a letter\n> PB> where Junio C Hamano told me that...\n> >> Created: t/t2000-diff.sh (mode:100755)\n> >> +Mode changed: path0 (100644->100755)\n> \n> PB> Great, so it's even worse than before. :/\n> \n> Depends on the definition of \"before\". At the beginning, we did\n> not do anything special and always said l/foo k/foo even when\n> create/delete was involved. Then we did a misguided attempt to\n> minimally be cg-diff compatible, which Linus complained that it\n> was too distracting for human consumption. The current one is\n> something in between, a lot more human side.\n\nBy \"before\" I meant the Linus proposal I was originally replying too.\nIt seems I'm still missing part of the history. :-)\n\n> You have seen what the current \"something in between\" does.\n> What I think is that in order not to distract human (read:\n> Linus) who reads patches, they should not share the same special\n> characters like \"@\". Which unfortunately completely contradicts\n> what you are attempting to do.\n\nI don't think it discards humans, actually. I'd rather say it makes them\naware that this is something special. And if you show it only when the\nmode changes, it will always be a special thing, not only something\nwhich clutters the view.\n\nSo I'd say it's better for humans too, since it is clear for them that\nthis is not part of the commit message, and it carries special meaning\nfor the tool they will feed it to.\n\n> Another thing we did while you were looking other way ;-) was that we\n> say mode changed only when things change, so in that sense it is\n> \"inconsistent\" from the scripting point of view.\n\nI have no issue with that.\n\n> I do not think nobody uses that current textual \"comment\"\n> information in automated tools (I do not), so changing them\n> should not be a problem. How about we do something like this:\n> \n> 1. Invent an environment variable you can define. Let's say\n> GIT_DIFF_SHOW_MODES. It could alternatively a flag you\n> pass from git-diff-{files,cache,tree,tree-helper} to the\n> internal diff engine but then you need to add the necessary\n> command line parameter for all these commands. I can be\n> persuaded in either way.\n\nI think this completely misses the point. You are viewing what I'm\nsuggesting as trying to just aid Cogito's internals using cg-diff |\ncg-patch, but that's actually not my major reason for doing this at all.\nI view that as a hack anyway and it should eventually do a three-way\nmerge too at those places.\n\nWhat I'm trying to do is to figure out a good encapsulation for mode\nchanges which can be put in *all* the patches. So when you are sending\nme some new testcases, I don't have to chmod them manually. That's the\nmain point of doing this. I could deal with mode changes completely\nseparately if it was only about Cogito's internal stuff.\n\n> BTW, what do you think about renaming git-diff-tree-helper to\n> just git-diff-helper? It used to be for grokking diff-tree's\n> output but now the family have the same raw output format it\n> does not make much sense to keep \"tree\" in its name.\n\nNo issue with that.\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":"3312","messageId":"20050514150356.GK3905@pasky.ji.cz","threadId":"412","inReplyTo":"7voebe63zs.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-14T15:03:56Z","receivedAt":"2005-05-14T15:03:56Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, May 14, 2005 at 02:33:11AM CEST, I got a letter\nwhere Junio C Hamano told me that...\n> Another possibility. How about generating the following _only_\n> when mode changes (including create and delete), even for human\n> consumption? There will be _no_ such line when mode or type\n> does not change.\n> \n> # mode: 100644 100755 path/to/a/file/that/changed/mode\n> # mode: 100644 120000 path/to/a/file/that/changed/to/symlink\n> # mode: 100644 100644 path/to/a/file/with/no/mode/change\n> # mode: . 100644 path/to/a/new/file\n> # mode: 100644 . path/to/a/deleted/file\n> \n> This is not \"something like this\", but a proposal for the exact\n> output format specification (I am going to code immediately).\n> Each token above is separated with exactly one ' ' (ASCII 0x20)\n> each, and such a line comes immediately before the patch for the\n> file. Showing both mode bits is to prepare for the case you\n> would want to apply the patch in reverse.\n> \n> This is for machine consumption and there is no need to force\n> them to parse out -> and (), so I dropped them. And mode or\n> type change happens so rarely, it would be OK for human\n> consumption if we show these garbage (from human point of view)\n> only when things change. Can you parse this, or do you always\n> want to have them even if nothing changes?\n> \n> Let's see how this would look like to humans.\n\nFor humans I'd say \"Mode change\" instead of \"mode\" would be better, and\nfor machines I still think \"@\" would be better than \"#\". \"#\" can occur\nquite naturally in some code snippets or whatever pasted to the commit\nmessage, which is extremely unlikely for \"@\". What are the advantages\nof \"#\"?\n\nI like the rest. That's basically what I've imagined, and without the\narrows it's even better. :-)\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":"3328","messageId":"7vu0l5zsb4.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050514150356.GK3905@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-14T16:27:27Z","receivedAt":"2005-05-14T16:27:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis writes:\n\nNow I understand which discussion I was missing ;-).\n\nPB> For humans I'd say \"Mode change\" instead of \"mode\" would be better, and\nPB> for machines I still think \"@\" would be better than \"#\". \"#\" can occur\nPB> quite naturally in some code snippets or whatever pasted to the commit\nPB> message, which is extremely unlikely for \"@\". What are the advantages\nPB> of \"#\"?\n\nWait a minute. Aren't we scanning starting from the first\n'---\\n'? Why does what's in commit message matter?\n\nAnd it is not really \"Mode change\" anymore. If you used to have\nfile there and you replaced it with a symlink, that is 100644 to\n120000 \"mode change\". I experimented with different things in\nwhere I have \"# mode: \" there and seriously considered to spell\nit \"# git:\" instead, because that is not really mode and it is\nsomething that means something special to git. Also I tried to\nsay just \"@. \" --- it _was_ confusing to human eye, especially\nif you are used to reading diffs.\n\nWhat I think is that this should not really matter much for\nhuman consumption, because mode change is rare and type change\nis even more rare.\n\nPB> I like the rest. That's basically what I've imagined, and\nPB> without the arrows it's even better. :-)\n\nHere is what I'd propose for you to do. (1) Take the patch as\nis and commit; (2) Change the definition of git_prefix in diff.c\nto \"\\n@. \" and commit; (3) If you already took the test suite,\nmatch t/t2000-diff.sh for the \"\\n@. \" format, and commit. \n\nIt will look something like this. thanks to the leading newline,\nthe output becomes a bit less confusing (without that blank\nline, it really is a disaster for human eyes).\n\n @. 100644 100755 path0\n --- a/path0\n +++ b/path0\n @@ -1,3 +1,3 @@\n Line 1\n Line 2\n -line 3\n +Line 3\n\n @. 100755 . path1\n --- a/path1\n +++ /dev/null\n @@ -1,3 +0,0 @@\n -Line 1\n -Line 2\n -line 3\n\n"},{"id":"3335","messageId":"20050514233538.GY3905@pasky.ji.cz","threadId":"412","inReplyTo":"7vu0l5zsb4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-14T23:35:38Z","receivedAt":"2005-05-14T23:35:38Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Sat, May 14, 2005 at 06:27:27PM CEST, I got a letter\nwhere Junio C Hamano told me that...\n> >>>>> \"PB\" == Petr Baudis writes:\n> \n> Now I understand which discussion I was missing ;-).\n> \n> PB> For humans I'd say \"Mode change\" instead of \"mode\" would be better, and\n> PB> for machines I still think \"@\" would be better than \"#\". \"#\" can occur\n> PB> quite naturally in some code snippets or whatever pasted to the commit\n> PB> message, which is extremely unlikely for \"@\". What are the advantages\n> PB> of \"#\"?\n> \n> Wait a minute. Aren't we scanning starting from the first\n> '---\\n'? Why does what's in commit message matter?\n\nOk, that changes the whole situation. I'll take your patches as they are\nnow in that case. :-)\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":"3344","messageId":"7vr7g9uhsl.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050514233538.GY3905@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-15T06:25:46Z","receivedAt":"2005-05-15T06:25:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis writes:\n\n>> Wait a minute. Aren't we scanning starting from the first\n>> '---\\n'? Why does what's in commit message matter?\n\nPB> Ok, that changes the whole situation. I'll take your patches as they are\nPB> now in that case. :-)\n\nShooooooooot. Seriously.\n\nI already am beginning to like \"\\n@. \" very much; it is much\nless distracting then the \"# mode: \" thing, especially with the\nhelp from additional newline.\n\nCould I have the following applied, pretty please?\n\n------------\nTweak diff output a bit further to make a bit less distracting.\n\nThis adds a blank line before start of diffs for each file, and\nalso changes \"# mode: \" header to \"@. \". One justification is\nthat it tells more than just mode, and \"@. \" is visually a lot\nless distracting.\n\nSigned-off-by: Junio C Hamano \n---\n\n--- a/diff.c\n+++ b/diff.c\n@@ -83,7 +83,7 @@ static void builtin_diff(const char *nam\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 @@ static void builtin_diff(const char *nam\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\n\n"},{"id":"3351","messageId":"20050515093040.GC13024@pasky.ji.cz","threadId":"412","inReplyTo":"7vr7g9uhsl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Petr Baudis","fromEmail":"pasky@ucw.cz","sentAt":"2005-05-15T09:30:40Z","receivedAt":"2005-05-15T09:30:40Z","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 08:25:46AM CEST, I got a letter\nwhere Junio C Hamano told me that...\n> >>>>> \"PB\" == Petr Baudis writes:\n> \n> >> Wait a minute. Aren't we scanning starting from the first\n> >> '---\\n'? Why does what's in commit message matter?\n> \n> PB> Ok, that changes the whole situation. I'll take your patches as they are\n> PB> now in that case. :-)\n> \n> Shooooooooot. Seriously.\n> \n> I already am beginning to like \"\\n@. \" very much; it is much\n> less distracting then the \"# mode: \" thing, especially with the\n> help from additional newline.\n\nI'd argue that it is too little distracting this way. But what I dislike\nmore is that the diff output is now visually inconsistent - some diffs\nare separated by a newline and some aren't.\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":"3371","messageId":"7v8y2guzvp.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"20050515093040.GC13024@pasky.ji.cz","subject":"Re: [PATCH] Resurrect diff-tree-helper -R","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-15T18:07:22Z","receivedAt":"2005-05-15T18:07:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"PB\" == Petr Baudis writes:\n\nPB> ... But what I dislike\nPB> more is that the diff output is now visually inconsistent - some diffs\nPB> are separated by a newline and some aren't.\n\nThat is already fixed in the second patch.\n\n"},{"id":"3372","messageId":"7vzmuwtl6j.fsf@assigned-by-dhcp.cox.net","threadId":"412","inReplyTo":"7vr7g9uhsl.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Tweak diff output further to make it a bit less distracting.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-15T18:10:12Z","receivedAt":"2005-05-15T18:10:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Seriously...\n\nAdds 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\n"}]}