{"thread":{"id":"7048","subject":"[PATCH] Fix typo on variable name $newref should be $newrev in sample update hook","startedAt":"2007-03-02T10:27:00Z","lastAt":"2007-03-04T21:44:46Z","messageCount":9,"participants":["Andy Parkins","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"36080","messageId":"200703021027.00203.andyparkins@gmail.com","threadId":"7048","inReplyTo":null,"subject":"[PATCH] Fix typo on variable name $newref should be $newrev in sample update hook","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-03-02T10:27:00Z","receivedAt":"2007-03-02T10:27:00Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"The log message would be empty for new branches because git-rev-list was\nbeing passed $newref instead of the correct variable $newrev.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\n templates/hooks--update |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/templates/hooks--update b/templates/hooks--update\nindex fd1f73d..2a707e5 100644\n--- a/templates/hooks--update\n+++ b/templates/hooks--update\n@@ -148,7 +148,7 @@ case \"$refname_type\" in\n \t\t\t# This shows all log entries that are not already covered by\n \t\t\t# another ref - i.e. commits that are now accessible from this\n \t\t\t# ref that were previously not accessible\n-\t\t\tgit-rev-parse --not --all | git-rev-list --stdin --pretty $newref\n+\t\t\tgit-rev-parse --not --all | git-rev-list --stdin --pretty $newrev\n \t\t\techo $LOGEND\n \t\telse\n \t\t\t# oldrev is valid\n-- \n1.5.0.2.205.g74e20\n"},{"id":"36109","messageId":"Pine.LNX.4.64.0703020839350.3953@woody.linux-foundation.org","threadId":"7048","inReplyTo":"200703021027.00203.andyparkins@gmail.com","subject":"Re: [PATCH] Fix typo on variable name $newref should be $newrev in sample update hook","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-02T16:44:06Z","receivedAt":"2007-03-02T16:44:06Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 2 Mar 2007, Andy Parkins wrote:\n>  \t\t\t# ref that were previously not accessible\n> -\t\t\tgit-rev-parse --not --all | git-rev-list --stdin --pretty $newref\n> +\t\t\tgit-rev-parse --not --all | git-rev-list --stdin --pretty $newrev\n\nWell, it should just avoid git-rev-parse these days and do\n\n\tgit rev-list --pretty $newrev --not --all\n\ninstead. And quite frankly, rather than \"git rev-list --pretty\", there's \nno real reason not to do\n\n\tgit log $newrev --not --all\n\nunless there is some subtle issue with pagers (but none of this goes to a \nterminal?) in which case you'd have to disable that (and then rev-list is \nprobably simpler and more readable than adding a \"GIT_PAGER=\" to the \nfront).\n\n\t\tLinus\n"},{"id":"36118","messageId":"200703021925.09441.andyparkins@gmail.com","threadId":"7048","inReplyTo":"Pine.LNX.4.64.0703020839350.3953@woody.linux-foundation.org","subject":"Re: [PATCH] Fix typo on variable name $newref should be $newrev in sample update hook","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-03-02T19:25:05Z","receivedAt":"2007-03-02T19:25:05Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Friday 2007, March 02, Linus Torvalds wrote:\n\n> Well, it should just avoid git-rev-parse these days and do\n>\n> \tgit rev-list --pretty $newrev --not --all\n\nAh - I didn't know that was possible.  I thought that the --not switch \nwould invert all the arguments not just those following it.\n\nPatch to follow.\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"36119","messageId":"200703021929.20969.andyparkins@gmail.com","threadId":"7048","inReplyTo":"Pine.LNX.4.64.0703020839350.3953@woody.linux-foundation.org","subject":"[PATCH] Remove use of git-rev-parse and replace git-rev-list --pretty with git-log","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-03-02T19:29:20Z","receivedAt":"2007-03-02T19:29:20Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"Linus noticed:\n\n--- >8 ---\nit should just avoid git-rev-parse these days and do\n\n  git rev-list --pretty $newrev --not --all\n\ninstead. And quite frankly, rather than \"git rev-list --pretty\", there's\nno real reason not to do\n\n  git log $newrev --not --all\n--- 8< ---\n\nThis patch makes those changes.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\n templates/hooks--update |    7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/templates/hooks--update b/templates/hooks--update\nindex 2a707e5..f580360 100644\n--- a/templates/hooks--update\n+++ b/templates/hooks--update\n@@ -148,7 +148,7 @@ case \"$refname_type\" in\n \t\t\t# This shows all log entries that are not already covered by\n \t\t\t# another ref - i.e. commits that are now accessible from this\n \t\t\t# ref that were previously not accessible\n-\t\t\tgit-rev-parse --not --all | git-rev-list --stdin --pretty $newrev\n+\t\t\tgit log $newrev --not --all\n \t\t\techo $LOGEND\n \t\telse\n \t\t\t# oldrev is valid\n@@ -165,7 +165,7 @@ case \"$refname_type\" in\n \t\t\tbaserev=$(git-merge-base $oldrev $newrev)\n \n \t\t\t# Commit with a parent\n-\t\t\tfor rev in $(git-rev-parse --not --all | git-rev-list --stdin $newrev ^$baserev)\n+\t\t\tfor rev in $(git-rev-list $newrev ^$baserev --not --all)\n \t\t\tdo\n \t\t\t\trevtype=$(git-cat-file -t \"$rev\")\n \t\t\t\techo \"       via  $rev ($revtype)\"\n@@ -190,8 +190,7 @@ case \"$refname_type\" in\n \t\t\tfi\n \t\t\techo \"\"\n \t\t\techo $LOGBEGIN\n-\t\t\tgit-rev-parse --not --all |\n-\t\t\tgit-rev-list --stdin --pretty $newrev ^$baserev\n+\t\t\tgit log $newrev ^$baserev --not --all\n \t\t\techo $LOGEND\n \t\t\techo \"\"\n \t\t\techo \"Diffstat:\"\n-- \n1.5.0.rc4.gb4d2\n"},{"id":"36125","messageId":"Pine.LNX.4.64.0703021246480.3953@woody.linux-foundation.org","threadId":"7048","inReplyTo":"200703021929.20969.andyparkins@gmail.com","subject":"Re: [PATCH] Remove use of git-rev-parse and replace git-rev-list --pretty with git-log","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-02T20:48:25Z","receivedAt":"2007-03-02T20:48:25Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 2 Mar 2007, Andy Parkins wrote:\n>  \t\t\t# Commit with a parent\n> -\t\t\tfor rev in $(git-rev-parse --not --all | git-rev-list --stdin $newrev ^$baserev)\n> +\t\t\tfor rev in $(git-rev-list $newrev ^$baserev --not --all)\n\nLooks fine, and I don't think it's worth changing, but you can avoid the \nextra caret by just moving \"$baserev\" to after the \"--not\", ie writing the \nthing as\n\n\tgit-rev-list $newrev --not $baserev --all\n\ninstead if you want to.\n\n> -\t\t\tgit-rev-parse --not --all |\n> -\t\t\tgit-rev-list --stdin --pretty $newrev ^$baserev\n> +\t\t\tgit log $newrev ^$baserev --not --all\n\nAnd that's true here too.\n\n\t\tLinus\n"},{"id":"36138","messageId":"7vbqjbdyxc.fsf@assigned-by-dhcp.cox.net","threadId":"7048","inReplyTo":"200703021929.20969.andyparkins@gmail.com","subject":"Re: [PATCH] Remove use of git-rev-parse and replace git-rev-list --pretty with git-log","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-02T23:10:39Z","receivedAt":"2007-03-02T23:10:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> Linus noticed:\n>\n> --- >8 ---\n> it should just avoid git-rev-parse these days and do\n>\n>   git rev-list --pretty $newrev --not --all\n>\n> instead. And quite frankly, rather than \"git rev-list --pretty\", there's\n> no real reason not to do\n>\n>   git log $newrev --not --all\n> --- 8< ---\n>\n> This patch makes those changes.\n>\n> Signed-off-by: Andy Parkins <andyparkins@gmail.com>\n\nPlease look at your Subject: line above and ponder how it would\nlook in the next issue of \"What's cooking in git.git\".  Do I\nhave to spend extra brain cycles to go back to \"git log --stat\"\nand realize that this one only updates a sample hook script?\n\nAlso, please don't do \"--- >8 ---\".  If you want to, please use\ntwo dashes; this is purely for technical reasons.\n\nI'll massage the log message and move \"^$base --not\" around as\nLinus suggested, but next time please be a bit more careful.\n"},{"id":"36157","messageId":"200703030825.18378.andyparkins@gmail.com","threadId":"7048","inReplyTo":"7vbqjbdyxc.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Remove use of git-rev-parse and replace git-rev-list --pretty with git-log","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-03-03T08:25:16Z","receivedAt":"2007-03-03T08:25:16Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Friday 2007, March 02, Junio C Hamano wrote:\n\n> Please look at your Subject: line above and ponder how it would\n> look in the next issue of \"What's cooking in git.git\".  Do I\n> have to spend extra brain cycles to go back to \"git log --stat\"\n> and realize that this one only updates a sample hook script?\n\nApologies.  It's easy to drop into focusing narrowly on on your own \npatch and forget about the larger picture.\n\n> Also, please don't do \"--- >8 ---\".  If you want to, please use\n> two dashes; this is purely for technical reasons.\n\nI'm happy to comply of course.  However, this seems like a bug in git to \nme.  This makes it so that some content is not allowed in a log \nmessage, which seems very much out of keeping with git's normal \"I can \nhandle anything\" stance.\n\nFinding the \"---\" separator between diff and log message could at least \nrely on finding \"---\" alone on a line so that \"--- something else\" \nwouldn't trigger the end of log?\n\nI assume this is too simple?\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 766a37e..4e0795a 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -670,7 +670,7 @@ static int handle_commit_msg(int *seen)\n        return 0;\n    do {\n        if (!memcmp(\"diff -\", line, 6) ||\n-           !memcmp(\"---\", line, 3) ||\n+           !memcmp(\"---\\n\", line, 4) ||\n            !memcmp(\"Index: \", line, 7))\n            break;\n        if ((multipart_boundary[0] && is_multipart_boundary(line))) {\n\n> I'll massage the log message and move \"^$base --not\" around as\n> Linus suggested, but next time please be a bit more careful.\n\nI will try.  Once again, my apologies.\n\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIET\nandyparkins@gmail.com\n"},{"id":"36167","messageId":"7virdiczn0.fsf@assigned-by-dhcp.cox.net","threadId":"7048","inReplyTo":"200703030825.18378.andyparkins@gmail.com","subject":"Re: [PATCH] Remove use of git-rev-parse and replace git-rev-list --pretty with git-log","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-03T11:52:51Z","receivedAt":"2007-03-03T11:52:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> Finding the \"---\" separator between diff and log message could at least \n> rely on finding \"---\" alone on a line so that \"--- something else\" \n> wouldn't trigger the end of log?\n>\n> I assume this is too simple?\n>\n> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\n> index 766a37e..4e0795a 100644\n> --- a/builtin-mailinfo.c\n> +++ b/builtin-mailinfo.c\n> @@ -670,7 +670,7 @@ static int handle_commit_msg(int *seen)\n>         return 0;\n>     do {\n>         if (!memcmp(\"diff -\", line, 6) ||\n> -           !memcmp(\"---\", line, 3) ||\n> +           !memcmp(\"---\\n\", line, 4) ||\n>             !memcmp(\"Index: \", line, 7))\n>             break;\n>         if ((multipart_boundary[0] && is_multipart_boundary(line))) {\n\nI think that would make us reject patches that we currently\naccept.  Today, you can submit a patch created this way:\n\n\t$ cp Makefile Makefile.orig\n        $ edit Makefile\n\n\t$ cat body-of-message\n        $ echo \"Signed-off-by: $me\"\n        $ diff -u Makefile.orig Makefile\n\nThe first line of the diff part would be \"--- Makefile.orig\"\n(and the second one begins with \"+++ Makefile\").\n\nI do not think I've asked Linus about how he originally came up\nwith \"three dashes at the beginning of line\" convention in the\nkernel circle, but my guess always have been that it would\nautomatically allow us to accept patches prepared like this.\n"},{"id":"36250","messageId":"Pine.LNX.4.64.0703041339070.3953@woody.linux-foundation.org","threadId":"7048","inReplyTo":"7virdiczn0.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Remove use of git-rev-parse and replace git-rev-list --pretty with git-log","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-03-04T21:44:46Z","receivedAt":"2007-03-04T21:44:46Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 3 Mar 2007, Junio C Hamano wrote:\n> \n> I do not think I've asked Linus about how he originally came up\n> with \"three dashes at the beginning of line\" convention in the\n> kernel circle, but my guess always have been that it would\n> automatically allow us to accept patches prepared like this.\n\nIndeed. The three dashes are just two different usage cases:\n\n - bare patches. So we stop on \n\n\t--- Makefile.orig\n\t+++ Makefile\n\n   exactly the same way we stop of \"^diff -\" and \"Index: \"\n\n - that then meant that I also extended it to ask people to add comments \n   after a \"---\" section.\n\nThat said, we could certainly require that it be:\n\n - either just three dashes on the line\nOR\n - three dashes, exactly one whitespace, and what looks like a filename \n   (which in turn can have a date after it, that's what many versions of \n   GNU patch do)\n\nbut it won't remove the case where if somebody writes some text message \nthat really *looks* like the beginning of a patch, it's what git \n\"mailinfo\" will use to split the patch from the top of the message. So we \ncan tighten it up a bit, but people still need to be aware of this!\n\n\t\tLinus\n"}]}