Hi,

On Mon, 21 Jan 2008, Paolo Bonzini wrote:

quoted

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.

quoted

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".

quoted

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.

quoted

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.

quoted

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