[pve-devel] [PATCH common] PVE::CLIHandler::print_text_table: use eq for $sort_key

[Thomas Lamprecht]

On 6/26/18 3:38 PM, Stoiko Ivanov wrote:
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
> I like the codechanges in the 2 patches! - it's shorter and more clear than the
> previous version. - Thanks!
> 
> One tiny thing which bit me, while trying it with the pveum changes was, that
> providing a prop as $sort_key resulted in a warning - see this fixup.
> 
> One cosmetic change to before, is that getting rid of the special treatment for
> the last column now adds quite some whitespace in the end of lines, where one
> entry in data is very long, when compared to the others. (pveum role list was
> my example) resulting in some unexpected newlines. (I would provide a follow-up
> unless there is a strong preference for the current version).
> 
> We now also print one extra space before each newline - however, given that
> this output is more for visual representation, than for automated parsing I'm
> fine with it.
> 


your change looks good, for the to much line problem we could do something like:

----8<----
diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
index 1b9ff55..13c3f67 100644
--- a/src/PVE/CLIHandler.pm
+++ b/src/PVE/CLIHandler.pm
@@ -460,7 +460,10 @@ sub print_text_table {
     my $colopts = {};
     my $formatstring = '';

-    foreach my $prop (@$props_to_print) {
+    my $column_count = scalar(@$props_to_print);
+
+    for (my $i = 0; $i < $column_count; $i++) {
+       my $prop = $props_to_print->[$i];
        my $propinfo = $returnprops->{$prop};
        die "undefined property '$prop'" if !$propinfo;

@@ -486,10 +489,16 @@ sub print_text_table {
            cutoff => $cutoff,
        };

-       $formatstring .= '%'. (($cutoff > $titlelen) ? '-' : '') . $cutoff . 's ';
+       # skip alignment and cutoff on last column
+       last if $i == $column_count - 1;
+
+       $formatstring .= '%';
+       $formatstring .= '-' if $cutoff > $titlelen;
+       $formatstring .= "${cutoff}s ";
+
     }

-    $formatstring .= "\n";
+    $formatstring .= "%s\n";

     printf $formatstring, map { $colopts->{$_}->{title} } @$props_to_print;

--



> src/PVE/CLIHandler.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
> index d7d1ce4..db03544 100644
> --- a/src/PVE/CLIHandler.pm
> +++ b/src/PVE/CLIHandler.pm
> @@ -452,7 +452,7 @@ sub print_text_table {
>      my ($data, $returnprops, $props_to_print, $sort_key) = @_;
>  
>      my $autosort = 0;
> -    if (defined($sort_key) && ($sort_key == 1)) {
> +    if (defined($sort_key) && ($sort_key eq 1)) {
>  	$autosort = 1;
>  	$sort_key = undef;
>      }
>