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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tom Russello <tom.russello@xxxxxxxxxxxxxxxx> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]