Re: [PATCH-v5 4/4] git-commit: add a prepare-commit-msg hook

[Junio C Hamano <gitster@pobox.com>]

Paolo Bonzini <bonzini@gnu.org> writes:

> If the exit status is non-zero, git-commit will abort.  The hook is
> not suppressed by the --no-verify option, so it should not be used
> as a replacement for the pre-commit hook.

I see this matches _exactly_ what I wanted to say.  I however am
suspecting that it may not match what you think the hook should
do, though.

For example,

> The hook is allowed to edit the message file in place.  It can also be
> used for the same purpose as the pre-commit message, if the verification
> should not be skipped for automatic commits (e.g. during interactive
> rebasing).

I, who is claiming that the purpose of this hook is about
preparing the message and not about validating and interfering
with the commit, would never say that.  It's not just "allowed";
editing is the _sole_ reason for its existence.

I'd make it even stronger and clearer:

    The purpose of the hook is to edit the message file in place
    before it gets presented to the user for further editing.
    It is not suppressed by the --no-verify option, because it
    is not about validating what is to be committed.  A non-zero
    exit from the hook means the hook failed, and aborts the
    commit (in other words, non-zero exit from the hook is an
    abnormal condition, perhaps a bug in the hook itself, and
    should never be about the hook not liking the commit being
    created).

But that is apparently quite different from what you wrote.  So
I am sensing some miscommunication here.  I suspect that you are
not convinced by my claim that the hook's sole purpose should be
preparing the commit message to be edited, and that it should
never be used for validation.  Yet an early part of your commit
log message pretends that you are agreeing with me.  The rest of
the patch however still seems to think differently.

And if you think the hook should actively interfere with the
commit, please argue for that case first, without sending an
incoherent patch.  As I often say, unlike Linus, I am not always
right.

> +If the exit status is non-zero, `git-commit` will abort.
> +The hook is not suppressed by the `\--no-verify` option.
> +
> +The hook is allowed to edit the message file in place.  It can also be
> +used for the same purpose as the pre-commit message, if the verification
> +should not be skipped for automatic commits (e.g. during interactive
> +rebasing).

The same comment applies here.

> +++ b/templates/hooks--prepare-commit-msg
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +#
> +# An example hook script to prepare the commit log message.
> +# Called by git-commit with the name of the file that has the
> +# commit message, followed by the description of the commit
> +# message's source.  The hook should exit with non-zero
> +# status after issuing an appropriate message if it wants to stop the
> +# commit.  The hook is allowed to edit the commit message file.

The same comment applies here as well.

There may be cases where an unmaskable validation hook is
desired.  But even if that is the case, I see little reason to
tie it to the act of commit message preparation.