From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <t.lamprecht@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 6256260231
for <pve-devel@lists.proxmox.com>; Mon, 16 Nov 2020 18:13:10 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 518A48F76
for <pve-devel@lists.proxmox.com>; Mon, 16 Nov 2020 18:12:40 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[212.186.127.180])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS id 2B3188F43
for <pve-devel@lists.proxmox.com>; Mon, 16 Nov 2020 18:12:39 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E3F1843725
for <pve-devel@lists.proxmox.com>; Mon, 16 Nov 2020 18:12:38 +0100 (CET)
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
=?UTF-8?Q?Dominic_J=c3=a4ger?= <d.jaeger@proxmox.com>
References: <20201116101457.61771-1-d.jaeger@proxmox.com>
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
Message-ID: <16bc7f15-a092-f021-a710-a1900baf41f6@proxmox.com>
Date: Mon, 16 Nov 2020 18:12:37 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:83.0) Gecko/20100101
Thunderbird/83.0
MIME-Version: 1.0
In-Reply-To: <20201116101457.61771-1-d.jaeger@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.093 Adjusted score from AWL reputation of From: address
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -0.001 Looks like a legit reply (A)
RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/,
medium trust
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [vzdump.pm]
Subject: Re: [pve-devel] [PATCH manager] vzdump mail: Refactor text part
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Mon, 16 Nov 2020 17:13:10 -0000
On 16.11.20 11:14, Dominic J=C3=A4ger wrote:
> Less lines, less nesting, less duplicate code, less magic numbers.
>=20
> Signed-off-by: Dominic J=C3=A4ger <d.jaeger@proxmox.com>
> ---
> Depends on the previous patch for #3136.
>=20
> Admittedly, using a sub and ternary operator might be seen as more comp=
lex than
> just the if-statements. So I'm not mad if we leave it as it is.
we could avoid the sub and the if by using the $size_conversion directly =
in
the format string?
Or move out even more than just the format string generation out, so that=
it
becomes a simple loop calling
$text .=3D render_task_plain($vmid, $task);
or something similar to that.
> But comparing the (first) parameters of 3 sprintf is not that much fun =
either.
>=20
> PVE/VZDump.pm | 27 ++++++++++-----------------
> 1 file changed, 10 insertions(+), 17 deletions(-)
>=20
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 1096d594..aaf8a84d 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -258,24 +258,17 @@ sub sendmail {
> =20
> # text part
> my $text =3D $err ? "$err\n\n" : '';
> - $text .=3D sprintf ("%-10s %-20s %-6s %10s %10s %s\n", qw(VMID NA=
ME STATUS TIME SIZE FILENAME));
> + my $namelength =3D 20;
> + my $conversions =3D sub { "%-10s %-${namelength}s %-6s %10s $_[0] =
%s\n" };
if, I'd call this $fmt but actually I'd rather avoid the sub completely (=
see above)
> + $text .=3D sprintf ($conversions->("%10s"), qw(VMID NAME STATUS TI=
ME SIZE FILENAME));
> foreach my $task (@$tasklist) {
> - my $vmid =3D $task->{vmid};
> - if ($task->{state} eq 'ok') {
> -
> - $text .=3D sprintf ("%-10s %-20s %-6s %10s %10s %s\n", $vmid,
> - substr($task->{hostname}, 0, 20),
> - $task->{state},
> - format_time($task->{backuptime}),
> - format_size ($task->{size}),
> - $task->{target});
> - } else {
> - $text .=3D sprintf ("%-10s %-20s %-6s %10s %8.2fMB %s\n", $vmid,=
> - substr($task->{hostname}, 0, 20),
> - $task->{state},
> - format_time($task->{backuptime}),
> - 0, '-');
> - }
> + my $name =3D substr($task->{hostname}, 0, $namelength);
> + my $successful =3D $task->{state} eq 'ok';
> + my $size =3D $successful ? format_size ($task->{size}) : 0;
> + my $filename =3D $successful ? $task->{target} : '-';
> + my $size_conversion =3D $successful ? "%10s": "%8.2fMB";
> + $text .=3D sprintf($conversions->($size_conversion), $task->{vmid}, $=
name,
> + $task->{state}, format_time($task->{backuptime}), $size, $filenam=
e);
> }
> =20
> my $text_log_part;
>=20