[pve-devel] [RFC pve-common v2 2/2] new schema property addStandardCliParameters

[Thomas Lamprecht]

Am 07/24/2018 um 11:42 AM schrieb Dietmar Maurer:
> Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>
> ---
>   src/PVE/CLIHandler.pm  | 8 +++++++-
>   src/PVE/JSONSchema.pm  | 5 +++++
>   src/PVE/RESTHandler.pm | 5 +++--
>   3 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/CLIHandler.pm b/src/PVE/CLIHandler.pm
> index 7145707..ef641e9 100644
> --- a/src/PVE/CLIHandler.pm
> +++ b/src/PVE/CLIHandler.pm
> @@ -406,7 +406,13 @@ my $print_bash_completion = sub {
>   
>       my $info = $class->map_method_by_name($name);
>   
> -    my $prop = $info->{parameters}->{properties};
> +    my $prop = { %{$info->{parameters}->{properties}} }; # copy
> +    if ($info->{addStandardCliParameters}) {
> +	my $std_cli_opts = $PVE::RESTHandler::standard_output_options;
> +	foreach my $opt (keys %$std_cli_opts) {
> +	    $prop->{$opt} = $std_cli_opts->{$opt};
> +	}
> +    }
>   
>       my $print_parameter_completion = sub {
>   	my ($pname) = @_;
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index c9a9b1e..ec74e7c 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -1280,6 +1280,11 @@ my $method_schema = {
>   	    type =>  'string',
>   	    description => "path for URL matching (uri template)",
>   	},
> +        addStandardCliParameters => {
> +            type => 'boolean',
> +            description => "Automatically add standard parameters for CLI command (--quiet, --output-format, ...).",
> +            optional => 1,
> +        },

Could one want to add only some standard options?
I.e., would be an array with enums more suited?
(with having an 'all' alias). Just a thought

>           fragmentDelimiter => {
>               type => 'string',
>   	    description => "A ways to override the default fragment delimiter '/'. This onyl works on a whole sub-class. You can set this to the empty string to match the whole rest of the URI.",
> diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
> index 1d6b206..951e7d7 100644
> --- a/src/PVE/RESTHandler.pm
> +++ b/src/PVE/RESTHandler.pm
> @@ -220,7 +220,8 @@ sub validate_method_schemas {
>   
>   	foreach my $info (@$ma) {
>   	    PVE::JSONSchema::validate_method_info($info);
> -	    if ($info->{parameters} && (my $pdef = $info->{parameters}->{properties})) {
> +	    if ($info->{addStandardCliParameters} && $info->{parameters} &&
> +		(my $pdef = $info->{parameters}->{properties})) {
>   		foreach my $opt (keys %$standard_output_options) {
>   		    if (defined($pdef->{$opt})) {
>   			die "detected overwriten standard CLI parameter '$opt' in ${class}->$info->{name}\n";
> @@ -795,7 +796,7 @@ sub cli_handler {
>       my $info = $self->map_method_by_name($name);
>       $options //= {};
>   
> -    my $add_stdopts = $options->{'std-output-opts'};
> +    my $add_stdopts = $info->{addStandardCliParameters};

IMO, this is the responsibility of the CLI cmddef, not the API schema
pointed to from the cmddef. We can have multiple CLI commands pointing 
to the same API path, with different output transformations for example,
where on one command it may make sense but on the other not.

That's why I put the workaround to turn this intrusive behavior of 
inserting - possibly overwriting - CLI commands options off in options 
not in the schema.

>   
>       my $res;
>       eval {
>