{"thread":{"id":"40346","subject":"[PATCH] Pass amend to pre-commit hook","startedAt":"2015-09-14T12:14:20Z","lastAt":"2015-09-27T22:09:02Z","messageCount":4,"participants":["Alan Clucas","Jeff King","Øystein Walle"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"269914","messageId":"55F6BA1C.7080507@teamwpc.co.uk","threadId":"40346","inReplyTo":null,"subject":"[PATCH] Pass amend to pre-commit hook","fromName":"Alan Clucas","fromEmail":"alan.clucas@teamwpc.co.uk","sentAt":"2015-09-14T12:14:20Z","receivedAt":"2015-09-14T12:14:20Z","isPatch":true,"sender":{"key":"alan.clucas@teamwpc.co.uk","avatar":null},"body":"Pass a single parameter 'amend' to the pre-commit hook when performing a\ncommit amend.\n\nThis allows 'incremental improvement' pre-commit hooks to prevent new\ncode from violating a rule, but also allow the pre-commit hook to pass an\namended commit where the amend has reverted back to the original\ncode (which may not pass that same rule).\n\nExample:\nI have a new whitespace rule. Old code violates this rule and will not\nbe fixed up for blame reasons.\nMy pre-commit hook detects _new_ lines which violate the rule and\nrejects them, however, my original commit passes.\nI amend the code to revert back to the original code (which violates the\nrule). Without this change I cannot detect this is an amend and reject the\nchange (unless --no-verify).\nWith this I can detect this is an amend and verify the patch as a whole\nis not in violation of the rule.\n\nSigned-off-by: Alan Clucas <alan.clucas@teamwpc.co.uk>\n---\nHello,\nThis is my first submission to git, so hopefully I've managed to get the \nformatting right. This patch should be explained above, and would also\nhelp out the folks at overcommit who have this (pretty horrid) solution to\nthe same issue:\nhttps://github.com/brigade/overcommit/issues/146\nhttps://github.com/brigade/overcommit/pull/167\nThanks,\nAlan Clucas\n\n Documentation/githooks.txt | 10 ++++++----\n builtin/commit.c           |  2 +-\n 2 files changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex 7ba0ac9..49d7adb 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -73,10 +73,12 @@ pre-commit\n ~~~~~~~~~~\n \n This hook is invoked by 'git commit', and can be bypassed\n-with `--no-verify` option.  It takes no parameter, and is\n-invoked before obtaining the proposed commit log message and\n-making a commit.  Exiting with non-zero status from this script\n-causes the 'git commit' to abort.\n+with `--no-verify` option.  It takes zero or one parameters.\n+If a parameter is given it will be 'amend' indicating this is\n+a commit amend (if an `--amend` option was given). It is invoked\n+before obtaining the proposed commit log message and making a\n+commit.  Exiting with non-zero status from this script causes the\n+'git commit' to abort.\n \n The default 'pre-commit' hook, when enabled, catches introduction\n of lines with trailing whitespaces and aborts the commit when\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 63772d0..936a614 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -671,7 +671,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t/* This checks and barfs if author is badly specified */\n \tdetermine_author_info(author_ident);\n \n-\tif (!no_verify && run_commit_hook(use_editor, index_file, \"pre-commit\", NULL))\n+\tif (!no_verify && run_commit_hook(use_editor, index_file, \"pre-commit\", amend?\"amend\":NULL, NULL))\n \t\treturn 0;\n \n \tif (squash_message) {\n-- \n2.4.1.168.g1ea28e1\n"},{"id":"269926","messageId":"20150914144727.GA25003@sigill.intra.peff.net","threadId":"40346","inReplyTo":"55F6BA1C.7080507@teamwpc.co.uk","subject":"Re: [PATCH] Pass amend to pre-commit hook","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-09-14T14:47:28Z","receivedAt":"2015-09-14T14:47:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote:\n\n> Pass a single parameter 'amend' to the pre-commit hook when performing a\n> commit amend.\n\nI think this is a sensible thing to want, and it has come up a few\ntimes. I'm not sure why the last round didn't get merged, though. Looks\nlike it just slipped through the cracks.\n\nHere are the relevant threads:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/260122\n\n  http://thread.gmane.org/gmane.comp.version-control.git/260245\n\nLooks like there was some question of what to pass in the normal,\nnon-amend case. I've added interested parties from the original thread\nto the cc here.\n\n-Peff\n"},{"id":"269931","messageId":"55F6FA8D.6030509@teamwpc.co.uk","threadId":"40346","inReplyTo":"20150914144727.GA25003@sigill.intra.peff.net","subject":"Re: [PATCH] Pass amend to pre-commit hook","fromName":"Alan Clucas","fromEmail":"alan.clucas@teamwpc.co.uk","sentAt":"2015-09-14T16:49:17Z","receivedAt":"2015-09-14T16:49:17Z","isPatch":true,"sender":{"key":"alan.clucas@teamwpc.co.uk","avatar":null},"body":"On 14/09/15 15:47, Jeff King wrote:\n> On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote:\n>\n>> Pass a single parameter 'amend' to the pre-commit hook when performing a\n>> commit amend.\n>\n> I think this is a sensible thing to want, and it has come up a few\n> times. I'm not sure why the last round didn't get merged, though. Looks\n> like it just slipped through the cracks.\n>\n> Here are the relevant threads:\n>\n>    http://thread.gmane.org/gmane.comp.version-control.git/260122\n>\n>    http://thread.gmane.org/gmane.comp.version-control.git/260245\n>\n> Looks like there was some question of what to pass in the normal,\n> non-amend case. I've added interested parties from the original thread\n> to the cc here.\n>\n> -Peff\n>\n\nAh thanks. My google-fu didn't find any of those threads, I should have \ntried specifically searching the archives I guess.\n\nMaybe with a little cajoling we could get some version of this passed. \nThe issue that came up before was my worry, I didn't like defining the \ninterface that way in case it needed extending... (but other hooks also \nhave bad interfaces, so wasn't sure what precedent to follow).\n\nAlan\n"},{"id":"270799","messageId":"loom.20150928T000850-141@post.gmane.org","threadId":"40346","inReplyTo":"20150914144727.GA25003@sigill.intra.peff.net","subject":"Re: [PATCH] Pass amend to pre-commit hook","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2015-09-27T22:09:02Z","receivedAt":"2015-09-27T22:09:02Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Jeff King <peff <at> peff.net> writes:\n\n> \n> On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote:\n> \n> > Pass a single parameter 'amend' to the pre-commit hook when performing a\n> > commit amend.\n> \n> I think this is a sensible thing to want, and it has come up a few\n> times. I'm not sure why the last round didn't get merged, though. Looks\n> like it just slipped through the cracks.\n> \n> Here are the relevant threads:\n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/260122\n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/260245\n> \n> Looks like there was some question of what to pass in the normal,\n> non-amend case. I've added interested parties from the original thread\n> to the cc here.\n> \n> -Peff\n> \n\nThere were so many different ways of solving them that we weren't able\nto decide between them:\n\nAssuming we give the string \"amend\" as the hook's argv[1], what to do\nwhen --amend is not used? We can pass nothing, or the empty string, or\nthe string \"noamend\". Then there was the suggestion of exporting\nGIT_AMEND=1 or something like that. In any case, what to do when we want\nto pass more arguments? Should we let the hook check argv[1] to decide\nwhether --amend was used, argv[2] to check whether {some scenario here}\nis the case? Or make the hook author effectively implement options\nparsing?\n\nAnd then it died out...\n\nIn my totally unprofessional opinion anything more complex than maybe\npassing \"amend\" in argv[1] is unwarranted. Since I posted my first patch\nalmost a year ago there has been no discussion regarding passing other\ninformation along to pre-commit. Furthermore --amend is the only thing I\nknow of that a pre-commit hook cannot discover on its own. On the other\nhand the desire for this has popped up at least twice on #git in the\nsame time span.\n\nAlan Clucas' solution looks fine to me. It is essentially the same as\nmine. But mine had tests and whatnot ;-)\n\nØsse\n\n"}]}