[RFC-PATCH 0/2] send-email: new --quote-mail option

[RFC-PATCH 0/2] send-email: new --quote-mail option

Hello,

With the current send-email command, you can send a series of patches "in reply
to" an email.
This patch adds a new option to `git send-email`, `--quote-mail=<file>`, to
quote an email in the cover letter in your series of patches.

The "To", "Cc" and "Subject" fields will be filled appropriately and the message
given quoted in the cover letter for the series of patches.

In this first patch, the new option `--quote-mail=<file>` needs an
email file and does not manage accents.

There is still work in progress, including:

1. An option `--quote-mail-id=<message-id>` to download the message
from any source, e.g. http://mid.gmane.org/<message-id>/raw.
The server's address could be set in the repo's config file.

2. The proper documentation for `--quote-mail=<file>` and
`--quote-mail-id=<message-id>` as soon as their definitive
behavior is approved by the community.

3. The code to parse the email headers is currently duplicated several
times, we should refactor it to help maintaining the code.

[RFC-PATCH 1/2] send-email: new option to quote an email and reply to

This new option takes an email message file, parses it, fills the "To",
"Subject" and "Cc" fields appropriately and quote the message.
This option involves the `--compose` mode to edit the cover letter quoting the
given email.

Is this really necessary to switch between "> " and ">" prefix?
AFAIK, MUAs prefix unconditionally with "> ".
At least that's how mutt does it...
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCH 0/2] send-email: new --quote-mail option

> Tom Russello <[hidden email]> writes:
>
>> Hello,
>>
>> With the current send-email command, you can send a series of patches "in reply
>> to" an email.
>> This patch adds a new option to `git send-email`, `--quote-mail=<file>`, to
>
> I think the option name should be --quote-email. Even though "mail"
> usually means "email" for French people, there's still non-electronic
> mail for english-speaking ones.

That makes sense, all occurrences of "mail" will be changed into "email"
for v2.

This adds the content of the To: field in the original email to the Cc:
field in the new message, right? If so, this is a weird behavior: when
following up to an email, one usually addresses to the person s/he's
replying, keeping the others Cc-ed, hence the original From: becomes the
To header, and the original To: and Cc: become Cc:.

> + } elsif (/^Cc:\s+(.*)$/i) {

Style: IIRC, there's no consensus on whether "elsif" should be on the
same line as the closing }, but please follow the same convention inside
a single if/elsif/ chain.

> + #Message body

Style: space after # (more below). And while you're there, the comment
could be "Quote the message body" or so, to give a hint to the user
about what's going on.

Splitting a patch series as "code first; tests after" is not a good idea
IMHO. When questionning the behavior of To: Vs Cc: in the previous
patch, I would have appreciated having tests in the same message, to
check that the tested behavior was indeed the one I was reading in the
code.

OTOH, having one patch to introduce "--quote-email populates To: and Cc:
headers", and then another one for "--quote-email quotes the message
body" would make the review much easier.

Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to

> On 05/23/2016 09:55 PM, Eric Wong wrote:
> >Cool! There should probably be some help text to encourage
> >trimming down the quoted text to only relevant portions.
>
> What kind of help text would you want to see?
>
> Maybe something like this:
>
> GIT: Quoted message body below.
> GIT: Feel free to trim down the quoted text
> GIT: to only relevant portions.
>
> As "GIT:" portions are ignored when parsed by `git send-email`.

Yes, given we have instructions for diffstat and table of contents;
I think it'd be useful to discourage quoting irrelevant parts of
the message (especially signatures and like).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to

>Samuel GROOT <[hidden email]> writes:
>
>> What kind of help text would you want to see?
>>
>> Maybe something like this:
>>
>> GIT: Quoted message body below.
>> GIT: Feel free to trim down the quoted text
>> GIT: to only relevant portions.
>>
>> As "GIT:" portions are ignored when parsed by `git send-email`.
>
>That's an option, but in the context of email, I think these
>instructions are not necessary.

In an ideal world that would be true. But in the real world I think
evidence of many messages to this mailing list containing full quotes
suggests it might be helpful. I'd actually argue that the message be
more forceful, making it a suggestion/request to trim rather than simply
telling the user that it's allowed.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to

On 05/23/2016 10:00 PM, Matthieu Moy wrote:
> I don't think this is right: I often reply to an email with a single
> patch, for which it would clearly be overkill to have a cover-letter.

Yes indeed, we are working on inserting the quoted message body in the
patch's "below-triple-dash" section.

> Your --quote-mail does two things:
>
> 1) Populate the To and Cc field
>
> 2) Include the original message body with quotation prefix.
>
> When not using --compose, 1) clearly makes sense already, and there's no
> reason to prevent this use-case. When sending a single patch, 2) also
> makes sense as "below-tripple-dash comment", like
>
> This is the commit message for feature A.
> ---
> John Smith wrote:
> > You should implement feature A.
>
> Indeed, here's a patch.
>
> modified-file.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>
> When sending multiple patches without --compose, 2) may not make sense,
> but I think a sane behavior would be:
>
> * If --compose is given, cite the message there.
>
> * If --compose is not given, don't send a cover-letter but cite the body
> as comment in the first patch.
>
> As a first step, the second point can be changed to "if --compose is not
> given, don't cite the message, just populate the To: and Cc: fields".

It seems a good behavior to me too.

> * If --compose is not given, don't send a cover-letter but cite the body
> as comment in the first patch.

Then should the option imply `--annotate`, to let the user edit the
patch containing the quoted email?

>> + push(@header, $_);
>
> I think the code would be clearer if @header was a list of pairs
> (header-name, header-content). Then you'd need much less regex magic
> when going through it.

The next series of patches may include (if the code is clean enough by
then) a cleaner subroutine to parse the email, probably returning
something like:

>> + elsif (/^From:\s+(.*)$/i) {
>> + push @initial_to, $1;
>> + }
>> + elsif (/^To:\s+(.*)$/i) {
>> + foreach my $addr (parse_address_line($1)) {
>> + if (!($addr eq $initial_sender)) {
>> + push @initial_to, $addr;
>> + }
>> + }
>
> This adds the content of the To: field in the original email to the Cc:
> field in the new message, right? If so, this is a weird behavior: when
> following up to an email, one usually addresses to the person s/he's
> replying, keeping the others Cc-ed, hence the original From: becomes the
> To header, and the original To: and Cc: become Cc:.

We made the option behave like Thunderbird does, but indeed RFC 2822 [1]
sees it the same you do, it will be fixed in next iteration.

Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to

> At 14:49 +0200 24 May 2016, Matthieu Moy <[hidden email]>
> wrote:
>> Samuel GROOT <[hidden email]> writes:
>>
>>> What kind of help text would you want to see?
>>>
>>> Maybe something like this:
>>>
>>> GIT: Quoted message body below.
>>> GIT: Feel free to trim down the quoted text
>>> GIT: to only relevant portions.
>>>
>>> As "GIT:" portions are ignored when parsed by `git send-email`.
>>
>> That's an option, but in the context of email, I think these
>> instructions are not necessary.
>
> In an ideal world that would be true. But in the real world I think
> evidence of many messages to this mailing list containing full quotes
> suggests it might be helpful. I'd actually argue that the message be
> more forceful, making it a suggestion/request to trim rather than simply
> telling the user that it's allowed.

Furthermore, it is a good way to avoid very long messages due to
unnecessary parts quoted.

Therefore, we thought about a request like "Please, trim down irrelevant
sections in the quoted message to keep your email concise"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to

> On 05/23/2016 10:00 PM, Matthieu Moy wrote:
>
>> Your --quote-mail does two things:
>>
>> 1) Populate the To and Cc field
>>
>> 2) Include the original message body with quotation prefix.
>>

[...]

>> * If --compose is not given, don't send a cover-letter but cite the body
>> as comment in the first patch.
>
> Then should the option imply `--annotate`, to let the user edit the
> patch containing the quoted email?

Actually, I'm not sure what the ideal behavior should be. Perhaps it's
better to distinguish 1) and 2) above, and have two options
--reply-to-email=<file> doing 1), and --quote doing 2), implying
--compose and requiring --reply-to-email.

That would be more flexible, but that would require two new options, and
I also like to keep things simple.

In any case, quoting the message without replying to it does not make
sense (especially if you add instructions to trim it: the user would not
even see it). So it its current form, I'd say --quote-email should imply
--annotate.

I tend to agree that sounds like a better way to structure these
features.

I wonder if we can safely repurpose existing --in-reply-to option?
That is, if the value of --in-reply-to can be reliably determined as
a filename that has the message (as opposed to a message-id), we
read the "Message-Id:" from that file to figuire out what message-id
to use, and figure out To/Cc: to use for the purpose of your (1) at
the same time. In the future, you might even teach send-email,
perhaps via a user configurable hook, a way to get to the message
header and text given a message-id, and when it happens, the same
logic can be used when --in-reply-to is given a message-id (i.e. you
go from the id to the message and find the addresses you would
To/Cc: your message).

> In any case, quoting the message without replying to it does not make
> sense (especially if you add instructions to trim it: the user would not
> even see it). So it its current form, I'd say --quote-email should imply
> --annotate.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to

In theory, obviously no as there can be a file with this name _and_ it
can be a valid message-id. In practice, it is clearly unlikely. The only
use-case I can think of where both would be valid is if the user happens
to have saved the message using the message-id as filename. But then,
the ambiguity would not harm, as the message-id contained in the file
would be the same as the filename.

> That is, if the value of --in-reply-to can be reliably determined as
> a filename that has the message (as opposed to a message-id), we
> read the "Message-Id:" from that file to figuire out what message-id
> to use, and figure out To/Cc: to use for the purpose of your (1) at
> the same time.

This should work, but sounds like too much of overloading of
--in-reply-to IMHO: if given a message id, it would only add a reference
to this message-id, but if given a file, it would also modify the To:
and Cc: list.

Not a strong objection, though.

> In the future, you might even teach send-email, perhaps via a user
> configurable hook, a way to get to the message header and text given a
> message-id, and when it happens, the same logic can be used when
> --in-reply-to is given a message-id (i.e. you go from the id to the
> message and find the addresses you would To/Cc: your message).

That is the plan indeed. Fetching from gmane for example should be
rather easy in perl, and would be really convenient!

Re: [RFC-PATCH 1/2] send-email: new option to quote an email and reply to

> This should work, but sounds like too much of overloading of
> --in-reply-to IMHO: if given a message id, it would only add a reference
> to this message-id, but if given a file, it would also modify the To:
> and Cc: list.
>
> Not a strong objection, though.

Well, with your "that is the plan indeed", the option would behave
the same whether given a message ID or a filename, no?

But I do agree that those who have accustomed to the behaviour of
--in-reply-to that does not mess with To/Cc:, such a behaviour
change is not desirable.

If we are adding a new --reply-to-email=<file|id>, it should behave
as a superset of --in-reply-to (i.e. it should set In-Reply-to:
using the message ID of the e-mail we are replying to), though.

>> In the future, you might even teach send-email, perhaps via a user
>> configurable hook, a way to get to the message header and text given a
>> message-id, and when it happens, the same logic can be used when
>> --in-reply-to is given a message-id (i.e. you go from the id to the
>> message and find the addresses you would To/Cc: your message).
>
> That is the plan indeed. Fetching from gmane for example should be
> rather easy in perl, and would be really convenient!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]More majordomo info at http://vger.kernel.org/majordomo-info.html