[PATCH] git-commit: add a prepare-commit-msg hook

[PATCH] git-commit: add a prepare-commit-msg hook

[Paolo Bonzini]

The prepare-commit-msg hook is run whenever a "fresh" commit message
(i.e. not one taken from another commit with -c) is shown in the editor.
It can modify the commit message in-place and/or abort the commit.

While the default hook just adds a Signed-Off-By line at the bottom
of the commit messsage, the hook is more intended to build a template
for the commit message following project standards.

Signed-Off-By: Paolo Bonzini <bonzini@gnu.org>
---
 Documentation/git-commit.txt        |   13 ++++++++-----
 Documentation/hooks.txt             |   21 +++++++++++++++++++++
 builtin-commit.c                    |    3 +++
 templates/hooks--commit-msg         |    3 +++
 templates/hooks--prepare-commit-msg |   14 ++++++++++++++
 5 files changed, 49 insertions(+), 5 deletions(-)
 create mode 100644 templates/hooks--prepare-commit-msg

	The real motivation for this is building the commit message
	from GNU ChangeLogs.  But I figured that putting it in the
	real commit message rather than a cover letter would have
	implied rejection of the patch...

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index c3725b2..c68191b 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -62,18 +62,21 @@ OPTIONS
 	and the authorship information (including the timestamp)
 	when creating the commit.  With '-C', the editor is not
 	invoked; with '-c' the user can further edit the commit
-	message.
+	message.  In either case, the prepare-commit-msg hook is
+	bypassed.

 -F <file>::
 	Take the commit message from the given file.  Use '-' to
-	read the message from the standard input.
+	read the message from the standard input.  This option
+	bypasses the prepare-commit-msg hook.

 --author <author>::
 	Override the author name used in the commit.  Use
 	`A U Thor <author@example.com>` format.

 -m <msg>|--message=<msg>::
-	Use the given <msg> as the commit message.
+	Use the given <msg> as the commit message.  This option
+	bypasses the prepare-commit-msg hook.

 -t <file>|--template=<file>::
 	Use the contents of the given file as the initial version
@@ -280,8 +283,8 @@ order).

 HOOKS
 -----
-This command can run `commit-msg`, `pre-commit`, and
-`post-commit` hooks.  See link:hooks.html[hooks] for more
+This command can run `commit-msg`, `prepare-commit-msg`, `pre-commit`,
+and `post-commit` hooks.  See link:hooks.html[hooks] for more
 information.


diff --git a/Documentation/hooks.txt b/Documentation/hooks.txt
index f110162..2ef5567 100644
--- a/Documentation/hooks.txt
+++ b/Documentation/hooks.txt
@@ -61,6 +61,27 @@ The default 'pre-commit' hook, when enabled, catches introduction
 of lines with trailing whitespaces and aborts the commit when
 such a line is found.

+prepare-commit-msg
+------------------
+
+This hook is invoked by `git-commit` right before starting the editor
+with an empty log message.  It is not executed if the commit message is
+specified otherwise, such as with the `\-m`, `\-F`, `\-c`, `\-C` options.
+It takes a single parameter, the name of the file that holds git's own
+template commit log message.  Exiting with non-zero status causes
+`git-commit` to abort.
+
+The hook is allowed to edit the message file in place, and
+can be used to augment the default commit message with some
+project standard information.  It can also be used for the same
+purpose as the pre-commit message, if the verification has
+to be skipped for automatic commits (e.g. during rebasing).
+
+The default 'prepare-commit-msg' hook adds a Signed-Off-By line
+(doing it with a hook is not necessarily a good idea, but doing
+it in 'commit-msg' is worse because you are not reminded in
+the editor).
+
 commit-msg
 ----------

diff --git a/builtin-commit.c b/builtin-commit.c
index 0227936..1de0d02 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -869,6 +869,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		char index[PATH_MAX];
 		const char *env[2] = { index, NULL };
 		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+		if (!edit_message) {
+			run_hook(index_file, "prepare-commit-msg", git_path(commit_editmsg));
+		}
 		launch_editor(git_path(commit_editmsg), NULL, env);
 	}
 	if (!no_verify &&
diff --git a/templates/hooks--commit-msg b/templates/hooks--commit-msg
index c5cdb9d..4ef86eb 100644
--- a/templates/hooks--commit-msg
+++ b/templates/hooks--commit-msg
@@ -9,6 +9,9 @@
 # To enable this hook, make this file executable.

 # Uncomment the below to add a Signed-off-by line to the message.
+# Doing this in a hook is a bad idea in general, but the prepare-commit-msg
+# hook is more suited to it.
+#
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"

diff --git a/templates/hooks--prepare-commit-msg b/templates/hooks--prepare-commit-msg
new file mode 100644
index 0000000..176283b
--- /dev/null
+++ b/templates/hooks--prepare-commit-msg
@@ -0,0 +1,14 @@
+#!/bin/sh
+#
+# An example hook script to prepare the commit log message.
+# Called by git-commit with one argument, the name of the file
+# that has the commit message.  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.
+#
+# To enable this hook, make this file executable.
+
+# This example adds a Signed-off-by line to the message, that can
+# still be edited.
+SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
+grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
-- 
1.5.3.4.910.gc5122-dirty

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

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Paolo Bonzini wrote:

> The prepare-commit-msg hook is run whenever a "fresh" commit message 
> (i.e. not one taken from another commit with -c) is shown in the editor. 
> It can modify the commit message in-place and/or abort the commit.
> 
> While the default hook just adds a Signed-Off-By line at the bottom of 
> the commit messsage, the hook is more intended to build a template for 
> the commit message following project standards.

Would it not be much better for that hook to verify that the template has 
not been added?

Or would not be yet even better to use the commit.template config 
variable, which was intended for that purpose?

Ciao,
Dscho

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

[Paolo Bonzini]

>> The prepare-commit-msg hook is run whenever a "fresh" commit message 
>> (i.e. not one taken from another commit with -c) is shown in the editor. 
>> It can modify the commit message in-place and/or abort the commit.
>>
>> While the default hook just adds a Signed-Off-By line at the bottom of 
>> the commit messsage, the hook is more intended to build a template for 
>> the commit message following project standards.
> 
> Would it not be much better for that hook to verify that the template has 
> not been added?

I fail to parse this.

> Or would not be yet even better to use the commit.template config 
> variable, which was intended for that purpose?

The template might not necessarily be fixed, for example it could be 
preloaded with the list of modified files.  See the cover letter for an 
example.

Paolo

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

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Paolo Bonzini wrote:

> 
> > > The prepare-commit-msg hook is run whenever a "fresh" commit message 
> > > (i.e. not one taken from another commit with -c) is shown in the 
> > > editor. It can modify the commit message in-place and/or abort the 
> > > commit.
> > > 
> > > While the default hook just adds a Signed-Off-By line at the bottom 
> > > of the commit messsage, the hook is more intended to build a 
> > > template for the commit message following project standards.
> > 
> > Would it not be much better for that hook to verify that the template 
> > has not been added?
> 
> I fail to parse this.

What I meant is this:

In the message hook, just grep if the template was already added.  If it 
was, just return.  If it was not, add it.

No need for yet another hook.

Ciao,
Dscho

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

[Paolo Bonzini]

> In the message hook, just grep if the template was already added.  If it 
> was, just return.  If it was not, add it.

Ah, so you want me to always type ":q!" to exit and unnecessarily 
complicate the commit-msg hook.  Cunning, but no, thanks.

I'll make an example.  This is my prepare-commit-msg hook:

diff_collect_changelog ()
{
   git diff "$@" -- \
    `git diff "$@" --name-status -r | \
         awk '/ChangeLog/ { print substr ($0, 3) }'` | sed -n \
     -e '/^@@/,/^+/ {' \
     -e '  s/^ //p' \
     -e '  t' \
     -e '}' \
     -e '/^diff/,/^@@/ {' \
     -e '  s/^diff --git a\/\(.*\)\/ChangeLog[^ ]* b\/.*/\1:/p' \
     -e '  tdummy' \
     -e '  :dummy' \
     -e '  d' \
     -e '}' \
     -e 's/^+//p' \
     -e 't'
}

diff_collect_changelog --cached > /tmp/foo$$
cat "$GIT_COMMIT_MSG" >> /tmp/foo$$ && \
   mv /tmp/foo$$ "$GIT_COMMIT_MSG"
rm -f /tmp/foo$$

or something like that.  The alternative I see would be to start the vi 
editing session with "!!make_changelog --cached".  So I thought about 
having an hook that runs the command for me.  Do you have better ideas?

Paolo

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

[Johannes Schindelin]

Hi,

On Fri, 18 Jan 2008, Paolo Bonzini wrote:

> 
> > In the message hook, just grep if the template was already added.  If it
> > was, just return.  If it was not, add it.
> 
> Ah, so you want me to always type ":q!" to exit and unnecessarily 
> complicate the commit-msg hook.  Cunning, but no, thanks.

No, my intention was to avoid complications.  Like introducing yet another 
commit hook.  I like clean, elegant semantics.  I don't like overbloated 
semantics.  That's why I speak up whenever I fear it is entering git.

Hth,
Dscho

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

[Paolo Bonzini]

> No, my intention was to avoid complications.  Like introducing yet another 
> commit hook.  I like clean, elegant semantics.  I don't like overbloated 
> semantics.  That's why I speak up whenever I fear it is entering git.

Do you think my three line patch is too complicated?  Possibly I was too
zealous in documenting the semantics for the new hook and that looked
like unelegant semantics.

Do you have another possibility which allows the same workflow (git
commit shows the output of that script) within the existing framework?

Paolo

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

[Benoit Sigoure]

On Jan 18, 2008, at 7:06 PM, Johannes Schindelin wrote:

> On Fri, 18 Jan 2008, Paolo Bonzini wrote:
>
>>
>>> In the message hook, just grep if the template was already  
>>> added.  If it
>>> was, just return.  If it was not, add it.
>>
>> Ah, so you want me to always type ":q!" to exit and unnecessarily
>> complicate the commit-msg hook.  Cunning, but no, thanks.
>
> No, my intention was to avoid complications.  Like introducing yet  
> another
> commit hook.  I like clean, elegant semantics.  I don't like  
> overbloated
> semantics.  That's why I speak up whenever I fear it is entering git.


FWIW I'd love such a hook and I've been writing wrappers around Git  
commit for over a year now to simulate this.  I know many people with  
whom I work that would also love such a feature.

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory

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

[Alex Riesen]

Paolo Bonzini, Fri, Jan 18, 2008 15:51:25 +0100:
> +		if (!edit_message) {
> +			run_hook(index_file, "prepare-commit-msg", git_path(commit_editmsg));
> +		}
>  		launch_editor(git_path(commit_editmsg), NULL, env);

"preedit-new-commit-msg", perhaps. A "prepare-" suggests it is called
every time, even if no editor is specified, which it is not.

And it really does look like a template...

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

[Paolo Bonzini]

Alex Riesen wrote:
> Paolo Bonzini, Fri, Jan 18, 2008 15:51:25 +0100:
>> +		if (!edit_message) {
>> +			run_hook(index_file, "prepare-commit-msg", git_path(commit_editmsg));
>> +		}
>>  		launch_editor(git_path(commit_editmsg), NULL, env);
> 
> "preedit-new-commit-msg", perhaps. A "prepare-" suggests it is called
> every time, even if no editor is specified, which it is not.
> 
> And it really does look like a template...

It is, but quite a powerful one. :-)  template-commit-msg?

Paolo

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

[Alex Riesen]

Paolo Bonzini, Fri, Jan 18, 2008 20:46:49 +0100:
> Alex Riesen wrote:
>> Paolo Bonzini, Fri, Jan 18, 2008 15:51:25 +0100:
>>> +		if (!edit_message) {
>>> +			run_hook(index_file, "prepare-commit-msg", git_path(commit_editmsg));
>>> +		}
>>>  		launch_editor(git_path(commit_editmsg), NULL, env);
>> "preedit-new-commit-msg", perhaps. A "prepare-" suggests it is called
>> every time, even if no editor is specified, which it is not.
>> And it really does look like a template...
>
> It is, but quite a powerful one. :-)

Except that "template" is already taken. Someone uses it and some may
even got used to "template" having that meaning.

> template-commit-msg?

Not really. It will be run even if the template (the one Git have
already) is not used.

It really looks a bit complicated. If at all, how about running that
"pre-editor" hook with information about where the message comes from?

I.e. if the message comes from a template:

    preedit-commit-msg .git/COMMIT_MSG template

or, for a message coming from commit:

    preedit-commit-msg .git/COMMIT_MSG commit 0123456789abdef

or, for a message coming from a file

    preedit-commit-msg .git/COMMIT_MSG file user/path/to/file

and finally, for a new message:

    preedit-commit-msg .git/COMMIT_MSG

?

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

[Junio C Hamano]

I do not particularly like hooks that act before or after an
operation is initiated locally, act solely on local data.  This
is maybe because I still consider git tools building blocks
suitable for higher level scripting more than other people do.

There are five valid reasons you might want a hook to a git
operation:

 (1) A hook that countermands the normal decision made by the
     underlying command.  Examples of this class are the update
     hook and the pre-commit hook.

 (2) A hook that operates on data generated after the command
     starts to run.  The ability to munge the commit log message
     by the commit-msg hook is an example.

 (3) A hook that operates on the remote end of the connection
     that you may not otherwise have access to other than over
     the git protocol.  An example is the post-update hook.

 (4) A hook that runs under a lock that is acquired by the
     command for mutual exclusion.  Currently there is no
     example, but if we allowed the update hook to modify the
     commit that was pushed through send-pack => receive-pack
     pair, which was discussed on the list a while ago, it would
     be a good example of this.

 (5) A hook that is run differently depending on the outcome of
     the command.  The post-merge hook conditionally run by
     git-pull is an example of this (it is not even run if no
     merge takes place).  Another example is the post-checkout
     hook that gets information that is otherwise harder to get
     (namely, if it was a branch checkout or file checkout --
     you can figure it out by examining the command line but
     that already is part of the processing git-checkout does
     anyway, so no need to force duplicating that code in the
     userland).

You cannot do an equivalent operation from outside the git
command for the above classes of operations.  You need hooks
for them.

On the other hand, if you want to always cause an action before
running a git opeation locally, you do not have to have a hook.
You can just prepare such a message based on GNU ChangeLog and
then run git-commit with -F, both inside your wrapper.

Of course there can be a very valid exception to the above
policy.  If it is common enough so that the policy means
effectively everybody has to reinvent the same wrapper.  But for
this particular case I still do not see that is the case.

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

[Paolo Bonzini]

> On the other hand, if you want to always cause an action before
> running a git opeation locally, you do not have to have a hook.
> You can just prepare such a message based on GNU ChangeLog and
> then run git-commit with -F, both inside your wrapper.

I see two other possibilities:

1) Would you prefer allowing to run a command by setting commit.template 
to something starting with an exclamation mark, i.e. something like 
"!git diff --cached --name-status -r"?

2) The pre-commit could receive a file name on the command line.  If it 
creates that file, it would be used as a template for the commit 
message.  The default implementation of pre-commit could include, 
commented out, a reimplementation of prepare_log_message.  I like this 
the least.

3) Consider that the patch, as I implemented it, does not act solely on 
local data, because the index could be modified by the pre-commit hook. 
  This possibility is contemplated explicitly in builtin-commit.c:

         /*
          * Re-read the index as pre-commit hook could have updated it,
          * and write it out as a tree.
          */

Paolo

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

[Johannes Schindelin]

Hi,

On Sat, 19 Jan 2008, Paolo Bonzini wrote:

> > On the other hand, if you want to always cause an action before 
> > running a git opeation locally, you do not have to have a hook. You 
> > can just prepare such a message based on GNU ChangeLog and then run 
> > git-commit with -F, both inside your wrapper.
> 
> I see two other possibilities:
> 
> 1) [..]
>
> 2) [..]
>
> 3) [..]

Of course, there is a fourth of "two other" possibilities:

Make a script calling git-commit with "-F - -e" and pipe your generated 
template into it.

Use this script whenever you want to create a new commit.

Hth,
Dscho

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

[Benoit Sigoure]

On Jan 19, 2008, at 12:20 PM, Johannes Schindelin wrote:

> On Sat, 19 Jan 2008, Paolo Bonzini wrote:
>
>>> On the other hand, if you want to always cause an action before
>>> running a git opeation locally, you do not have to have a hook. You
>>> can just prepare such a message based on GNU ChangeLog and then run
>>> git-commit with -F, both inside your wrapper.
>>
>> I see two other possibilities:
>>
>> 1) [..]
>>
>> 2) [..]
>>
>> 3) [..]
>
> Of course, there is a fourth of "two other" possibilities:
>
> Make a script calling git-commit with "-F - -e" and pipe your  
> generated
> template into it.
>
> Use this script whenever you want to create a new commit.


That's what I've done for over a year (http://repo.or.cz/w/svn- 
wrapper.git -- it started out as a wrapper around SVN but also works  
fine with Git) and many people also made their own script to achieve  
something similar (e.g, vc-dwim http://lists.gnu.org/archive/html/bug- 
gnulib/2007-10/msg00135.html).

Having such a wrapper in Git would just make our life easier.

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory

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

[Paolo Bonzini]

> Of course, there is a fourth of "two other" possibilities:

(The third was just the previously posted patch, so two "other than the 
posted one") :-)

> Make a script calling git-commit with "-F - -e" and pipe your generated 
> template into it.

You considered that this script should parse -a, -i, -o, whatever, 
right? ;-)

The point is that a hook can use the index as prepared by git-commit.

Paolo

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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Of course, there is a fourth of "two other" possibilities:
>
> Make a script calling git-commit with "-F - -e" and pipe your generated 
> template into it.
>
> Use this script whenever you want to create a new commit.

I think that the approach has one huge advantage.  Commands
other than "git commit" itself ("git merge", "git rebase", "git
am", etc.) do call "git commit" to record the changes they made.
I suspect these command would not want this template behaviour,
and not adding this custom commit message "feature" to "git
commit" would avoid the risk of breaking them.

At the same time, this exact issue could be a drawback.  Some of
them _might_ want it.  But in that case, the the custom template
"hook" needs to be told _why_ it is being called, so that it can
adjust its behaviour.

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

[Paolo Bonzini]

> I think that the approach has one huge advantage.  Commands
> other than "git commit" itself ("git merge", "git rebase", "git
> am", etc.) do call "git commit" to record the changes they made.
> I suspect these command would not want this template behaviour,
> and not adding this custom commit message "feature" to "git
> commit" would avoid the risk of breaking them.

My patch does not do that.  Of all these commands, only git-rebase uses 
porcelain git-commit rather than plumbing git-commit-tree, and it uses 
the -C or -F options (which disable the hook in my patch).

> At the same time, this exact issue could be a drawback.  Some of
> them _might_ want it.  But in that case, the the custom template
> "hook" needs to be told _why_ it is being called, so that it can
> adjust its behaviour.

I would like to understand why both of you have not considered the point 
  that the script would need the updated index from git-commit.  Because 
short of reusing half of the old Bourne-shell git-commit, I don't see 
how this would be possible, but maybe there's something obvious.  Or 
maybe we need new plumbing like git-build-index -- then I'd still need 
to complicate my script to build a template commit message, but I 
wouldn't need to muddle myself (not too much at least) in handling the 
command line.

Paolo

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

[Johannes Schindelin]

Hi,

On Mon, 21 Jan 2008, Paolo Bonzini wrote:

> I would like to understand why both of you have not considered the point 
> that the script would need the updated index from git-commit.

I have.  But I want to avoid a regression at any cost.  And your patch 
just looks to me like it could do that.

But it _has_ been already suggested that you could provide arguments for 
the existing msg-hook, which would not break anything, since the hook does 
not get any argument yet, and therefore existing hooks would be 
unaffected.

Also, the change would be non-intrusive, easy-to-review, and it would not 
take so much time away from the bug-fixing that we want to do right now in 
the rc-cycle.

Okay?
Dscho

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

[Paolo Bonzini]

> I have.  But I want to avoid a regression at any cost.  And your patch 
> just looks to me like it could do that.

What kind of regression?

> But it _has_ been already suggested that you could provide arguments for 
> the existing msg-hook, which would not break anything

Sure it won't break anything, but it won't work either!  The existing 
message hook runs after the editing session -- I want the hook to 
introduce text that is merely a suggestion that the user can delete, or 
a template that the user needs to customize further.

> since the hook does 
> not get any argument yet, and therefore existing hooks would be 
> unaffected.

How does adding a new hook affect existing hooks?

> Also, the change would be non-intrusive, easy-to-review

Please.  That's ludicrous.

My patch is 3 lines of inserted code and 0 modified lines, checking one 
variable that is set once in builtin-commit.c (edit_message).  The 
documentation says that it runs whenever the editor runs except for -c, 
and launch_editor runs after the 3 lines of code I inserted.

Seeing how biased you are, I don't really know why I bothered answering you.

> take so much time away from the bug-fixing that we want to do right now in 

That's the first sensible argument that I hear.

Paolo

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

[Johannes Schindelin]

Hi,

On Mon, 21 Jan 2008, Paolo Bonzini wrote:

> > I have.  But I want to avoid a regression at any cost.  And your patch 
> > just looks to me like it could do that.
> 
> What kind of regression?

See below.

> > But it _has_ been already suggested that you could provide arguments 
> > for the existing msg-hook, which would not break anything
> 
> Sure it won't break anything, but it won't work either!  The existing 
> message hook runs after the editing session -- I want the hook to 
> introduce text that is merely a suggestion that the user can delete, or 
> a template that the user needs to customize further.

OMG you're right.  But why didn't you say so in the commit message?  
Something like "This hook complements the commit-msg hook, in that it runs 
_before_ the editor is launched".

> > since the hook does not get any argument yet, and therefore existing 
> > hooks would be unaffected.
> 
> How does adding a new hook affect existing hooks?

My impression -- even after reading your commit message -- was that it 
does almost the same as the commit-msg hook, only that it runs _in 
addition_ to it when doing a non-amend commit.

FWIW this is the first reply by you that uncovers this error of mine.

> > Also, the change would be non-intrusive, easy-to-review
> 
> Please.  That's ludicrous.
> 
> My patch is 3 lines of inserted code and 0 modified lines, checking one 
> variable that is set once in builtin-commit.c (edit_message).

Actually, after reading the commit message I was in 
"this-is-not-necessary" mode, and therefore the diffstat looked too large 
for me.

That is why I thought that a regression was looming somewhere.

And in reality, your patch should be 2 lines of code.

> The documentation says that it runs whenever the editor runs except for 
> -c, and launch_editor runs after the 3 lines of code I inserted.

Actually, reading your patch again I think it also triggers for "-c", as 
well as for "[-C|-F|-m] ... -e".

And it does not necessarily start with "an empty log message"; think of 
"--signoff".

Besides, I think that the name should be more to the point, something like 
"pre-edit-commit-msg".

Also, I should think that the hook should get some information about the 
circumstances (possibly as command line arguments), given that it is 
called in more cases than you said.

So I think the patch is not ready yet, although I finally got the _point_ 
of having yet-another hook.

> Seeing how biased you are, I don't really know why I bothered answering 
> you.

So I am biased.  And you have to convince me.  It's not that hard to 
convince me.

> > take so much time away from the bug-fixing that we want to do right 
> > now in
> 
> That's the first sensible argument that I hear.

Heh.  Then I'm happy that I put it into my mail, too!

Ciao,
Dscho

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

[Paolo Bonzini]

>> Sure it won't break anything, but it won't work either!  The existing 
>> message hook runs after the editing session -- I want the hook to 
>> introduce text that is merely a suggestion that the user can delete, or 
>> a template that the user needs to customize further.
> 
> OMG you're right.  But why didn't you say so in the commit message?  
> Something like "This hook complements the commit-msg hook, in that it runs 
> _before_ the editor is launched".

I can't believe this. ;-)  At least I'm laughing for something nice now!

You're right, I should have said instead of this:

> The prepare-commit-msg hook is run whenever a "fresh" commit message 
> (i.e. not one taken from another commit with -c) is shown in the editor. 

... just before a "fresh" commit message is shown in the editor.
          ^^^^^^

>>> Also, the change would be non-intrusive, easy-to-review
>> Please.  That's ludicrous.
>>
>> My patch is 3 lines of inserted code and 0 modified lines, checking one 
>> variable that is set once in builtin-commit.c (edit_message).
> 
> Actually, after reading the commit message I was in 
> "this-is-not-necessary" mode, and therefore the diffstat looked too large 
> for me.

Yes, but still the diffstat was all .txt files... ;-)

> Actually, reading your patch again I think it also triggers for "-c", as 
> well as for "[-C|-F|-m] ... -e".

Not for "-c", that's the point of the "edit_message" check.  You're 
right about "-e" though.

Points taken, and patch will be resubmitted after 1.5.4.

Paolo

[PATCH] git-commit: add a prepare-commit-msg hook

[Paolo Bonzini]

This series of patches adds a prepare-commit-msg hook.
The prepare-commit-msg hook is run whenever a "fresh" commit message
is prepared, just before it is shown in the editor (if it is).
It can modify the commit message in-place and/or abort the commit.
I implemented Alex Riesen's suggestion to tell the hook where the
message came from, and now run the hook even if the editor is not
run.

Patches 1 and 2 are small changes.  Patch 1 changes run_hook to
accept a variable-length NULL-terminated list of arguments.  Patch 2
forces GIT_EDITOR to : if editor will not be launched; this is the
simplest way I found to tell the prepare-commit-msg hook whether
the editor will be launched.

Patch 3 is bigger; it refactors parts of git-commit to do all the
log message processing at the same time.  Currently the message
is prepared soon, but only edited after the first part of the commit
object is prepared.  This simplifies a little the code for part 4.

Part 4 actually adds the hook, including documentation and testcases.
The hook takes two parameters.  The first is the source of the commit
message (detailed more in the commit message and in the docs), which
is either an English word or a commit SHA1.  The second
parameter if the name of the file that the commit log message.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>

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

[Junio C Hamano]

Paolo Bonzini <bonzini@gnu.org> writes:

>> Actually, reading your patch again I think it also triggers for
>> "-c", as well as for "[-C|-F|-m] ... -e".
>
> Not for "-c", that's the point of the "edit_message" check.  You're
> right about "-e" though.
>
> Points taken, and patch will be resubmitted after 1.5.4.

Thanks.  I think the approach is sane, and it naturally falls
into the second category of "why we might want to a hook for"
list I sent earlier.

I do not think adding "-X makes that hook ignored" for
description of every option is warranted, though.  Users should
not have to be reminded that there is a hook he may never use
and it does not trigger if he specifies his own message using -X
or -Y or -Z option.

The usage of the hook is optional, and the primary description
of what it does and why a user might want to use it should be in
its own description (perhaps in hooks.txt and a section that
lists hooks in git-commit manpage).  As long as that description
makes it clear that the hook is a way to specify a dynamic
template in a situation that requries a fresh message, the users
who are interested in using it would perfectly well understand
that options that make git-commit not to take a fresh message
would not invoke the hook.