{"thread":{"id":"2242","subject":"gitk shows an empty line between \"Comments\" and changed files","startedAt":"2005-10-27T17:30:30Z","lastAt":"2005-10-29T04:54:00Z","messageCount":11,"participants":["Pavel Roskin","Junio C Hamano","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"10709","messageId":"1130434230.19641.21.camel@dv","threadId":"2242","inReplyTo":null,"subject":"gitk shows an empty line between \"Comments\" and changed files","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2005-10-27T17:30:30Z","receivedAt":"2005-10-27T17:30:30Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Hello!\n\ngitk is now showing an empty line between \"Comments\" and changes files.\nThis change was caused by 8b7e5d76e836396a097bb6f61cf930ea872a7bd3 (Make\n\"gitk\" work better with dense revlists).\n\ngit-diff-tree outputs some SHA1 as the first line if only one tree-ish\nargument is provided. I don't see any way to suppress that output (see\ndiff-tree.c, line 114).\n\nOne solution would be to add an option to git-diff-tree to suppress all\nheaders (let's call it --no-headers).\n\nOr maybe the SHA1 header should never be printed at all? It looks like\nit's not documented anywhere. It doesn't break the tests.\n\nWhile debugging the patch, I have found that the p variable is unused in\nboth functions that stopped passing it to git-diff-tree.\n\nAlso, gettreediffs function in gitk could use --names-only for\ngit-diff-tree, because it only needs names.\n\nMaybe --names-only and --name-status should suppress the SHA1 header to\nmatch their descriptions? Unfortunately, they don't suppress the patch\nis -p is specified, so \"matching the descriptions\" would have to take\ncare of it.\n\nP.S. I consider printing the SHA1 header in git-diff-tree an\nundocumented feature (in other words, a bug).\n\nProposed patch:\n\nDon't print the SHA1 when only one tree-ish is given to git-diff-tree.\n\nSigned-off-by: Pavel Roskin \n\ndiff --git a/diff-tree.c b/diff-tree.c\nindex 382011a..ac53f48 100644\n--- a/diff-tree.c\n+++ b/diff-tree.c\n@@ -111,7 +111,6 @@ static int diff_tree_commit(const unsign\n \t\tunsigned char parent[20];\n \t\tif (get_sha1_hex(buf + offset + 7, parent))\n \t\t\treturn -1;\n-\t\theader = generate_header(name, sha1_to_hex(parent), buf, size);\n \t\tdiff_tree_sha1_top(parent, commit, \"\");\n \t\tif (!header && verbose_header) {\n \t\t\theader_prefix = \"\\ndiff-tree \";\n\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"10712","messageId":"7vslum3l2w.fsf@assigned-by-dhcp.cox.net","threadId":"2242","inReplyTo":"1130434230.19641.21.camel@dv","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-27T17:51:03Z","receivedAt":"2005-10-27T17:51:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavel Roskin writes:\n\n> Or maybe the SHA1 header should never be printed at all? It looks like\n> it's not documented anywhere. It doesn't break the tests.\n\nAFAIK, its only user (except humans) is patch-id.\n\n> P.S. I consider printing the SHA1 header in git-diff-tree an\n> undocumented feature (in other words, a bug).\n"},{"id":"10728","messageId":"1130463389.2186.14.camel@dv","threadId":"2242","inReplyTo":"7vslum3l2w.fsf@assigned-by-dhcp.cox.net","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2005-10-28T01:36:29Z","receivedAt":"2005-10-28T01:36:29Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Hi, Junio!\n\nOn Thu, 2005-10-27 at 10:51 -0700, Junio C Hamano wrote:\n> Pavel Roskin writes:\n> \n> > Or maybe the SHA1 header should never be printed at all? It looks like\n> > it's not documented anywhere. It doesn't break the tests.\n> \n> AFAIK, its only user (except humans) is patch-id.\n\nThank you for reply! git-patch-id is badly documented, and its output\nis not documented at all. It outputs two SHA1 hashes, and the second\none is taken from the line produced by git-diff-tree. If there is no\nsuch line, the second ID is 0.\n\ngit-patch-id is only used by git-cherry. git-cherry writes the second\nSHA1 to some files in a temporary directory, but it never reads those\nfiles, it only checks that they exist.\n\nI'm pretty confident now that the patch I posted in the last message was\ncorrect.\n\nOf course, more cleanup will be needed to remove the second ID from\ngit-patch-id, to adjust git-cherry accordingly and to remove \"p\"\nassignments in gitk.\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"10754","messageId":"7v4q72xavz.fsf@assigned-by-dhcp.cox.net","threadId":"2242","inReplyTo":"1130463389.2186.14.camel@dv","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-28T09:13:04Z","receivedAt":"2005-10-28T09:13:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavel Roskin writes:\n\n> git-patch-id is only used by git-cherry. git-cherry writes the second\n> SHA1 to some files in a temporary directory, but it never reads those\n> files, it only checks that they exist.\n\nI do not oppose dropping the commit-id line from the default\noutput, but having it optionally available would be useful in\none application. Somebody _could_ write a tool that does\nsomething like:\n\n git-rev-list ^$old_head $new_head |\n git-diff-tree -p -m --stdin --with-commit-ids |\n git-patch-id\n\nto cache the patch-id --> commit-id mappings. If this were kept\non the upstream repo for public query, it would be useful for\nyou to find out if your favorite patch as already been merged.\nFor example, gitweb could have an query page to let you submit a\npatch-id and return the commit (or \"no such patch merged yet\").\n"},{"id":"10768","messageId":"1130539503.10531.43.camel@dv","threadId":"2242","inReplyTo":"7v4q72xavz.fsf@assigned-by-dhcp.cox.net","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2005-10-28T22:45:03Z","receivedAt":"2005-10-28T22:45:03Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Hi, Junio!\n\nOn Fri, 2005-10-28 at 02:13 -0700, Junio C Hamano wrote:\n> Pavel Roskin writes:\n> \n> > git-patch-id is only used by git-cherry. git-cherry writes the second\n> > SHA1 to some files in a temporary directory, but it never reads those\n> > files, it only checks that they exist.\n> \n> I do not oppose dropping the commit-id line from the default\n> output, but having it optionally available would be useful in\n> one application. Somebody _could_ write a tool that does\n> something like:\n> \n> git-rev-list ^$old_head $new_head |\n> git-diff-tree -p -m --stdin --with-commit-ids |\n> git-patch-id\n\nSounds good. Perhaps the commit IDs should have a prefix identifying\nthem.\n\nAnother approach would be to use something slightly more elaborate than\na pipe. If I understand correctly, the commit ID would be already known\nfrom the git-rev-list output. Passing commit IDs through patch-id\nwithout actually doing anything with them seems non-elegant. Maybe we\ncould teach git-patch-id (or another script) to get patches by commit-id\ninstead of using stdin?\n\n-- \nRegards,\nPavel Roskin\n"},{"id":"10769","messageId":"Pine.LNX.4.64.0510281557580.3348@g5.osdl.org","threadId":"2242","inReplyTo":"1130539503.10531.43.camel@dv","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-28T23:00:41Z","receivedAt":"2005-10-28T23:00:41Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 28 Oct 2005, Pavel Roskin wrote:\n> > \n> > I do not oppose dropping the commit-id line from the default\n> > output, but having it optionally available would be useful in\n> > one application. Somebody _could_ write a tool that does\n> > something like:\n> > \n> > git-rev-list ^$old_head $new_head |\n> > git-diff-tree -p -m --stdin --with-commit-ids |\n> > git-patch-id\n> \n> Sounds good. Perhaps the commit IDs should have a prefix identifying\n> them.\n\nGuys, why do you want to drop it? We've always had it, and it doesn't \nreally hurt.\n\nYes, gitk got a new empty line because I didn't realize that the output of \n\"git-diff-tree $commit\" is slightly different from \"git-diff-tree $t2 \n$t2\", but hey, that was due to a gitk change, and I think it should be \ntrivial for gitk to just react to it.\n\nSo if we add a new flag, please make it go the other way: one that makes \nthe output really quiet, but keeps the standard output the same.\n\n\t\tLinus\n"},{"id":"10773","messageId":"7vslul2g29.fsf@assigned-by-dhcp.cox.net","threadId":"2242","inReplyTo":"1130539503.10531.43.camel@dv","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-29T02:49:18Z","receivedAt":"2005-10-29T02:49:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pavel Roskin writes:\n\n>> git-rev-list ^$old_head $new_head |\n>> git-diff-tree -p -m --stdin --with-commit-ids |\n>> git-patch-id\n>\n> Sounds good. Perhaps the commit IDs should have a prefix identifying\n> them.\n\nI do not think git-diff-tree -p output can have 40-byte\nhexadecimal at the beginning of the output anywhere other than\ncommit object names; why clutter output?\n\n> Another approach would be to use something slightly more elaborate than\n> a pipe. If I understand correctly, the commit ID would be already known\n> from the git-rev-list output. Passing commit IDs through patch-id\n> without actually doing anything with them seems non-elegant.\n\nSorry you lost me. I am not sure what you mean by \"without\nactually doing anything\" part. The input to patch-id command in\nthe above pipe is (commit-object-name patch)*. The command\nreads such a stream, and transforms it to a (patch-id\ncommit-object-name)* stream. In other words, the input\nidentifies each patch with a commit-object-name, and the command\ncondenses each patch to a patch-id, and spits them out, labelled\nwith commit-object-name.\n"},{"id":"10774","messageId":"Pine.LNX.4.64.0510282003360.3348@g5.osdl.org","threadId":"2242","inReplyTo":"7vslul2g29.fsf@assigned-by-dhcp.cox.net","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-29T03:07:26Z","receivedAt":"2005-10-29T03:07:26Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 28 Oct 2005, Junio C Hamano wrote:\n> \n> Sorry you lost me. I am not sure what you mean by \"without\n> actually doing anything\" part. The input to patch-id command in\n> the above pipe is (commit-object-name patch)*. The command\n> reads such a stream, and transforms it to a (patch-id\n> commit-object-name)* stream. In other words, the input\n> identifies each patch with a commit-object-name, and the command\n> condenses each patch to a patch-id, and spits them out, labelled\n> with commit-object-name.\n\nNote that git-patch-id will happily take a patch without the commit ID at \nthe head, it just won't have a commit ID to match it up with. For such \npatches it will just spit it out with an all-zero commit-object-name.\n\nAnd that's very much by design. The point is that you can match up your \n(perhaps non-git) patches with what has been accepted. Which is why \ngit-patch-id should always take non-git patches too, and then you can \nmatch them up by sorting by patch ID and doing \"join -1\" to match up \nduplicates.\n\nSo git-patch-id will work with or without the commit ID, but the commit ID \nis then later needed to figure out _which_ commit you matched up.\n\n\t\tLinus\n"},{"id":"10783","messageId":"20051029002357.iq88cg8gcsogk4kc@webmail.spamcop.net","threadId":"2242","inReplyTo":"Pine.LNX.4.64.0510282003360.3348@g5.osdl.org","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2005-10-29T04:23:57Z","receivedAt":"2005-10-29T04:23:57Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Quoting Linus Torvalds :\n\n> Note that git-patch-id will happily take a patch without the commit ID at\n> the head, it just won't have a commit ID to match it up with. For such\n> patches it will just spit it out with an all-zero commit-object-name.\n>\n> And that's very much by design.\n[snip]\n\nOK, if it's by design, I'll fix gitk only.\n\nI understand the default behavior of git-diff-tree won't change, so I'll simply\nstrip the first line.\n\n--\nRegards,\nPavel Roskin\n"},{"id":"10784","messageId":"Pine.LNX.4.64.0510282132120.3348@g5.osdl.org","threadId":"2242","inReplyTo":"20051029002357.iq88cg8gcsogk4kc@webmail.spamcop.net","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-10-29T04:33:03Z","receivedAt":"2005-10-29T04:33:03Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 29 Oct 2005, Pavel Roskin wrote:\n> \n> I understand the default behavior of git-diff-tree won't change, so I'll simply\n> strip the first line.\n\nBe careful, though. The merge case uses git-diff-tree differently, so I \nthink there is no extra line for a merge. \n\n\t\tLinus\n"},{"id":"10785","messageId":"20051029005400.s1ge8c4ok0wo4s4s@webmail.spamcop.net","threadId":"2242","inReplyTo":"Pine.LNX.4.64.0510282132120.3348@g5.osdl.org","subject":"Re: gitk shows an empty line between \"Comments\" and changed files","fromName":"Pavel Roskin","fromEmail":"proski@gnu.org","sentAt":"2005-10-29T04:54:00Z","receivedAt":"2005-10-29T04:54:00Z","isPatch":false,"sender":{"key":"proski@gnu.org","avatar":null},"body":"Quoting Linus Torvalds :\n\n> On Sat, 29 Oct 2005, Pavel Roskin wrote:\n> >\n> > I understand the default behavior of git-diff-tree won't change, so I'll\n> simply\n> > strip the first line.\n>\n> Be careful, though. The merge case uses git-diff-tree differently, so I\n> think there is no extra line for a merge.\n\nSure. Thanks.\n\n--\nRegards,\nPavel Roskin\n"}]}