Re: [PATCH 2/2] send-email: quote-email quotes the message body

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 1, 2017, 06:40 UTC
Message-ID: <xmqqa806tscf.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171030223444.5052-3-nathan.payre@etu.univ-lyon1.fr>

Payre Nathan <second.payre@gmail.com> writes:

Show 20 quoted lines

> From: Tom Russello <tom.russello@grenoble-inp.org>
>
> ---
>  Documentation/git-send-email.txt |  4 +-
>  git-send-email.perl              | 80 ++++++++++++++++++++++++++++++++++++++--
>  t/t9001-send-email.sh            | 19 +++++++++-
>  3 files changed, 97 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 710b5ff32..329af66af 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -107,7 +107,9 @@ Only necessary if --compose is also set.  If --compose
>  is not set, this will be prompted for.
>  
>  --quote-email=<email_file>::
> -	Fill appropriately header fields for the reply to the given email.
> +	Fill appropriately header fields for the reply to the given email and quote
> +	the message body in the cover letter if `--compose` is set or otherwise
> +	after the triple-dash in the first patch given.

Hmmm. I have a strong suspicion that people want an option to trigger the feature from just 1/2 but not 2/2 some of the time. Sure, removing the unwanted lines in the compose editor may be easy, but it feels wasteful use of user's time to include the lines of text from the original only to have them removed.

Also, if you are not offering these two as separate features, there isn't much point splitting them into two patches.

Show 17 quoted lines

> @@ -743,6 +768,9 @@ if ($compose) {
>  	my $tpl_sender = $sender || $repoauthor || $repocommitter || '';
>  	my $tpl_subject = $initial_subject || '';
>  	my $tpl_reply_to = $initial_reply_to || '';
> +	my $tpl_quote = $message_quoted &&
> +		"\nGIT: Please, trim down irrelevant sections in the quoted message\n".
> +		"GIT: to keep your email concise.\n" . $message_quoted || '';
>  
>  	print $c <<EOT1, Git::prefix_lines("GIT: ", __ <<EOT2), <<EOT3;
>  From $tpl_sender # This line is ignored.
> @@ -756,7 +784,7 @@ EOT2
>  From: $tpl_sender
>  Subject: $tpl_subject
>  In-Reply-To: $tpl_reply_to
> -
> +$tpl_quote
>  EOT3

OK, by emitting it into $compose_filename as part of the front matter, you get the "do we have to do mime?" etc. for free, which sort-of makes sense.

Show 28 quoted lines

> @@ -821,9 +849,53 @@ EOT3
>  		$compose = -1;
>  	}
>  } elsif ($annotate) {
> -	do_edit(@files);
> +	if ($quote_email) {
> +		my $quote_email_filename = ($repo ?
> +			tempfile(".gitsendemail.msg.XXXXXX",
> +				DIR => $repo->repo_path()) :
> +			tempfile(".gitsendemail.msg.XXXXXX",
> +				DIR => "."))[1];
> +
> +		# Insertion in a temporary file to keep the original file clean
> +		# in case of cancellation/error.
> +		do_insert_quoted_message($quote_email_filename, $files[0]);
> +
> +		my $tmp = $files[0];
> +		$files[0] = $quote_email_filename;
> +
> +		do_edit(@files);
> +
> +		# Erase the original patch if the edition went well
> +		move($quote_email_filename, $tmp);
> +		$files[0] = $tmp;
> +	} else {
> +		do_edit(@files);
> +	}
>  }