Re: [PATCH v6 1/2] send-email: refactor header generation functions

[Luben Tuikov <luben.tuikov@amd.com>]

On 2023-01-16 20:39, Strawbridge, Michael wrote:
> Split process_file and send_message into easier to use functions.
> Making SMTP header information more widely available.

"more widely" --> "widely"

> 
> Cc: Luben Tuikov <luben.tuikov@amd.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com>
> ---
>  git-send-email.perl | 49 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..810dd1f1ce 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1495,16 +1495,7 @@ sub file_name_is_absolute {
>  	return File::Spec::Functions::file_name_is_absolute($path);
>  }
>  
> -# Prepares the email, then asks the user what to do.
> -#
> -# If the user chooses to send the email, it's sent and 1 is returned.
> -# If the user chooses not to send the email, 0 is returned.
> -# If the user decides they want to make further edits, -1 is returned and the
> -# caller is expected to call send_message again after the edits are performed.
> -#
> -# If an error occurs sending the email, this just dies.
> -
> -sub send_message {
> +sub gen_header {
>  	my @recipients = unique_email_list(@to);
>  	@cc = (grep { my $cc = extract_valid_address_or_die($_);
>  		      not grep { $cc eq $_ || $_ =~ /<\Q${cc}\E>$/ } @recipients
> @@ -1546,6 +1537,22 @@ sub send_message {
>  	if (@xh) {
>  		$header .= join("\n", @xh) . "\n";
>  	}
> +	my $recipients_ref = \@recipients;
> +	return ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header);
> +}
> +
> +# Prepares the email, then asks the user what to do.
> +#
> +# If the user chooses to send the email, it's sent and 1 is returned.
> +# If the user chooses not to send the email, 0 is returned.
> +# If the user decides they want to make further edits, -1 is returned and the
> +# caller is expected to call send_message again after the edits are performed.
> +#
> +# If an error occurs sending the email, this just dies.
> +
> +sub send_message {
> +	my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
> +	my @recipients = @$recipients_ref;
>  
>  	my @sendmail_parameters = ('-i', @recipients);
>  	my $raw_from = $sender;
> @@ -1735,11 +1742,8 @@ sub send_message {
>  $references = $initial_in_reply_to || '';
>  $message_num = 0;
>  
> -# Prepares the email, prompts the user, sends it out
> -# Returns 0 if an edit was done and the function should be called again, or 1
> -# otherwise.
> -sub process_file {
> -	my ($t) = @_;
> +sub pre_process_file {
> +	my ($t, $quiet) = @_;
>  
>  	open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
>  
> @@ -1893,9 +1897,9 @@ sub process_file {
>  	}
>  	close $fh;
>  
> -	push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t)
> +	push @to, recipients_cmd("to-cmd", "to", $to_cmd, $t, $quiet)
>  		if defined $to_cmd;
> -	push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t)
> +	push @cc, recipients_cmd("cc-cmd", "cc", $cc_cmd, $t, $quiet)
>  		if defined $cc_cmd && !$suppress_cc{'cccmd'};
>  
>  	if ($broken_encoding{$t} && !$has_content_type) {
> @@ -1954,6 +1958,15 @@ sub process_file {
>  			@initial_to = @to;
>  		}
>  	}
> +}
> +
> +# Prepares the email, prompts the user, sends it out

Perhaps add an "and" as "... user, and sends it out."

> +# Returns 0 if an edit was done and the function should be called again, or 1
> +# otherwise.

"otherwise" is usually used on error. Perhaps we want to say here
"or 1 on the email being successfully sent out."?
-- 
Regards,
Luben