[pve-devel] applied: [PATCH common] cli_handler: make standard options opt-in

[Wolfgang Bumiller]

applied

It makes definitely more sense to make this opt-in for now rather than
patching up the names and bash completion with a heuristic based on
output functions, since we don't need/use the functionality yet in the
existing cli tools.

On Tue, Jul 17, 2018 at 04:19:31PM +0200, Thomas Lamprecht wrote:
> The new standard options overwrite some already existing options in
> already existing CLI tools, which is surely no good, e.g. vzdump
> already has an quiet option which now gets overwritten, thus any
> backup triggered by CRON always sents out an (additional) email.
> 
> Further, they may even only have an effect if the respective CLI
> command uses an CLIFormatter print* method to output things.
> 
> And for some commands, like 'pvecm status' they may even never make
> sense, as they exec an external program which doesn't honors nor gets
> those std options...
> Also some internal commands, like 'qm mtunnel' will never use it, as
> probably even "normal" commands, as it may simply not make sense for
> every command..
> 
> So make it opt-in for now, any CLI command can set std-output-opts to
> get them.
> 
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> 
> Details (option name, variables, ...) may be argued.
> Until we discussed and agreed what direction we want to go with this I highly
> recommend to make such things opt-in, tampering with all (CLI) schemas
> unconditionally is dangerous... (and yeah I know that it's my fault as I pushed
> the commit doing this unconditionally)
> 
>  src/PVE/RESTHandler.pm | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
> index af9024e..8be1503 100644
> --- a/src/PVE/RESTHandler.pm
> +++ b/src/PVE/RESTHandler.pm
> @@ -788,15 +788,19 @@ sub cli_handler {
>      my $info = $self->map_method_by_name($name);
>      $options //= {};
>  
> +    my $add_stdopts = $options->{'std-output-opts'};
> +
>      my $res;
>      eval {
>  	my $param_map = {};
>  	$param_map = $compute_param_mapping_hash->($param_cb->($name)) if $param_cb;
> -	my $schema = add_standard_output_parameters($info->{parameters});
> +	my $schema = $add_stdopts ? add_standard_output_parameters($info->{parameters}) : $info->{properties} ;
>  	my $param = PVE::JSONSchema::get_options($schema, $args, $arg_param, $fixed_param, $param_map);
>  
> -	foreach my $opt (keys %$standard_output_options) {
> -	    $options->{$opt} = delete $param->{$opt} if defined($param->{$opt});
> +	if ($add_stdopts) {
> +	    foreach my $opt (keys %$standard_output_options) {
> +		$options->{$opt} = delete $param->{$opt} if defined($param->{$opt});
> +	    }
>  	}
>  
>  	if (defined($param_map)) {
> -- 
> 2.18.0