Hello Junio,
Just checking, is there something I can do to get this patch
series moving forward?
On 2024-04-13 08:27, Dragan Simic wrote:
On 2024-04-10 18:28, Junio C Hamano wrote:
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>
...
Huh, I understand your confusion and those are all valid remarks.
However, my intention was to include excerpts that are long enough
to illustrate the points to someone already familiar enough with
the outputs produced by "git send-mail".
If that isn't good enough for the intended audience of the Git
repository log, I unfortunately see no good way to provide excerpts
that are long enough to eliminate any doubts. Such excerpts would
need to be half a dozen screens long, which would turn the patch
description into a monster.
With all that in mind, perhaps it's the best to simply delete all
excerpts from the patch description, if you agree?
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.
Basically, I went with a rather simple reasoning: the confirmation
prompts, just like the SMTP statuses, aren't part of the emitted SMTP
traces and patch descriptions. They're different kinds of emitted
messages, if you agree.
Thus, separating the prompts with vertical whitespace is actually
consistent, and should help with the overall readability, by taking
the prompts visually out of the other produced messages. In other
words, it's about keeping different kinds of emitted messages
separate, with the focus on the SMTP traces and patch descriptions,
instead of making the prompts locatable.
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.
