* [PATCH] Pass amend to pre-commit hook
@ 2015-09-14 12:14 Alan Clucas
2015-09-14 14:47 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Alan Clucas @ 2015-09-14 12:14 UTC (permalink / raw)
To: git
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 <alan.clucas@teamwpc.co.uk>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Pass amend to pre-commit hook
2015-09-14 12:14 [PATCH] Pass amend to pre-commit hook Alan Clucas
@ 2015-09-14 14:47 ` Jeff King
2015-09-14 16:49 ` Alan Clucas
2015-09-27 22:09 ` Øystein Walle
0 siblings, 2 replies; 4+ messages in thread
From: Jeff King @ 2015-09-14 14:47 UTC (permalink / raw)
To: Alan Clucas; +Cc: Øystein Walle, Mark Levedahl, git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Pass amend to pre-commit hook
2015-09-14 14:47 ` Jeff King
@ 2015-09-14 16:49 ` Alan Clucas
2015-09-27 22:09 ` Øystein Walle
1 sibling, 0 replies; 4+ messages in thread
From: Alan Clucas @ 2015-09-14 16:49 UTC (permalink / raw)
To: Jeff King; +Cc: Øystein Walle, Mark Levedahl, git
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Pass amend to pre-commit hook
2015-09-14 14:47 ` Jeff King
2015-09-14 16:49 ` Alan Clucas
@ 2015-09-27 22:09 ` Øystein Walle
1 sibling, 0 replies; 4+ messages in thread
From: Øystein Walle @ 2015-09-27 22:09 UTC (permalink / raw)
To: git
Jeff King <peff <at> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-27 22:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 12:14 [PATCH] Pass amend to pre-commit hook Alan Clucas
2015-09-14 14:47 ` Jeff King
2015-09-14 16:49 ` Alan Clucas
2015-09-27 22:09 ` Øystein Walle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.