* 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@xxxxxxxxxxx/T/#u Link to v2: https://lore.kernel.org/git/0e087ed992def0746f3d437253248904c2126464.1712262791.git.dsimic@xxxxxxxxxxx/T/#u Link to v3: https://lore.kernel.org/git/e3212c0a4ad331685c68c13afcdbced20982ab32.1712364420.git.dsimic@xxxxxxxxxxx/T/#u Link to v4: https://lore.kernel.org/git/8a9f4927aab96f2f62e2467e59fb6150d7e931fc.1712367983.git.dsimic@xxxxxxxxxxx/T/#u Link to v5: https://lore.kernel.org/git/cover.1712486910.git.dsimic@xxxxxxxxxxx/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@xxxxxxxxxxx/ [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@xxxxxxxxxxx/ 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@xxxxxxxxxxx> ## 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@xxxxxxxxxxx> ## 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@xxxxxxxxxxx> ... - 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@xxxxxxxxxxx> + ... + + 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@xxxxxxxxxxx> + ... + ... + 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@xxxxxxxxxxx> + ... 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@xxxxxxxxx> Signed-off-by: Dragan Simic <dsimic@xxxxxxxxxxx> ## 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@xxxxxxxxxxx> from line 'From: A <author@xxxxxxxxxxx>' + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF + (mbox) Adding cc: A <author@xxxxxxxxxxx> from line 'From: A <author@xxxxxxxxxxx>' + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF + (mbox) Adding cc: A <author@xxxxxxxxxxx> from line 'From: A <author@xxxxxxxxxxx>' + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-cccmd <<\EOF + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (body) Adding cc: C O Mitter <committer@xxxxxxxxxxx> from line 'Signed-off-by: C O Mitter <committer@xxxxxxxxxxx>' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ 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@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-body <<\EOF + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (cc-cmd) Adding cc: cc-cmd@xxxxxxxxxxx from: './cccmd' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-body-cccmd <<\EOF + (mbox) Adding cc: A <author@xxxxxxxxxxx> from line 'From: A <author@xxxxxxxxxxx>' + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF + (mbox) Adding cc: A <author@xxxxxxxxxxx> from line 'From: A <author@xxxxxxxxxxx>' + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-bodycc <<\EOF + (mbox) Adding cc: One <one@xxxxxxxxxxx> from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (mbox) Adding cc: two@xxxxxxxxxxx from line 'Cc: One <one@xxxxxxxxxxx>, two@xxxxxxxxxxx' + (body) Adding cc: C O Mitter <committer@xxxxxxxxxxx> from line 'Signed-off-by: C O Mitter <committer@xxxxxxxxxxx>' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> +@@ t/t9001-send-email.sh: cat >expected-suppress-cc <<\EOF + 0001-Second.patch + (mbox) Adding cc: A <author@xxxxxxxxxxx> from line 'From: A <author@xxxxxxxxxxx>' + (body) Adding cc: C O Mitter <committer@xxxxxxxxxxx> from line 'Signed-off-by: C O Mitter <committer@xxxxxxxxxxx>' ++ + Dry-OK. Log says: + Server: relay.example.com + MAIL FROM:<from@xxxxxxxxxxx> 3: 7b99e5c7c0b0 < -: ------------ send-email: separate the confirmation prompts from the messages