Subject: [PATCH v6 0/2] send-email: make produced outputs more readable

From: Dragan Simic <[email-masked]>
Date: Wed, 10 Apr 2024 09:01:28 +0200

* 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/[email-masked]/T/#u
Link to v2: https://lore.kernel.org/git/[email-masked]/T/#u
Link to v3: https://lore.kernel.org/git/[email-masked]/T/#u
Link to v4: https://lore.kernel.org/git/[email-masked]/T/#u
Link to v5: https://lore.kernel.org/git/[email-masked]/T/#u

[1] https://lore.kernel.org/git/[email-masked]/
[2] https://lore.kernel.org/git/[email-masked]/
[3] https://lore.kernel.org/git/[email-masked]/
[4] https://lore.kernel.org/git/[email-masked]/
[5] https://lore.kernel.org/git/[email-masked]/
[6] https://lore.kernel.org/git/[email-masked]/
[7] https://lore.kernel.org/git/[email-masked]/

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 <[email-masked]>
     
      ## 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 <[email-masked]>
     
      ## 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:<[email-masked]>
             ...
     
    -    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:<[email-masked]>
    +        ...
    +
    +    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:<[email-masked]>
    +        ...
    +
             ...
    +        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:<[email-masked]>
    +        ...
     
         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 <[email-masked]>
         Signed-off-by: Dragan Simic <[email-masked]>
     
      ## 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 <[email-masked]> from line 'From: A <[email-masked]>'
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF
    + (mbox) Adding cc: A <[email-masked]> from line 'From: A <[email-masked]>'
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF
    + (mbox) Adding cc: A <[email-masked]> from line 'From: A <[email-masked]>'
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-cccmd <<\EOF
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    + (body) Adding cc: C O Mitter <[email-masked]> from line 'Signed-off-by: C O Mitter <[email-masked]>'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ 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:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-body <<\EOF
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    + (cc-cmd) Adding cc: [email-masked] from: './cccmd'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-body-cccmd <<\EOF
    + (mbox) Adding cc: A <[email-masked]> from line 'From: A <[email-masked]>'
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-sob <<\EOF
    + (mbox) Adding cc: A <[email-masked]> from line 'From: A <[email-masked]>'
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-bodycc <<\EOF
    + (mbox) Adding cc: One <[email-masked]> from line 'Cc: One <[email-masked]>, [email-masked]'
    + (mbox) Adding cc: [email-masked] from line 'Cc: One <[email-masked]>, [email-masked]'
    + (body) Adding cc: C O Mitter <[email-masked]> from line 'Signed-off-by: C O Mitter <[email-masked]>'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
    +@@ t/t9001-send-email.sh: cat >expected-suppress-cc <<\EOF
    + 0001-Second.patch
    + (mbox) Adding cc: A <[email-masked]> from line 'From: A <[email-masked]>'
    + (body) Adding cc: C O Mitter <[email-masked]> from line 'Signed-off-by: C O Mitter <[email-masked]>'
    ++
    + Dry-OK. Log says:
    + Server: relay.example.com
    + MAIL FROM:<[email-masked]>
3:  7b99e5c7c0b0 < -:  ------------ send-email: separate the confirmation prompts from the messages