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

[Christian Couder <christian.couder@gmail.com>]

On Wed, Oct 30, 2013 at 8:07 PM, Johan Herland <johan@herland.net> wrote:
> On Tue, Oct 29, 2013 at 7:23 AM, Christian Couder
> <christian.couder@gmail.com> wrote:
>>
>> 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>".
>
> I don't like putting that much Magic into core Git... Especially not
> into builtin/commit.c. However, if we - as you suggest further below -
> put it into a separate helper, and we make that helper available (and
> usable) from elsewhere (most importantly from hooks where
> people/projects can add their own more specific functionality), then I
> don't have a problem with it.

Ok, great! I started working on "git interpret-trailers" and I will
post an RFC patch soon.
It will support both configuration as Junio suggested and reading a
commit template file as you suggested.

>> 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.
>
> This approach looks OK to me, as long as we make sure that this
> functionality is (a) optional, (b) flexible/reusable from hooks, and
> (c) not bound tightly to core Git (and AFAICS, your proposal is just
> that). As I said above, this stuff certainly does not belong in
> builtin/commit.c...

Ok, I think it will be very easy to do all with "git interpret-trailers".

Best regards,
Christian.