Dragan Simic <dsimic@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ...
>
> ...
> 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@xxxxxxxxxxx>
> ...
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@xxxxxxxxxxx>
> ...
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.
