Re: [PATCH RESEND] send-email: make produced outputs more readable
- From
- Dragan Simic <dsimic@manjaro.org>
- Date
- Apr 4, 2024, 19:40 UTC
- Message-ID
- <44ed7af52e2968993d9b2b99faec1f64@manjaro.org>
- In-Reply-To
- <xmqqy19tylrm.fsf@gitster.g>
Hello Junio,
On 2024-04-04 21:23, Junio C Hamano wrote:
Show 21 quoted lines
> Dragan Simic <dsimic@manjaro.org> writes: > >> Notes: >> * send-email: make produced outputs more readable by separating >> the result statuses from the subsequent patch outputs >> >> This is a resubmission of the patch I submitted about a week and a >> half >> ago. [1] 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. > > I tried to cram a bit more information than "output more readable" > that lacks in what way the result is easier to read. > > send-email: make boundaries between messages easier to spot > > perhaps?
Looking good to me, will use that as the patch subject in v2.
Show 52 quoted lines
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 821b2b3a135a..62505ab2707c 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -1576,7 +1576,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.")
>> }
>> @@ -1686,9 +1685,9 @@ 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), "\n\n";
>> } else {
>> - print __("Result: OK\n");
>> + print __("Result: OK\n\n");
>> }
>
> It would be nicer to instead add a single separate
>
> print "\n";
>
> after these if/else alternatives, without touching the existing
> message lines, I would think. That way, existing message
> translations do not have to change.
>
> If we were to change the translatable string anyway, it would be
> even better to remove the newline from the translatable part of the
> message, rendering the thing to:
>
> if ($smtp) {
> print __("Result: "), ..., ($smtp->message =~ /.../);
> } else {
> print __("Result: OK");
> }
> print "\n\n";
>
> Strictly speaking, that is an orthogonal clean-up, so it may have to
> make it into two patch series, one for preliminary clean-up "to
> excise terminating newline out of translatable strings" patch that
> adds a separate print that adds a single newline, plus the "make it
> easier to spot where a message ends and another one starts" patch
> that makes the new print statement that adds a single newline to
> instead add two. In a patch as simple as this one, however, I think
> killing two birds with a stone, i.e., directly go to the "if we were
> to change the translatable string anyway" final shape in a single
> patch, would be fine.Thank you very much for such a detailed review! I also think that putting everything into a single patch is a better option, because the changes are rather small. Will do that in v2.