* [PATCH v6 0/2] send-email: make produced outputs more readable
@ 2024-04-10 7:01 Dragan Simic
2024-04-10 7:01 ` [PATCH v6 1/2] send-email: move newline characters out of a few translatable strings Dragan Simic
2024-04-10 7:01 ` [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch Dragan Simic
0 siblings, 2 replies; 15+ messages in thread
From: Dragan Simic @ 2024-04-10 7:01 UTC (permalink / raw
To: git; +Cc: gitster, code
* send-email: make produced outputs more readable by separating
the result statuses and prompts from the other messages
This series makes the outputs of "git send-mail" more readable, by
adding vertical whitespace to make discerning the messages produced
for each patch easy. More specifically, these patches ensure that
single newlines exists before the displayed result statues and after
the displayed confirmation prompts.
Making the outputs of "git send-mail" more readable is quite important,
because the Git users rather regularly complain about the workflows that
require project patches to be sent to various mailing lists. Making the
produced outputs more readable can only help there.
Changes in v6:
- Squashed the 3/3 patch into 2/2, because the simplified logic
no longer requires separate treatment for the prompts
- Moved the emitting of additional newlines into the send_message()
function, and simplified the whole logic; the key is to notice
that the separation of message blocks comes from the emitted SMTP
traces, except when using the "--quiet" option, so emitting the
additional newlines as part of the emitting of the SMTP traces
makes everything much more simple
- Ensured that the newline is emitted before the SMTP trace even
if "sendemail.confirm" is set to "auto" or "never"
- Ensured that the newline is emitted before the brief outputs
when using the "--quit" option, regardless of "sendemail.confirm"
being set to "auto", "always" or "never"
- Improved the naming of a variable, as suggested by Junio [2]
- Improved the patch descriptions, as suggested by Junio, [1][2]
and updated to match the new nature of the patches
- Updated the test t9001 to learn about the additional newlines
Changes in v5:
- Split into a three-patch series, because changes introduced in
some versions of this patch made the original assumptions about
squashing the changes together no longer apply [3]
- Updated and extended the patch descriptions, to hopefully describe
the changes performed in each patch a bit better
Changes in v4:
- Dropped the changes to the styling of the produced prompts, as
reasonably requested by Junio, [4] because it extended the scope
of the patch with no good reason, and may also create issues on
some platforms, whose Perl packages may actually not support the
"->ornaments()" feature of Term::ReadLine
- Updated the patch description and the "what's cooking" summary
to cover the changes
Changes in v3:
- Removed a redundant comment, as suggested by Junio [5]
- Did the right thing and made the vertical separators emitted as
message separators, instead of having them emitted as message
terminators, as suggested by Junio [5]
- Additional vertical whitespace is now also emitted after the
prompt for sending emails, to "de-bunch" it from the subsequent
messages and make discerning the messages easier
- The above-mentioned prompt no longer uses underlined text, to make
it significantly easier on the eyes
- Fixed one indentation by spaces to use tabs and removed one stray
newline in the source code, as spotted
- Updated and extended the patch description to cover the changes
- Updated the "what's cooking" summary to cover the changes
- Cleaned up the older notes a bit
Changes in v2:
- Improved the way additional newline separators are introduced to
the source code, as suggested by Junio, [6], to help a bit with
the translation efforts
- Improved the patch subject and description a bit, to provide some
additional information, as suggested by Junio [6]
- Added a Helped-by tag
Notes for v1:
- This is a resubmission of the patch I submitted about a week and
a half ago; [7] the patch subject in the original submission was
selected in a bit unfortunate way, which this submission corrects,
and also improves the patch description a bit
- There are no changes to the patch itself, vs. the original patch
Link to v1: https://lore.kernel.org/git/62553db377c28458883b66bcdc0c58cc0f32d15b.1712250366.git.dsimic@manjaro.org/T/#u
Link to v2: https://lore.kernel.org/git/0e087ed992def0746f3d437253248904c2126464.1712262791.git.dsimic@manjaro.org/T/#u
Link to v3: https://lore.kernel.org/git/e3212c0a4ad331685c68c13afcdbced20982ab32.1712364420.git.dsimic@manjaro.org/T/#u
Link to v4: https://lore.kernel.org/git/8a9f4927aab96f2f62e2467e59fb6150d7e931fc.1712367983.git.dsimic@manjaro.org/T/#u
Link to v5: https://lore.kernel.org/git/cover.1712486910.git.dsimic@manjaro.org/T/#u
[1] https://lore.kernel.org/git/xmqq7ch7a7gt.fsf@gitster.g/
[2] https://lore.kernel.org/git/xmqq1q7fa6u7.fsf@gitster.g/
[3] https://lore.kernel.org/git/713c28cfc9dff2d01159105ddd2fd0f5@manjaro.org/
[4] https://lore.kernel.org/git/xmqq8r1rs39g.fsf@gitster.g/
[5] https://lore.kernel.org/git/xmqqzfu8yc40.fsf@gitster.g/
[6] https://lore.kernel.org/git/xmqqy19tylrm.fsf@gitster.g/
[7] https://lore.kernel.org/git/6ee28707b9eb8bd8fdfc8756c351455c6bc3bb62.1711447365.git.dsimic@manjaro.org/
Dragan Simic (2):
send-email: move newline characters out of a few translatable strings
send-email: make it easy to discern the messages for each patch
git-send-email.perl | 19 ++++++++++++-------
t/t9001-send-email.sh | 10 ++++++++++
2 files changed, 22 insertions(+), 7 deletions(-)
Range-diff against v5:
1: d2d456edcb5b ! 1: 29ea3a9b07bf send-email: move newline character out of a translatable string
@@ Metadata
Author: Dragan Simic <dsimic@manjaro.org>
## Commit message ##
- send-email: move newline character out of a translatable string
+ send-email: move newline characters out of a few translatable strings
- Move the already existing newline character out of a translatable string,
- to help a bit with the translation efforts.
+ Move the already existing newline characters out of a few translatable
+ strings, to help a bit with the translation efforts.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
## git-send-email.perl ##
+@@ git-send-email.perl: sub send_message {
+ $smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
+ }
+ if ($quiet) {
+- printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject);
++ printf($dry_run ? __("Dry-Sent %s") : __("Sent %s"), $subject);
++ print "\n";
+ } else {
+- print($dry_run ? __("Dry-OK. Log says:\n") : __("OK. Log says:\n"));
++ print($dry_run ? __("Dry-OK. Log says:") : __("OK. Log says:"));
++ print "\n";
+ if (!defined $sendmail_cmd && !file_name_is_absolute($smtp_server)) {
+ print "Server: $smtp_server\n";
+ print "MAIL FROM:<$raw_from>\n";
@@ git-send-email.perl: sub send_message {
print $header, "\n";
if ($smtp) {
2: 7f8738308901 ! 2: c78b043b5a6c send-email: make it easy to discern the messages for each patch
@@ Metadata
## Commit message ##
send-email: make it easy to discern the messages for each patch
- When sending multiple patches at once, without prompting the user to confirm
- the sending of each patch separately, the displayed result statuses for each
- patch become bunched together with the messages produced for the subsequent
- patch. This unnecessarily makes discerning each of the result statuses a bit
- difficult, as visible in the sample output excerpt below:
+ 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:
...
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: 250
OK. Log says:
+ Server: smtp.example.com
+ MAIL FROM:<test@example.com>
...
- As visible in the excerpt above, bunching the "Result: <status-code>" lines
- together with the messages produced for the subsequent patch makes the output
- unreadable, which actually becomes worse as the number of patches sent at
- once increases. To make the produced outputs more readable, add vertical
- whitespace (more precisely, a newline) between the displayed result statuses
- and the subsequent messages, as visible in the sample output excerpt below,
- produced after the addition of vertical whitespace:
+ ...
+ 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>
+ ...
+
+ As visible in the excerpts above, bunching the "Result: <status-code>" lines
+ or the "Send this email [y/n/a/...]?" confirmation prompts together with the
+ other messages makes the outputs a bit unreadable, which actually becomes
+ worse as the number of patches sent at once increases.
+
+ To make the produced outputs more readable, ensure that vertical whitespace
+ (more precisely, single newlines) exist before the displayed result statuses
+ and after the confirmation prompts, as visible in the two updated excerpts
+ from a sample output below:
...
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: 250
OK. Log says:
+ Server: smtp.example.com
+ MAIL FROM:<test@example.com>
+ ...
+
...
+ MIME-Version: 1.0
+ Content-Transfer-Encoding: 8bit
+
+ Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
- These changes don't emit additional vertical whitespace after the result
- status produced for the last processed patch, i.e. the vertical whitespace
- is treated as a separator between the groups of produced messages, not as
- their terminator. This follows the Git's general approach of not wasting
- the vertical screen space whenever reasonably possible.
+ OK. Log says:
+ Server: smtp.example.com
+ MAIL FROM:<test@example.com>
+ ...
While there, remove a couple of spotted stray newlines in the source code
and convert one indentation from spaces to tabs, for consistency.
- The associated test, t9001, requires no updates to cover these changes.
+ Update the associated test, t9001, by including additional newlines into
+ the expected outputs of separate tests affected by these changes.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
## git-send-email.perl ##
-@@ git-send-email.perl: sub format_2822_time {
- my $compose_filename;
- my $force = 0;
- my $dump_aliases = 0;
-+my $needs_separator = 0;
-
- # Variables to prevent short format-patch options from being captured
- # as abbreviated send-email options
@@ git-send-email.perl: 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;
+@@ git-send-email.perl: 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;
@@ git-send-email.perl: sub send_message {
- exit(0);
} elsif (/^a/i) {
$confirm = 'never';
-+ $needs_separator = 1;
}
-+ } else {
-+ $needs_separator = 1;
++ $confirm_shown = 1;
}
unshift (@sendmail_parameters, @smtp_server_options);
@@ git-send-email.perl: 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.")
}
+@@ git-send-email.perl: 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)) {
@@ git-send-email.perl: sub pre_process_file {
sub process_file {
my ($t) = @_;
- pre_process_file($t, $quiet);
+ pre_process_file($t, $quiet);
-+ print "\n" if ($needs_separator);
my $message_was_sent = send_message();
if ($message_was_sent == -1) {
+
+ ## t/t9001-send-email.sh ##
+@@ t/t9001-send-email.sh: cat >expected-show-all-headers <<\EOF
+ (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF
+ (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF
+ (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-cccmd <<\EOF
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+ (body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: test_expect_success $PREREQ 'sendemail.cccmd' '
+ test_expect_success $PREREQ 'setup expect' '
+ cat >expected-suppress-all <<\EOF
+ 0001-Second.patch
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-body <<\EOF
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+ (cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-body-cccmd <<\EOF
+ (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF
+ (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-bodycc <<\EOF
+ (mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
+ (mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+ (body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
+@@ t/t9001-send-email.sh: cat >expected-suppress-cc <<\EOF
+ 0001-Second.patch
+ (mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
+ (body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
++
+ Dry-OK. Log says:
+ Server: relay.example.com
+ MAIL FROM:<from@example.com>
3: 7b99e5c7c0b0 < -: ------------ send-email: separate the confirmation prompts from the messages
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v6 1/2] send-email: move newline characters out of a few translatable strings
2024-04-10 7:01 [PATCH v6 0/2] send-email: make produced outputs more readable Dragan Simic
@ 2024-04-10 7:01 ` Dragan Simic
2024-04-10 16:12 ` Junio C Hamano
2024-04-10 7:01 ` [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch Dragan Simic
1 sibling, 1 reply; 15+ messages in thread
From: Dragan Simic @ 2024-04-10 7:01 UTC (permalink / raw
To: git; +Cc: gitster, code
Move the already existing newline characters out of a few translatable
strings, to help a bit with the translation efforts.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
git-send-email.perl | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 821b2b3a135a..f0be4b4560f7 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1664,9 +1664,11 @@ sub send_message {
$smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
}
if ($quiet) {
- printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject);
+ printf($dry_run ? __("Dry-Sent %s") : __("Sent %s"), $subject);
+ print "\n";
} else {
- print($dry_run ? __("Dry-OK. Log says:\n") : __("OK. Log says:\n"));
+ print($dry_run ? __("Dry-OK. Log says:") : __("OK. Log says:"));
+ print "\n";
if (!defined $sendmail_cmd && !file_name_is_absolute($smtp_server)) {
print "Server: $smtp_server\n";
print "MAIL FROM:<$raw_from>\n";
@@ -1686,10 +1688,11 @@ sub send_message {
print $header, "\n";
if ($smtp) {
print __("Result: "), $smtp->code, ' ',
- ($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
+ ($smtp->message =~ /\n([^\n]+\n)$/s);
} else {
- print __("Result: OK\n");
+ print __("Result: OK");
}
+ print "\n";
}
return 1;
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-10 7:01 [PATCH v6 0/2] send-email: make produced outputs more readable Dragan Simic
2024-04-10 7:01 ` [PATCH v6 1/2] send-email: move newline characters out of a few translatable strings Dragan Simic
@ 2024-04-10 7:01 ` Dragan Simic
2024-04-10 16:28 ` Junio C Hamano
1 sibling, 1 reply; 15+ messages in thread
From: Dragan Simic @ 2024-04-10 7:01 UTC (permalink / raw
To: git; +Cc: gitster, code
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:
...
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: 250
OK. Log says:
Server: smtp.example.com
MAIL FROM:<test@example.com>
...
...
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>
...
As visible in the excerpts above, bunching the "Result: <status-code>" lines
or the "Send this email [y/n/a/...]?" confirmation prompts together with the
other messages makes the outputs a bit unreadable, which actually becomes
worse as the number of patches sent at once increases.
To make the produced outputs more readable, ensure that vertical whitespace
(more precisely, single newlines) exist before the displayed result statuses
and after the confirmation prompts, as visible in the two updated excerpts
from a sample output below:
...
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: 250
OK. Log says:
Server: smtp.example.com
MAIL FROM:<test@example.com>
...
...
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>
...
While there, remove a couple of spotted stray newlines in the source code
and convert one indentation from spaces to tabs, for consistency.
Update the associated test, t9001, by including additional newlines into
the expected outputs of separate tests affected by these changes.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
git-send-email.perl | 8 +++++---
t/t9001-send-email.sh | 10 ++++++++++
2 files changed, 15 insertions(+), 3 deletions(-)
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) {
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 5a771000c9ca..a5cb198a5c5f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -208,6 +208,7 @@ cat >expected-show-all-headers <<\EOF
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -821,6 +822,7 @@ cat >expected-suppress-sob <<\EOF
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -869,6 +871,7 @@ cat >expected-suppress-sob <<\EOF
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -904,6 +907,7 @@ cat >expected-suppress-cccmd <<\EOF
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -940,6 +944,7 @@ test_expect_success $PREREQ 'sendemail.cccmd' '
test_expect_success $PREREQ 'setup expect' '
cat >expected-suppress-all <<\EOF
0001-Second.patch
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -968,6 +973,7 @@ cat >expected-suppress-body <<\EOF
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
(cc-cmd) Adding cc: cc-cmd@example.com from: './cccmd'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -1003,6 +1009,7 @@ cat >expected-suppress-body-cccmd <<\EOF
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -1036,6 +1043,7 @@ cat >expected-suppress-sob <<\EOF
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -1071,6 +1079,7 @@ cat >expected-suppress-bodycc <<\EOF
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
@@ -1105,6 +1114,7 @@ cat >expected-suppress-cc <<\EOF
0001-Second.patch
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
+
Dry-OK. Log says:
Server: relay.example.com
MAIL FROM:<from@example.com>
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v6 1/2] send-email: move newline characters out of a few translatable strings
2024-04-10 7:01 ` [PATCH v6 1/2] send-email: move newline characters out of a few translatable strings Dragan Simic
@ 2024-04-10 16:12 ` Junio C Hamano
2024-04-13 6:12 ` Dragan Simic
0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2024-04-10 16:12 UTC (permalink / raw
To: Dragan Simic; +Cc: git, code
Dragan Simic <dsimic@manjaro.org> writes:
> Move the already existing newline characters out of a few translatable
> strings, to help a bit with the translation efforts.
>
> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
> ---
> git-send-email.perl | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
Looking good. Thanks for spotting similar issues and fixing them at
the same time.
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 821b2b3a135a..f0be4b4560f7 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1664,9 +1664,11 @@ sub send_message {
> $smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
> }
> if ($quiet) {
> - printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject);
> + printf($dry_run ? __("Dry-Sent %s") : __("Sent %s"), $subject);
> + print "\n";
> } else {
> - print($dry_run ? __("Dry-OK. Log says:\n") : __("OK. Log says:\n"));
> + print($dry_run ? __("Dry-OK. Log says:") : __("OK. Log says:"));
> + print "\n";
> if (!defined $sendmail_cmd && !file_name_is_absolute($smtp_server)) {
> print "Server: $smtp_server\n";
> print "MAIL FROM:<$raw_from>\n";
> @@ -1686,10 +1688,11 @@ sub send_message {
> print $header, "\n";
> if ($smtp) {
> print __("Result: "), $smtp->code, ' ',
> - ($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
> + ($smtp->message =~ /\n([^\n]+\n)$/s);
> } else {
> - print __("Result: OK\n");
> + print __("Result: OK");
> }
> + print "\n";
> }
>
> return 1;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-10 7:01 ` [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch Dragan Simic
@ 2024-04-10 16:28 ` Junio C Hamano
2024-04-10 22:59 ` Eric Sunshine
2024-04-13 6:27 ` Dragan Simic
0 siblings, 2 replies; 15+ messages in thread
From: Junio C Hamano @ 2024-04-10 16:28 UTC (permalink / raw
To: Dragan Simic; +Cc: git, code
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-10 16:28 ` Junio C Hamano
@ 2024-04-10 22:59 ` Eric Sunshine
2024-04-13 6:10 ` Dragan Simic
2024-04-13 6:27 ` Dragan Simic
1 sibling, 1 reply; 15+ messages in thread
From: Eric Sunshine @ 2024-04-10 22:59 UTC (permalink / raw
To: Junio C Hamano; +Cc: Dragan Simic, git, code
On Wed, Apr 10, 2024 at 12:28 PM Junio C Hamano <gitster@pobox.com> wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
> > 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
>
> 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.
>
> I'll let others comment as the "blank around prompt" smells quite
> subjective and do not want to be the sole reviewer on it.
For what it's worth, I had the same reaction about the extra blank
line related to the prompt being unnecessary, but I didn't feel
strongly enough to say anything about it, and I still don't feel
strongly enough to suggest dropping this change.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-10 22:59 ` Eric Sunshine
@ 2024-04-13 6:10 ` Dragan Simic
0 siblings, 0 replies; 15+ messages in thread
From: Dragan Simic @ 2024-04-13 6:10 UTC (permalink / raw
To: Eric Sunshine; +Cc: Junio C Hamano, git, code
Hello Eric,
On 2024-04-11 00:59, Eric Sunshine wrote:
> On Wed, Apr 10, 2024 at 12:28 PM Junio C Hamano <gitster@pobox.com>
> wrote:
>> Dragan Simic <dsimic@manjaro.org> writes:
>> > 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
>>
>> 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.
>>
>> I'll let others comment as the "blank around prompt" smells quite
>> subjective and do not want to be the sole reviewer on it.
>
> For what it's worth, I had the same reaction about the extra blank
> line related to the prompt being unnecessary, but I didn't feel
> strongly enough to say anything about it, and I still don't feel
> strongly enough to suggest dropping this change.
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. As such, 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 1/2] send-email: move newline characters out of a few translatable strings
2024-04-10 16:12 ` Junio C Hamano
@ 2024-04-13 6:12 ` Dragan Simic
0 siblings, 0 replies; 15+ messages in thread
From: Dragan Simic @ 2024-04-13 6:12 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
Hello Junio,
On 2024-04-10 18:12, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>
>> Move the already existing newline characters out of a few translatable
>> strings, to help a bit with the translation efforts.
>>
>> Signed-off-by: Dragan Simic <dsimic@manjaro.org>
>> ---
>> git-send-email.perl | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> Looking good. Thanks for spotting similar issues and fixing them at
> the same time.
I'm glad that you like it.
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 821b2b3a135a..f0be4b4560f7 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -1664,9 +1664,11 @@ sub send_message {
>> $smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"),
>> $subject).$smtp->message;
>> }
>> if ($quiet) {
>> - printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject);
>> + printf($dry_run ? __("Dry-Sent %s") : __("Sent %s"), $subject);
>> + print "\n";
>> } else {
>> - print($dry_run ? __("Dry-OK. Log says:\n") : __("OK. Log
>> says:\n"));
>> + print($dry_run ? __("Dry-OK. Log says:") : __("OK. Log says:"));
>> + print "\n";
>> if (!defined $sendmail_cmd && !file_name_is_absolute($smtp_server))
>> {
>> print "Server: $smtp_server\n";
>> print "MAIL FROM:<$raw_from>\n";
>> @@ -1686,10 +1688,11 @@ sub send_message {
>> print $header, "\n";
>> if ($smtp) {
>> print __("Result: "), $smtp->code, ' ',
>> - ($smtp->message =~ /\n([^\n]+\n)$/s), "\n";
>> + ($smtp->message =~ /\n([^\n]+\n)$/s);
>> } else {
>> - print __("Result: OK\n");
>> + print __("Result: OK");
>> }
>> + print "\n";
>> }
>>
>> return 1;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-10 16:28 ` Junio C Hamano
2024-04-10 22:59 ` Eric Sunshine
@ 2024-04-13 6:27 ` Dragan Simic
2024-04-27 17:27 ` Dragan Simic
1 sibling, 1 reply; 15+ messages in thread
From: Dragan Simic @ 2024-04-13 6:27 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
On 2024-04-10 18:28, Junio C Hamano wrote:
> 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>
>> ...
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@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.
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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-13 6:27 ` Dragan Simic
@ 2024-04-27 17:27 ` Dragan Simic
2024-04-27 17:41 ` Junio C Hamano
0 siblings, 1 reply; 15+ messages in thread
From: Dragan Simic @ 2024-04-27 17:27 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
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@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>
>>> ...
>
> 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@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.
>
> 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.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-27 17:27 ` Dragan Simic
@ 2024-04-27 17:41 ` Junio C Hamano
2024-04-27 17:49 ` Dragan Simic
0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2024-04-27 17:41 UTC (permalink / raw
To: Dragan Simic; +Cc: git, code
Dragan Simic <dsimic@manjaro.org> writes:
> Just checking, is there something I can do to get this patch
> series moving forward?
Get it into a shape that is more palatable to people? That's easy
to say and hard to define X-<.
I didn't see anybody enthused about the change, and I didn't get
enthused about it, either.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-27 17:41 ` Junio C Hamano
@ 2024-04-27 17:49 ` Dragan Simic
2024-04-27 18:06 ` Junio C Hamano
0 siblings, 1 reply; 15+ messages in thread
From: Dragan Simic @ 2024-04-27 17:49 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
On 2024-04-27 19:41, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>
>> Just checking, is there something I can do to get this patch
>> series moving forward?
>
> Get it into a shape that is more palatable to people? That's easy
> to say and hard to define X-<.
>
> I didn't see anybody enthused about the change, and I didn't get
> enthused about it, either.
Hmm, I was under impression that the amount of discussion that
already went into the different versions of this series clearly
showed at least some interest in these patches. We've reached
the sixth iteration, which shows something.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-27 17:49 ` Dragan Simic
@ 2024-04-27 18:06 ` Junio C Hamano
2024-04-27 18:18 ` Junio C Hamano
0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2024-04-27 18:06 UTC (permalink / raw
To: Dragan Simic; +Cc: git, code
Dragan Simic <dsimic@manjaro.org> writes:
> On 2024-04-27 19:41, Junio C Hamano wrote:
>> Dragan Simic <dsimic@manjaro.org> writes:
>>
>>> Just checking, is there something I can do to get this patch
>>> series moving forward?
>> Get it into a shape that is more palatable to people? That's easy
>> to say and hard to define X-<.
>> I didn't see anybody enthused about the change, and I didn't get
>> enthused about it, either.
>
> Hmm, I was under impression that the amount of discussion that
> already went into the different versions of this series clearly
> showed at least some interest in these patches. We've reached
> the sixth iteration, which shows something.
Well, v1 and v3-v5 saw comments only from me. There was a comment
on v2 by Kristoffer Haugsbakk about phrasing in proposed log
message. v6 had Eric's "Meh, either way".
I was not enthused in the topic myself, but was reviewing and
suggesting improvements merely to help you turn the patch into a
shape that would interest others, but the thing is, if the current
output is good enough, it is perfectly normal that there is no more
room to make material improvements, and it is not your fault.
[References]
v1. https://lore.kernel.org/git/62553db377c28458883b66bcdc0c58cc0f32d15b.1712250366.git.dsimic@manjaro.org/
v2. https://lore.kernel.org/git/0e087ed992def0746f3d437253248904c2126464.1712262791.git.dsimic@manjaro.org/
v3. https://lore.kernel.org/git/e3212c0a4ad331685c68c13afcdbced20982ab32.1712364420.git.dsimic@manjaro.org/
v4. https://lore.kernel.org/git/8a9f4927aab96f2f62e2467e59fb6150d7e931fc.1712367983.git.dsimic@manjaro.org/
v5. https://lore.kernel.org/git/cover.1712486910.git.dsimic@manjaro.org/
v6. https://lore.kernel.org/git/cover.1712732383.git.dsimic@manjaro.org/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-27 18:06 ` Junio C Hamano
@ 2024-04-27 18:18 ` Junio C Hamano
2024-04-28 3:03 ` Dragan Simic
0 siblings, 1 reply; 15+ messages in thread
From: Junio C Hamano @ 2024-04-27 18:18 UTC (permalink / raw
To: Dragan Simic; +Cc: git, code
Junio C Hamano <gitster@pobox.com> writes:
> Well, v1 and v3-v5 saw comments only from me. There was a comment
> on v2 by Kristoffer Haugsbakk about phrasing in proposed log
> message. v6 had Eric's "Meh, either way".
FWIW, somewhere between v2 and v3 may have been a good place to
stop.
"After calling send_message(), give an extra blank line before the
next message" was extremely easy to see why it was a good idea.
Mixing it with "oh, during the process of a single invocation of
send_message(), there may be other places that want extra blank
lines" made the changes less focused. Personally I lost interest
in the series at around that point.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch
2024-04-27 18:18 ` Junio C Hamano
@ 2024-04-28 3:03 ` Dragan Simic
0 siblings, 0 replies; 15+ messages in thread
From: Dragan Simic @ 2024-04-28 3:03 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
On 2024-04-27 20:18, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Well, v1 and v3-v5 saw comments only from me. There was a comment
>> on v2 by Kristoffer Haugsbakk about phrasing in proposed log
>> message. v6 had Eric's "Meh, either way".
>
> FWIW, somewhere between v2 and v3 may have been a good place to
> stop.
>
> "After calling send_message(), give an extra blank line before the
> next message" was extremely easy to see why it was a good idea.
> Mixing it with "oh, during the process of a single invocation of
> send_message(), there may be other places that want extra blank
> lines" made the changes less focused. Personally I lost interest
> in the series at around that point.
Thank you for your additional feedback.
Admittedly, the whole endeavor could have been handled better from
my side, with fewer "oh, let's do it differently and a bit better"
iterations that inevitably made the whole thing harder to follow and
become interested with.
However, I find improving the way "git send-mail" works rather
important,
because if it bothers me, it must be bothering other people, who may
simply not be on the mailing list. As we know, a lot of people quite
regularly complain through different channels about sending patches by
email being not user-friendly, which should make any attempts to make
it more user-friendly quite important, if you agree.
I have some more plans for improving "git send-mail" and adding some new
features, but I need to get everything into the final shape I'm pretty
much 100% satisfied with, both from the design and the implementation
standpoints, before sending the patches over.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2024-04-28 3:03 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-10 7:01 [PATCH v6 0/2] send-email: make produced outputs more readable Dragan Simic
2024-04-10 7:01 ` [PATCH v6 1/2] send-email: move newline characters out of a few translatable strings Dragan Simic
2024-04-10 16:12 ` Junio C Hamano
2024-04-13 6:12 ` Dragan Simic
2024-04-10 7:01 ` [PATCH v6 2/2] send-email: make it easy to discern the messages for each patch Dragan Simic
2024-04-10 16:28 ` Junio C Hamano
2024-04-10 22:59 ` Eric Sunshine
2024-04-13 6:10 ` Dragan Simic
2024-04-13 6:27 ` Dragan Simic
2024-04-27 17:27 ` Dragan Simic
2024-04-27 17:41 ` Junio C Hamano
2024-04-27 17:49 ` Dragan Simic
2024-04-27 18:06 ` Junio C Hamano
2024-04-27 18:18 ` Junio C Hamano
2024-04-28 3:03 ` Dragan Simic
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.