Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch

[Junio C Hamano <gitster@pobox.com>]

Dragan Simic <dsimic@manjaro.org> writes:

> When sending one or multiple patches at once, the displayed result statuses
> for each patch and the "Send this email [y/n/a/...]?" confirmation prompts
> become bunched together with the messages produced for the subsequent patch,
> or with the produced SMTP trace, respectively.
>
> This makes reading the outputs unnecessarily harder, as visible in a couple
> of excerpts from a sample output below:

It is unclear where the boundaries between the messages in the
example are, though.

>     ...
>     MIME-Version: 1.0
>     Content-Transfer-Encoding: 8bit
>
>     Result: 250

Is this where one message ends, and the next line "OK. Log says:" is
the beginning of the next message?

>     OK. Log says:
>     Server: smtp.example.com
>     MAIL FROM:<test@example.com>
>     ...
>
>     ...
>     MIME-Version: 1.0
>     Content-Transfer-Encoding: 8bit

Is the above about a single (i.e. the second) message ...

>     Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y

... and the user is asked about that message?

>     OK. Log says:
>     Server: smtp.example.com
>     MAIL FROM:<test@example.com>
>     ...

And is this about a separate (i.e. the third) message?  Without
making these clear, it is hard to agree or disagree with the claim
that the current presentation is hard to read.

>     MIME-Version: 1.0
>     Content-Transfer-Encoding: 8bit
>
>     Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
>
>     OK. Log says:
>     Server: smtp.example.com
>     MAIL FROM:<test@example.com>
>     ...

This is obviously in the realm of subjective preference, but I find
that the prompt line is distinct enough among all other output that
we do not need an extra blank line to locate them.

> diff --git a/git-send-email.perl b/git-send-email.perl
> index f0be4b4560f7..1d6712a44e95 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1361,7 +1361,6 @@ sub smtp_host_string {
>  
>  # Returns 1 if authentication succeeded or was not necessary
>  # (smtp_user was not specified), and 0 otherwise.
> -
>  sub smtp_auth_maybe {
>  	if (!defined $smtp_authuser || $auth || (defined $smtp_auth && $smtp_auth eq "none")) {
>  		return 1;
> @@ -1510,6 +1509,7 @@ sub gen_header {
>  sub send_message {
>  	my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
>  	my @recipients = @$recipients_ref;
> +	my $confirm_shown = 0;
>  
>  	my @sendmail_parameters = ('-i', @recipients);
>  	my $raw_from = $sender;
> @@ -1555,6 +1555,7 @@ sub send_message {
>  		} elsif (/^a/i) {
>  			$confirm = 'never';
>  		}
> +		$confirm_shown = 1;
>  	}
>  
>  	unshift (@sendmail_parameters, @smtp_server_options);
> @@ -1576,7 +1577,6 @@ sub send_message {
>  		print $sm "$header\n$message";
>  		close $sm or die $!;
>  	} else {
> -
>  		if (!defined $smtp_server) {
>  			die __("The required SMTP server is not properly defined.")
>  		}
> @@ -1664,9 +1664,11 @@ sub send_message {
>  		$smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
>  	}
>  	if ($quiet) {
> +		print "\n" if ($confirm_shown);
>  		printf($dry_run ? __("Dry-Sent %s") : __("Sent %s"), $subject);
>  		print "\n";
>  	} else {
> +		print "\n";
>  		print($dry_run ? __("Dry-OK. Log says:") : __("OK. Log says:"));
>  		print "\n";
>  		if (!defined $sendmail_cmd && !file_name_is_absolute($smtp_server)) {
> @@ -1923,7 +1925,7 @@ sub pre_process_file {
>  sub process_file {
>  	my ($t) = @_;
>  
> -        pre_process_file($t, $quiet);
> +	pre_process_file($t, $quiet);
>  
>  	my $message_was_sent = send_message();
>  	if ($message_was_sent == -1) {

I'll let others comment as the "blank around prompt" smells quite
subjective and do not want to be the sole reviewer on it.

Thanks, will queue.