[pve-devel] [PATCH common v3 2/3] add print_text_table, print_entry to CLIHandler

[Thomas Lamprecht]

On 6/12/18 12:41 PM, Stoiko Ivanov wrote:
> These two function could serve as a generic output sub for various CLI
> utilities
>  * print_text_table prints an array of objects in a tabular fashion,
>    the formating is passed as an array containg hashes with titles, maximal
>    lengths and default values. This way we can stay extensible, by adding other
>    keys to the formatting options
>  * print_entry prints out a single entry, handling array-refs as properties
> 

looks quite good now, few comments inline

> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
>  src/PVE/CLIHandler.pm | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
> index 316d29d..f87720c 100644
> --- a/src/PVE/CLIHandler.pm
> +++ b/src/PVE/CLIHandler.pm
> @@ -396,6 +396,69 @@ my $print_bash_completion = sub {
>      &$print_result(@option_list);
>  };
>  
> +# prints a formatted table with a title row.
> +# $formatopts is an array of hashes, with the following keys:
> +# 'key' - the key in the data-objects to use for this column
> +# 'title' - the title to print above the column, defaults to 'key' - always gets printed in full
> +# 'cutoff' - the maximal length of the data, overlong values will be truncated> +# 'default' - an optional default value for the column
> +# the last column always gets printed in full
> +
> +sub print_text_table {
> +    my ($formatopts, $data) = @_;
> +    my ($formatstring, @keys, @titles, %cutoffs, %defaults, $last_col);
> +
> +    $last_col = $formatopts->[$#{$formatopts}];
> +    foreach my $col ( @$formatopts ) {
> +	my ($key, $title, $cutoff, $default) = @$col{ qw(key title cutoff default)};
> +
> +	push @keys, $key;
> +	push @titles, ($title // $key);

do: $title //= $key; before that, else $title may still be undef...

> +	$defaults{$key} = $default;
> +
> +	#calculate maximal print width and cutoff
> +	my $titlelen = length($title);
...here

> +
> +	my $longest = $titlelen;
> +	foreach my $entry (@$data) {
> +	    my $len = length($entry->{$key}) // 0;
> +	    $longest = $len if $len > $longest;
> +	}
> +
> +	if (defined($cutoff)){
> +	    $cutoff = $cutoff < $longest ? $cutoff : $longest;
> +	} else {
> +	    $cutoff = $longest;
> +	}
> +	$cutoffs{$key} = $cutoff;

$cutoffs{$key} = defined($cutoff) && $cutoff < $longest ? $cutoff : $longest;

could be used instead of above 6 lines, defined-ness check optional as
cutoff==0 makes not much sense, but having it hedges against caller bugs

> +
> +	my $printalign = $cutoff > $titlelen ? '-' : '';
> +	if ($col == $last_col) {
> +	    $formatstring .= "%$printalign$titlelen"."s\n";

FYI, you can use "${titlelen}s" if you need to concat variables and
strings without white space boundary in-between, e.g.:

$formatstring .= "%${printalign}$[titlelen}s\n";

> +	} else {
> +	    $formatstring .= "%$printalign$cutoff".'s ';
> +	}
> +    }
> +
> +    printf $formatstring, @titles;
> +
> +    foreach my $entry (sort { $a->{$keys[0]} cmp $b->{$keys[0]} } @$data){
> +        printf $formatstring, map { substr(($entry->{$_} // $defaults{$_}), 0 , $cutoffs{$_}) } @keys;
> +    }
> +}
> +
> +sub print_entry {
> +    my $entry = shift;
> +    #TODO: handle objects/hashes as well
> +    foreach my $item (sort keys %$entry) {
> +	if (ref($entry->{$item}) eq 'ARRAY'){
> +	    printf "%s: [ %s ]\n", $item, join(", ", @{$entry->{$item}});
> +	} else {
> +	    printf "%s: %s\n", $item, $entry->{$item};
> +	}
> +    }
> +}
> +
>  sub verify_api {
>      my ($class) = @_;
>  
>