Re: [RFC-PATCH v2 1/2] send-email: quote-email populates the fields

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Tom Russello <tom.russello@grenoble-inp.org> writes:

> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -106,6 +106,11 @@ illustration below where `[PATCH v2 0/3]` is in reply to `[PATCH 0/2]`:
>  Only necessary if --compose is also set.  If --compose
>  is not set, this will be prompted for.
>  
> +--quote-email=<email_file>::
> +	Reply to the given email and automatically populate the "To:", "Cc:" and
> +	"In-Reply-To:" fields.

I think this is a bit too technical for a user documentation. To: and
Cc: is OK, but people need not know about "In-Reply-To:" to understand
this. See what the doc of --in-reply-to says. If you want to be
technical, you'd need to mention the References: field too.

Talking about Reference: field, something your patch could do is to add
all references in <email_file> to the references of the new email (see
what a mailer is doing when replying). This way, the recipient can still
get threading if the last message being replied-to is missing.

> +"Re: [<email_file>'s subject]".

Perhaps `Re: ...` instead of double-quotes.

> +if ($quote_email) {
> +	my $error = validate_patch($quote_email);
> +	$error and die "fatal: $quote_email: $error\nwarning: no patches were sent\n";

I know it's done this way elsewhere, but I don't like this "$error and
die", I'd rather see a proper if here.

> +		if (defined $input_format && $input_format eq 'mbox') {

To me, the input format refers to patch files, not the <email_file>.

I'm not sure anyone still use the "lots of email" format, and you are
not testing it. So, this is claiming that we have a feature without
being sure we have it nor that anyone's ever going to use it.

I'd just drop this "if" and the "else" branch, and just assume the email
file is a normal email file.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/