* [PATCH v5 0/3] send-email: make produced outputs more readable
@ 2024-04-07 10:48 Dragan Simic
2024-04-07 10:48 ` [PATCH v5 1/3] send-email: move newline character out of a translatable string Dragan Simic
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Dragan Simic @ 2024-04-07 10:48 UTC (permalink / raw
To: git; +Cc: gitster, code
* send-email: make produced outputs more readable by separating
the result statuses and prompts from the subsequent 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, and by adding vertical whitespace to separate the
emitted prompts from the other produced messages. Having the prompts
and some of the produced messages bunched together made the navigation
through the produced outputs unnecessarily hard.
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 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 [1]
- 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, [2] 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 [3]
- 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 [3]
- 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, [4], 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 [4]
- Added a Helped-by tag
Notes for v1:
- This is a resubmission of the patch I submitted about a week and
a half ago; [5] 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
[1] https://lore.kernel.org/git/713c28cfc9dff2d01159105ddd2fd0f5@manjaro.org/
[2] https://lore.kernel.org/git/xmqq8r1rs39g.fsf@gitster.g/
[3] https://lore.kernel.org/git/xmqqzfu8yc40.fsf@gitster.g/
[4] https://lore.kernel.org/git/xmqqy19tylrm.fsf@gitster.g/
[5] https://lore.kernel.org/git/6ee28707b9eb8bd8fdfc8756c351455c6bc3bb62.1711447365.git.dsimic@manjaro.org/
Dragan Simic (3):
send-email: move newline character out of a translatable string
send-email: make it easy to discern the messages for each patch
send-email: separate the confirmation prompts from the messages
git-send-email.perl | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/3] send-email: move newline character out of a translatable string
2024-04-07 10:48 [PATCH v5 0/3] send-email: make produced outputs more readable Dragan Simic
@ 2024-04-07 10:48 ` Dragan Simic
2024-04-07 10:48 ` [PATCH v5 2/3] send-email: make it easy to discern the messages for each patch Dragan Simic
2024-04-07 10:48 ` [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages Dragan Simic
2 siblings, 0 replies; 10+ messages in thread
From: Dragan Simic @ 2024-04-07 10:48 UTC (permalink / raw
To: git; +Cc: gitster, code
Move the already existing newline character out of a translatable string,
to help a bit with the translation efforts.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
git-send-email.perl | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 821b2b3a135a..a22f299ba051 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1686,10 +1686,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] 10+ messages in thread
* [PATCH v5 2/3] send-email: make it easy to discern the messages for each patch
2024-04-07 10:48 [PATCH v5 0/3] send-email: make produced outputs more readable Dragan Simic
2024-04-07 10:48 ` [PATCH v5 1/3] send-email: move newline character out of a translatable string Dragan Simic
@ 2024-04-07 10:48 ` Dragan Simic
2024-04-08 21:08 ` Junio C Hamano
2024-04-07 10:48 ` [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages Dragan Simic
2 siblings, 1 reply; 10+ messages in thread
From: Dragan Simic @ 2024-04-07 10:48 UTC (permalink / raw
To: git; +Cc: gitster, code
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:
...
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Result: 250
OK. Log says:
...
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
Result: 250
OK. Log says:
...
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.
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.
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
git-send-email.perl | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index a22f299ba051..4127fbe6b936 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -212,6 +212,7 @@ 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
@@ -1361,7 +1362,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;
@@ -1554,7 +1554,10 @@ sub send_message {
exit(0);
} elsif (/^a/i) {
$confirm = 'never';
+ $needs_separator = 1;
}
+ } else {
+ $needs_separator = 1;
}
unshift (@sendmail_parameters, @smtp_server_options);
@@ -1576,7 +1579,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.")
}
@@ -1921,7 +1923,8 @@ 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) {
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages
2024-04-07 10:48 [PATCH v5 0/3] send-email: make produced outputs more readable Dragan Simic
2024-04-07 10:48 ` [PATCH v5 1/3] send-email: move newline character out of a translatable string Dragan Simic
2024-04-07 10:48 ` [PATCH v5 2/3] send-email: make it easy to discern the messages for each patch Dragan Simic
@ 2024-04-07 10:48 ` Dragan Simic
2024-04-08 21:21 ` Junio C Hamano
2 siblings, 1 reply; 10+ messages in thread
From: Dragan Simic @ 2024-04-07 10:48 UTC (permalink / raw
To: git; +Cc: gitster, code
Emit additional vertical whitespace after the "Send this email [y/n/...]?"
confirmation prompts, more specifically after each confirmed email is sent,
but before the subsequent messages are emitted, to make the produced output
more readable. The subsequent produced messages were bunched together with
the confirmation prompts, as visible in the sample output excerpt below,
which made discerning the outputs unnecessarily harder.
...
Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
OK. Log says:
...
The introduced changes don't emit additional vertical whitespace after the
confirmation prompt if the user selects to skip sending the email they were
asked about, or if the user selects to quit the procedure entirely. This
follows the Git's general approach of not wasting the vertical screen space
whenever reasonably possible.
The associated test, t9001, requires no updates to cover these changes.
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
---
git-send-email.perl | 3 +++
1 file changed, 3 insertions(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index 4127fbe6b936..a09bc7fd6b96 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1510,6 +1510,7 @@ sub gen_header {
sub send_message {
my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
my @recipients = @$recipients_ref;
+ my $prompt_separator = 0;
my @sendmail_parameters = ('-i', @recipients);
my $raw_from = $sender;
@@ -1556,6 +1557,7 @@ sub send_message {
$confirm = 'never';
$needs_separator = 1;
}
+ $prompt_separator = 1;
} else {
$needs_separator = 1;
}
@@ -1665,6 +1667,7 @@ sub send_message {
$smtp->dataend() or die $smtp->message;
$smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
}
+ print "\n" if ($prompt_separator);
if ($quiet) {
printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject);
} else {
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] send-email: make it easy to discern the messages for each patch
2024-04-07 10:48 ` [PATCH v5 2/3] send-email: make it easy to discern the messages for each patch Dragan Simic
@ 2024-04-08 21:08 ` Junio C Hamano
2024-04-09 3:37 ` Dragan Simic
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2024-04-08 21:08 UTC (permalink / raw
To: Dragan Simic; +Cc: git, code
Dragan Simic <dsimic@manjaro.org> writes:
> ... 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 above feels a bit roundabout way to say "the logic is that we
need to add a gap before showing the next message, if we did things
that cause the smtp traces to be shown", but OK.
> 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.
I do not see this paragraph is relevant to the target audience. It
may be a good advice to give to a reader who attempts to solve the
problem this patch solved themselves, botches the attempt and ends
up with a code with the terminator semantics. But for other readers
of "git log" and reviewers of the patch, "I did not make a silly
mistake, and instead correctly chose to use the separator semantics"
is not something worth boasting about.
> 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.
These are worth recording.
> @@ -1554,7 +1554,10 @@ sub send_message {
> exit(0);
> } elsif (/^a/i) {
> $confirm = 'never';
> + $needs_separator = 1;
> }
> + } else {
> + $needs_separator = 1;
> }
If you do not add this "else" clause to the outer "are we doing
confirmation?" if statement, and instead just set $needs_separator
*after* it, it would make it even more obvious what is going on.
The codeflow would become
sub send_message {
do bunch of things that do not yet send e-mail
and possibly return or die
$needs_separator = 1;
do things that cause the smtp exchange and trace
to be emitted
}
That makes it obvious that the purpose of $needs_separator is to
record the fact that "this" message has already been sent and we
need to add a "gap" before attempting to send the "next" message.
Other than the above points, very well done.
> unshift (@sendmail_parameters, @smtp_server_options);
> @@ -1576,7 +1579,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.")
> }
> @@ -1921,7 +1923,8 @@ sub pre_process_file {
> sub process_file {
> my ($t) = @_;
>
> - pre_process_file($t, $quiet);
> + pre_process_file($t, $quiet);
nice ;-)
> + print "\n" if ($needs_separator);
>
> my $message_was_sent = send_message();
> if ($message_was_sent == -1) {
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages
2024-04-07 10:48 ` [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages Dragan Simic
@ 2024-04-08 21:21 ` Junio C Hamano
2024-04-09 3:25 ` Dragan Simic
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2024-04-08 21:21 UTC (permalink / raw
To: Dragan Simic; +Cc: git, code
Dragan Simic <dsimic@manjaro.org> writes:
> Emit additional vertical whitespace after the "Send this email [y/n/...]?"
> confirmation prompts, more specifically after each confirmed email is sent,
> but before the subsequent messages are emitted, to make the produced output
> more readable. The subsequent produced messages were bunched together with
> the confirmation prompts, as visible in the sample output excerpt below,
> which made discerning the outputs unnecessarily harder.
>
> ...
> Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
> OK. Log says:
> ...
What comes before "send this email" prompt needs to be shown to make
the argument more convincing, but with "..." there is no cue to decide
if the output is hard to read.
> sub send_message {
> my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
> my @recipients = @$recipients_ref;
> + my $prompt_separator = 0;
>
> my @sendmail_parameters = ('-i', @recipients);
> my $raw_from = $sender;
> @@ -1556,6 +1557,7 @@ sub send_message {
> $confirm = 'never';
> $needs_separator = 1;
> }
> + $prompt_separator = 1;
> } else {
> $needs_separator = 1;
> }
> @@ -1665,6 +1667,7 @@ sub send_message {
> $smtp->dataend() or die $smtp->message;
> $smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"), $subject).$smtp->message;
> }
> + print "\n" if ($prompt_separator);
"prompt separator" sounds more like a separator that separates
prompts, but that is not what is going on, no?
Do we even need that new varible in the first place? I am wondering
if you can just do the print "\n" right where you assign to that
variable.
When $confirm is set to 'never', you have both $needs_separtor and
$prompt_separator set. Would it mean you'd have two extra blank
lines for that message?
All these questions you should have been able to avoided with a bit
more helpful explanation in the proposed log message, I hope.
Thanks.
> if ($quiet) {
> printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject);
> } else {
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages
2024-04-08 21:21 ` Junio C Hamano
@ 2024-04-09 3:25 ` Dragan Simic
2024-04-10 3:53 ` Dragan Simic
0 siblings, 1 reply; 10+ messages in thread
From: Dragan Simic @ 2024-04-09 3:25 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
Hello Junio,
On 2024-04-08 23:21, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>
>> Emit additional vertical whitespace after the "Send this email
>> [y/n/...]?"
>> confirmation prompts, more specifically after each confirmed email is
>> sent,
>> but before the subsequent messages are emitted, to make the produced
>> output
>> more readable. The subsequent produced messages were bunched together
>> with
>> the confirmation prompts, as visible in the sample output excerpt
>> below,
>> which made discerning the outputs unnecessarily harder.
>>
>> ...
>> Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
>> OK. Log says:
>> ...
>
> What comes before "send this email" prompt needs to be shown to make
> the argument more convincing, but with "..." there is no cue to decide
> if the output is hard to read.
Believe it or not, I also saw the provided sample excerpt as
too short. However, when I tried to make it longer and more
obvious, it turned out that simply too many lines needed to be
included. I'll give it some more though, and possibly delete
the sample entirely.
>> sub send_message {
>> my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header)
>> = gen_header();
>> my @recipients = @$recipients_ref;
>> + my $prompt_separator = 0;
>>
>> my @sendmail_parameters = ('-i', @recipients);
>> my $raw_from = $sender;
>> @@ -1556,6 +1557,7 @@ sub send_message {
>> $confirm = 'never';
>> $needs_separator = 1;
>> }
>> + $prompt_separator = 1;
>> } else {
>> $needs_separator = 1;
>> }
>> @@ -1665,6 +1667,7 @@ sub send_message {
>> $smtp->dataend() or die $smtp->message;
>> $smtp->code =~ /250|200/ or die sprintf(__("Failed to send %s\n"),
>> $subject).$smtp->message;
>> }
>> + print "\n" if ($prompt_separator);
>
> "prompt separator" sounds more like a separator that separates
> prompts, but that is not what is going on, no?
Yup, I already wanted to rename that variable, because its name
isn't really great, but I didn't do that because it went as such
in earlier versions of the patch. Will rename it in the v6.
> Do we even need that new varible in the first place? I am wondering
> if you can just do the print "\n" right where you assign to that
> variable.
The reason why the newline isn't emitted right away is because
sending the message takes some time, and if we print it right
away, there's an additional empty line staring at the user while
they wait for the message to be sent. If we emit the newline
a bit later, using that variable, it gets displayed together
with the printed message, making the displaying of the output
much smoother.
I already tried to describe that behavior in the patch description.
I'll try to improve that description in the v6.
> When $confirm is set to 'never', you have both $needs_separtor and
> $prompt_separator set. Would it mean you'd have two extra blank
> lines for that message?
Actually, there are no unneeded blank lines in that case, which
I've also tested before sending the patches. One of the blank
lines (the one for $needs_separator) goes after the patch messages,
and the other one (the one for $prompt_separator) goes after the
prompt, which is displayed before the patch messages.
> All these questions you should have been able to avoided with a bit
> more helpful explanation in the proposed log message, I hope.
I already tried to do that, but it seems it needed more detailed
explanations in the patch description. Will be improved in the v6.
>> if ($quiet) {
>> printf($dry_run ? __("Dry-Sent %s\n") : __("Sent %s\n"), $subject);
>> } else {
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/3] send-email: make it easy to discern the messages for each patch
2024-04-08 21:08 ` Junio C Hamano
@ 2024-04-09 3:37 ` Dragan Simic
0 siblings, 0 replies; 10+ messages in thread
From: Dragan Simic @ 2024-04-09 3:37 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
On 2024-04-08 23:08, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
>
>> ... 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 above feels a bit roundabout way to say "the logic is that we
> need to add a gap before showing the next message, if we did things
> that cause the smtp traces to be shown", but OK.
Yeah, the wording I used isn't perfect, but I think it's still
understandable. I'll see to possibly improve it in the v6.
>> 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.
>
> I do not see this paragraph is relevant to the target audience. It
> may be a good advice to give to a reader who attempts to solve the
> problem this patch solved themselves, botches the attempt and ends
> up with a code with the terminator semantics. But for other readers
> of "git log" and reviewers of the patch, "I did not make a silly
> mistake, and instead correctly chose to use the separator semantics"
> is not something worth boasting about.
Makes sense, will delete that paragraph in the v6.
>> 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.
>
> These are worth recording.
Thanks.
>> @@ -1554,7 +1554,10 @@ sub send_message {
>> exit(0);
>> } elsif (/^a/i) {
>> $confirm = 'never';
>> + $needs_separator = 1;
>> }
>> + } else {
>> + $needs_separator = 1;
>> }
>
> If you do not add this "else" clause to the outer "are we doing
> confirmation?" if statement, and instead just set $needs_separator
> *after* it, it would make it even more obvious what is going on.
> The codeflow would become
>
> sub send_message {
> do bunch of things that do not yet send e-mail
> and possibly return or die
>
> $needs_separator = 1;
>
> do things that cause the smtp exchange and trace
> to be emitted
> }
>
> That makes it obvious that the purpose of $needs_separator is to
> record the fact that "this" message has already been sent and we
> need to add a "gap" before attempting to send the "next" message.
Very good point, thanks! I've somehow managed to miss that while
iterating through a few code variants and the associated testing.
Will be improved in the v6.
> Other than the above points, very well done.
Thank you! :)
>> unshift (@sendmail_parameters, @smtp_server_options);
>> @@ -1576,7 +1579,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.")
>> }
>> @@ -1921,7 +1923,8 @@ sub pre_process_file {
>> sub process_file {
>> my ($t) = @_;
>>
>> - pre_process_file($t, $quiet);
>> + pre_process_file($t, $quiet);
>
> nice ;-)
It had to be fixed, IMHO. :)
>> + print "\n" if ($needs_separator);
>>
>> my $message_was_sent = send_message();
>> if ($message_was_sent == -1) {
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages
2024-04-09 3:25 ` Dragan Simic
@ 2024-04-10 3:53 ` Dragan Simic
2024-04-10 6:07 ` Dragan Simic
0 siblings, 1 reply; 10+ messages in thread
From: Dragan Simic @ 2024-04-10 3:53 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
On 2024-04-09 05:25, Dragan Simic wrote:
> On 2024-04-08 23:21, Junio C Hamano wrote:
>> When $confirm is set to 'never', you have both $needs_separtor and
>> $prompt_separator set. Would it mean you'd have two extra blank
>> lines for that message?
>
> Actually, there are no unneeded blank lines in that case, which
> I've also tested before sending the patches. One of the blank
> lines (the one for $needs_separator) goes after the patch messages,
> and the other one (the one for $prompt_separator) goes after the
> prompt, which is displayed before the patch messages.
Actually, there's a much, _much_ simpler solution for everything,
which also works as expected when running "git send-email --quiet"
with sendmail.confirm set to "auto" or "never".
I need to test it a bit more, and I'll send the updated patches.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages
2024-04-10 3:53 ` Dragan Simic
@ 2024-04-10 6:07 ` Dragan Simic
0 siblings, 0 replies; 10+ messages in thread
From: Dragan Simic @ 2024-04-10 6:07 UTC (permalink / raw
To: Junio C Hamano; +Cc: git, code
On 2024-04-10 05:53, Dragan Simic wrote:
> On 2024-04-09 05:25, Dragan Simic wrote:
>> On 2024-04-08 23:21, Junio C Hamano wrote:
>>> When $confirm is set to 'never', you have both $needs_separtor and
>>> $prompt_separator set. Would it mean you'd have two extra blank
>>> lines for that message?
>>
>> Actually, there are no unneeded blank lines in that case, which
>> I've also tested before sending the patches. One of the blank
>> lines (the one for $needs_separator) goes after the patch messages,
>> and the other one (the one for $prompt_separator) goes after the
>> prompt, which is displayed before the patch messages.
>
> Actually, there's a much, _much_ simpler solution for everything,
> which also works as expected when running "git send-email --quiet"
> with sendmail.confirm set to "auto" or "never".
>
> I need to test it a bit more, and I'll send the updated patches.
This new, simplified approach works well, but I've spotted some
more scenarios that also require addition of newlines. Though,
based on previous lessons, :) I'll leave that to the follow-up
patches.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-04-10 6:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-07 10:48 [PATCH v5 0/3] send-email: make produced outputs more readable Dragan Simic
2024-04-07 10:48 ` [PATCH v5 1/3] send-email: move newline character out of a translatable string Dragan Simic
2024-04-07 10:48 ` [PATCH v5 2/3] send-email: make it easy to discern the messages for each patch Dragan Simic
2024-04-08 21:08 ` Junio C Hamano
2024-04-09 3:37 ` Dragan Simic
2024-04-07 10:48 ` [PATCH v5 3/3] send-email: separate the confirmation prompts from the messages Dragan Simic
2024-04-08 21:21 ` Junio C Hamano
2024-04-09 3:25 ` Dragan Simic
2024-04-10 3:53 ` Dragan Simic
2024-04-10 6:07 ` 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.