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

> On Mon, Oct 28, 2013 at 3:46 AM, Johan Herland <johan@herland.net> wrote:
>> 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
>
> Here it could already be:
>
>      Fixes: 1234beef56 (Commit message summmary)
>      Acked-by: Jeff King <peff@peff.net>
>
> Because builtin/commit.c hook could already have expanded everything.
>
>>     |
>>     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.
>
> I don't agree. Git doesn't need to dictate anything to be able to do
> these expansions.
> Git only needs some hints to do these expansions properly and it could
> just look at the commit template, or the config, to get those hints.
>
> For example, if there is a "Acked-by:" line in the commit template,
> then Git might decide that "ack" means "Acked-by", and then that "-by"
> means that "Peff" should be related to an author, and then that it is
> probably "Jeff King <peff@peff.net>".

>>> 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?
>
> Ok, let's call the new plumbing command "git interpret-trailers".
> And let's suppose that "git commit" is passed "-f ack:Peff -f
> fix:security-bug" (or "--trailer ack=Peff --trailer
> fix=security-bug").
>
> "git commit" would then call something like:
>
> git interpret-trailers --file commit_message_template.txt 'ack:Peff'
> 'fix:security-bug'
>
> And this command would output:
>
> ------------------
> <<<upper part of commit_message_template.txt>>>
>
> Fixes: 1234beef56 (Commit message summmary)
> Reported-by:
> Suggested-by:
> Improved-by:
> Acked-by: Jeff King <peff@peff.net>
> Reviewed-by:
> Tested-by:
> Signed-off-by: Myself <myself@example.com>
> ------------------
>
> Because it would have looked at the commit template it is passed and
> filled in the blanks it could fill using the arguments it is also
> passed.
>
> "git commit" would then put the above lines in the file that it passes
> to the prepare-commit-msg hook.
>
> Then the prepare-commit-msg could just do nothing.
>
> After the user has edited the commit message, the commit-msg hook
> could just call:
>
> git interpret-trailers --trim-empty --file commit_message.txt
>
> so that what the user changed is interpreted again.
>
> For example if the user changed the "Reviewed-by:" line to
> "Reviewed-by: Johan", then the output would be:
>
> ------------------
> <<<upper part of commit_message.txt>>>
>
> Fixes: 1234beef56 (Commit message summmary)
> Acked-by: Jeff King <peff@peff.net>
> Reviewed-by: Johan Herland <johan@herland.net>
> Signed-off-by: Myself <myself@example.com>
> ------------------
>
> And that would be the final commit message in most cases.