Re: [PATCH] Pass amend to pre-commit hook
Jeff King peff.net> writes: > > On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote: > > > Pass a single parameter 'amend' to the pre-commit hook when performing a > > commit amend. > > I think this is a sensible thing to want, and it has come up a few > times. I'm not sure why the last round didn't get merged, though. Looks > like it just slipped through the cracks. > > Here are the relevant threads: > > http://thread.gmane.org/gmane.comp.version-control.git/260122 > > http://thread.gmane.org/gmane.comp.version-control.git/260245 > > Looks like there was some question of what to pass in the normal, > non-amend case. I've added interested parties from the original thread > to the cc here. > > -Peff > There were so many different ways of solving them that we weren't able to decide between them: Assuming we give the string "amend" as the hook's argv[1], what to do when --amend is not used? We can pass nothing, or the empty string, or the string "noamend". Then there was the suggestion of exporting GIT_AMEND=1 or something like that. In any case, what to do when we want to pass more arguments? Should we let the hook check argv[1] to decide whether --amend was used, argv[2] to check whether {some scenario here} is the case? Or make the hook author effectively implement options parsing? And then it died out... In my totally unprofessional opinion anything more complex than maybe passing "amend" in argv[1] is unwarranted. Since I posted my first patch almost a year ago there has been no discussion regarding passing other information along to pre-commit. Furthermore --amend is the only thing I know of that a pre-commit hook cannot discover on its own. On the other hand the desire for this has popped up at least twice on #git in the same time span. Alan Clucas' solution looks fine to me. It is essentially the same as mine. But mine had tests and whatnot ;-) Øsse
Re: [PATCH] Pass amend to pre-commit hook
On 14/09/15 15:47, Jeff King wrote: On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote: Pass a single parameter 'amend' to the pre-commit hook when performing a commit amend. I think this is a sensible thing to want, and it has come up a few times. I'm not sure why the last round didn't get merged, though. Looks like it just slipped through the cracks. Here are the relevant threads: http://thread.gmane.org/gmane.comp.version-control.git/260122 http://thread.gmane.org/gmane.comp.version-control.git/260245 Looks like there was some question of what to pass in the normal, non-amend case. I've added interested parties from the original thread to the cc here. -Peff Ah thanks. My google-fu didn't find any of those threads, I should have tried specifically searching the archives I guess. Maybe with a little cajoling we could get some version of this passed. The issue that came up before was my worry, I didn't like defining the interface that way in case it needed extending... (but other hooks also have bad interfaces, so wasn't sure what precedent to follow). Alan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Pass amend to pre-commit hook
On Mon, Sep 14, 2015 at 01:14:20PM +0100, Alan Clucas wrote: > Pass a single parameter 'amend' to the pre-commit hook when performing a > commit amend. I think this is a sensible thing to want, and it has come up a few times. I'm not sure why the last round didn't get merged, though. Looks like it just slipped through the cracks. Here are the relevant threads: http://thread.gmane.org/gmane.comp.version-control.git/260122 http://thread.gmane.org/gmane.comp.version-control.git/260245 Looks like there was some question of what to pass in the normal, non-amend case. I've added interested parties from the original thread to the cc here. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Pass amend to pre-commit hook
Pass a single parameter 'amend' to the pre-commit hook when performing a commit amend. This allows 'incremental improvement' pre-commit hooks to prevent new code from violating a rule, but also allow the pre-commit hook to pass an amended commit where the amend has reverted back to the original code (which may not pass that same rule). Example: I have a new whitespace rule. Old code violates this rule and will not be fixed up for blame reasons. My pre-commit hook detects _new_ lines which violate the rule and rejects them, however, my original commit passes. I amend the code to revert back to the original code (which violates the rule). Without this change I cannot detect this is an amend and reject the change (unless --no-verify). With this I can detect this is an amend and verify the patch as a whole is not in violation of the rule. Signed-off-by: Alan Clucas --- Hello, This is my first submission to git, so hopefully I've managed to get the formatting right. This patch should be explained above, and would also help out the folks at overcommit who have this (pretty horrid) solution to the same issue: https://github.com/brigade/overcommit/issues/146 https://github.com/brigade/overcommit/pull/167 Thanks, Alan Clucas Documentation/githooks.txt | 10 ++ builtin/commit.c | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index 7ba0ac9..49d7adb 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -73,10 +73,12 @@ pre-commit ~~ This hook is invoked by 'git commit', and can be bypassed -with `--no-verify` option. It takes no parameter, and is -invoked before obtaining the proposed commit log message and -making a commit. Exiting with non-zero status from this script -causes the 'git commit' to abort. +with `--no-verify` option. It takes zero or one parameters. +If a parameter is given it will be 'amend' indicating this is +a commit amend (if an `--amend` option was given). It is invoked +before obtaining the proposed commit log message and making a +commit. Exiting with non-zero status from this script causes the +'git commit' to abort. The default 'pre-commit' hook, when enabled, catches introduction of lines with trailing whitespaces and aborts the commit when diff --git a/builtin/commit.c b/builtin/commit.c index 63772d0..936a614 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -671,7 +671,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, /* This checks and barfs if author is badly specified */ determine_author_info(author_ident); - if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL)) + if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", amend?"amend":NULL, NULL)) return 0; if (squash_message) { -- 2.4.1.168.g1ea28e1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html