Re: [PATCH 0/2] git-am: add --message-id/--no-message-id options

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

On Wed, Nov 26, 2014 at 10:07 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>
>
> On 25/11/2014 22:21, Christian Couder wrote:
>> On Tue, Nov 25, 2014 at 6:01 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>
>>> As far as I understand, all the git-am hooks are called on the commit
>>> rather than the incoming email: all headers are lost by the time
>>> git-mailinfo exits, including the Message-Id.  And you cannot call any
>>> hook before git-mailinfo because git-mailinfo is where the
>>> Content-Transfer-Encoding is processed.
>>>
>>> How would you integrate git-interpret-trailers in git-mailinfo?
>>
>> I don't know exactly, but people may want to add trailers when they
>> run git-am, see:
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/251412/
>>
>> and we decided that it was better to let something like git
>> interpret-trailers decide how they should be handled.
>>
>> Maybe if git-interpret-trailers could be called from git-mailinfo with
>> some arguments coming from git-am, it could be configured with
>> something like:
>>
>> git config trailer.Message-Id.command 'perl -ne '\''print $1 if
>> m/^Message-Id: (.*)$/'\'' $ARG'
>>
>> So "git am --trailer 'Message-Id: msg-file' msg-file" would call "git
>> mailinfo ..." that would call "git interpret-trailers --trailer
>> 'Message-Id: msg-file'" that would call "perl -ne 'print $1 if
>> m/^Message-Id: (.*)$/' msg-file" and the output of this command, let's
>> call it $id, would be put into a "Message-Id: $id" trailer in the
>> commit message.
>
> I think overloading trailer.Message-Id.command is not a good idea,
> because it would prevent using "git interpret-trailers" to add a message
> id manually ("git interpret-trailers --trailer message-id='<foo@bar>'").

Well, it is possible to configure a trailer.Message-Id.command that
can detect if it is passed an existing file or not.
If it is passed an existing file, it could lookup the message id in
the file and print it, otherwise it would just print what it is
passed.

> Another possibility could be to add a third output file to git-mailinfo,
> including all the headers.  Then a hook could be called with the headers
> and commit message.

Yeah, but this hook could not do everything, because some people might
want to add trailers from the command line anyway.
So git interpret-trailers could be called once with the command line
arguments and once inside the hook.

If the user wants to have some processing done by some commands for
different trailers, it makes sense to have all the processing done by
commands specified in the trailer.<token>.command config variables,
instead of having some of it done by such config variables and other
done in some hooks.

> The question is: what would it be used for?  There aren't that many mail
> headers, and most of them (From, Subject, Date) are recorded in the
> commit anyway.  One idea could be to record who was a recipient of the
> original message, even if no Cc line was added explicitly.  In most
> projects, Cc is often added randomly, but I guess that's a valid
> usecase.  I can certainly code the above hook instead of this approach
> if Junio thinks it's better.

Yes, recording Cc'ed people in Cc: trailers is a very valid use case.
I don't think the trailer.<token>.command mechanism supports that well
if people want only one person per Cc: trailer.
That's something that could be improved in git-interpret-trailers.

> In the meanwhile, I have thought of a couple additions to "git
> interpret-trailers" and I can submit patches for them.

You are welcome to suggest and even more to submit patches for additions to it.
If you want, you can also have a look at some of these threads for
some things that have been suggested already:

http://thread.gmane.org/gmane.comp.version-control.git/259614/
thread.gmane.org/gmane.comp.version-control.git/259275/

Thanks,
Christian.