[pve-devel] [PATCH common] cli_handler: don't clobber existing options

[Stoiko Ivanov]

Currently there is, since most commands don't use the central helpers yet.
Agreed there shouldn't be one. The only option where the output prefix
seemed like a good solution IMO is 'format'. The others were treated
likewise, since this seemed like a robust way to circumvent the problem
with having a option with the same name in an API-call (however I guess if
we ever have an option 'human-readable' in an API-call, the surprise factor
of magically getting an 'output-human-readable' is not nice either).

Maybe it would be better to only do the 'output-' prefix for format, and
simply omit deleting the others (quiet, noborder, noheader,
human-readable), if they exist in the parameter definition of the called
method? (A quick overview says, apart from pve-client, we only have 'quiet'
in 2 places in pve-manager (vzdump and API2::APT).
Would be grateful for other ideas/input/opinions (and provide updated
patches).


On Tue, Jul 17, 2018 at 08:27:55AM +0200, Dietmar Maurer wrote:
> Is there a 'semantic' difference between 'quiet' and 'output-quiet'?
> 
> > On July 16, 2018 at 2:44 PM Stoiko Ivanov <s.ivanov at proxmox.com> wrote:
> > 
> > 
> > If an API call has one of the standard_output_options defined (e.g. format for
> > diskimages, quiet for vzdump), it would get overridden by the
> > standard_output_option. Instead add an 'output-' prefix in that case.
> > 
> > Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> > ---
> > This fixed the problem of the duplicate notifications from scheduled backups
> > in my tests.
> > A follow-up of adding the output-options to the usage messages and to
> > bash-completion is in the workings.
> > Additionally I would only add the options if an output-sub is defined for a
> > command.
> > 
> > src/PVE/RESTHandler.pm | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
> > index af9024e..35e46eb 100644
> > --- a/src/PVE/RESTHandler.pm
> > +++ b/src/PVE/RESTHandler.pm
> > @@ -777,6 +777,12 @@ sub add_standard_output_parameters {
> >      my ($org_schema) = @_;
> >  
> >      my $schema = { %$org_schema };
> > +
> > +    foreach my $prop (keys %{$schema->{properties}}) {
> > +	if (exists($standard_output_options->{$prop})) {
> > +	    $standard_output_options->{'output-'.$prop} = delete
> > $standard_output_options->{$prop};
> > +	}
> > +    }
> >      $schema->{properties} = { %{$schema->{properties}},
> > %$standard_output_options };
> >  
> >      return $schema;
> > -- 
> > 2.11.0
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel at pve.proxmox.com
> > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel