Re: [PATCH] commit: Add -f, --fixes <commit> option to add Fixes: line

> On Sun, Oct 27, 2013 at 8:04 PM, Christian Couder <christian.couder@gmail.com> wrote:
>>
>> If "git commit" processes these arguments and puts the result in the
>> commit message file that is passed to the
>> prepare-commit-msg hook, then this hook can still get them from the
>> file and process them however it wants.
>>
>> And in most cases the processing could be the same as what is done by
>> the commit-msg hook when the user changes the "Fixes: xxx" and
>> "Stuffed-by: yyy" lines in the editor.
>>
>> So it would probably be easier for people customizing the
>> prepare-commit-msg and commit-msg if "git commit" processes the
>> arguments instead of just passing them to the prepare-commit-msg hook.
>>
>> And it will be better for people who don't set up any *commit-msg hook.
>> Even if there is no commit template, "-f Acked-by:Peff" and "-f
>> Fixes:security-bug" could still work.
>> I suspect most users don't setup any hook or commit template.
>
> Hmm. I'm not sure what you argue about which part of the system should
> perform which function. Let's examine the above options in more
> detail. Roughly, the flow of events look like this
>
>   git commit -f ack:Peff -f fix:security-bug
>     |
>     v
>   builtin/commit.c (i.e. inside "git commit")
>     |
>     v
>   prepare-commit-msg hook
>     |
>     v
>   commit message template:
>     Fixes: security-bug
>     Acked-by: Peff

>     |
>     v
>   user edits commit message (may or may not change Fixes/Acked-by lines)
>     |
>     v
>   commit-msg hook
>     |
>     v
>   commit message:
>     Fixes: 1234beef56 (Commit message summmary)
>     Acked-by: Jeff King <peff@peff.net>
>
> (The above is even a bit simplified, but I believe it's sufficient for
> the current discussion.) So, there are several expansions happening
> between the initial "git commit" and the final commit message. They
> are:
>
>  1. "fix" -> "Fixes: "
>  2. "security-bug" -> "1234beef56 (Commit message summmary)"
>  3. "ack" -> "Acked-by: "
>  4. "Peff" -> "Jeff King <peff@peff.net>"
>
> First, I think we both agree that expansions #2 and #4 MUST be done by
> the commit-msg hook. The reason for this is two-fold: (a) the
> expansion must be done (at least) after the user has edited the commit
> message (since the values entered by the user might require the same
> expansion), and (b) how (and whether) to perform the expansion is a
> project-specific policy question, and not something that Git can
> dictate.

> Obviously, common functionality can be made available in the
> default hook shipped by Git, but it's up to each project to enable
> and/or customize this.
>
> Second, there is #1 and #3, the expansion of "ack" -> "Acked-by:" and
> "fix" -> "Fixes:". Is this expansion performed by the
> prepare-commit-msg hook, or directly inside builtin/commit.c?
>
> If you are arguing for the latter (and I'm not sure that you are), we
> would need to add a dictionary to "git commit" that maps shorthand
> field names ("ack") to the RFC822 -style equivalent ("Acked-by: ").

> I would instead argue for the former, i.e. simply forwarding "ack" and
> "fix" as-is to the prepare-commit-msg hook, and let it deal with the
> appropriate expansion. The main reason for this is that if a project
> wants to add another shorthand expansion (e.g. "bug" ->
> "Related-Bugzilla-Id: "), they can do so without hacking
> builtin/commit.c.

>> Supporting project specific conventions/rules would still be possible
>> by processing lines in the commit message file without changing "git
>> commit".
>>
>> If "git commit" is already able to do some processing, it only adds
>> power to what can be done by people writing hooks.
>>
>> We could even have git plumbing commands used by git commit to process
>> the -f (or whatever option) arguments and they could be reused by the
>> *commit-msg hooks if they find them useful.
>
> Can you walk through an example of such reusable functionality?