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 - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
