From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 ; 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 ; Mon, 16 Nov 2020 18:12:38 +0100 (CET) To: Proxmox VE development discussion , =?UTF-8?Q?Dominic_J=c3=a4ger?= References: <20201116101457.61771-1-d.jaeger@proxmox.com> From: Thomas Lamprecht 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 > --- > 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