Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 10, 2024, 16:28 UTC
- Message-ID
- <xmqqh6g9w5bf.fsf@gitster.g>
- In-Reply-To
- <c78b043b5a6cf0de712d36e6e000804bd6e1316d.1712732383.git.dsimic@manjaro.org>
Dragan Simic <dsimic@manjaro.org> writes:
Show 7 quoted lines
> 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.
Show 5 quoted lines
> ... > 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?
Show 8 quoted lines
> 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.
Show 9 quoted lines
> 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.
Show 57 quoted lines
> 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.