[libcamera-devel] [PATCH 3/3] cam: options: Add support for repeatable options

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2019-03-26 08:28:21 +0200, Laurent Pinchart wrote:
> > -	values_[opt] = value;
> > +	if (option.array)
> > +		values_[opt].add(value);
> > +	else
> > +		values_[opt] = value;
> 
> I would be tempted to overload the [] operator for values_ to just do
> the right thing depending on the type, but I think it would be bother
> overkill and confusing. Still tempting to play with the available toys
> though :-)

Playing with new toys are always fun :-) In this particular instance I 
think overloading [] would just add to the confusion.

> > diff --git a/src/cam/options.h b/src/cam/options.h
> > index 6a887416c0070c41..1dac15ea90f2ffd2 100644
> > --- a/src/cam/options.h
> > +++ b/src/cam/options.h
> > @@ -36,6 +36,7 @@ struct Option {
> >  	const char *argumentName;
> >  	const char *help;
> >  	KeyValueParser *keyValueParser;
> > +	bool array;
> 
> Maybe isArray ?

Better, will switch in next version.

> 
> I think the rest is fine. How does the help output look like ?

$ ./cam --help
Options:
  -c, --camera camera                                   Specify which camera to operate on
  -C, --capture                                         Capture until interrupted by user
  -F, --file[=filename]                                 Write captured frames to disk
                                                        The first '#' character in the file name is expanded to the frame sequence number.
                                                        The default file name is 'frame-#.bin'.
  -f, --format key=value[,key=value,...] ...            Set format of the camera's first stream
                                                        height=integer          Height in pixels
                                                        id=integer              ID of stream
                                                        pixelformat=integer     Pixel format
                                                        width=integer           Width in pixels
  -h, --help                                            Display this help message
  -l, --list                                            List all cameras

I let you comment on if you like the ... to indicate that an option can 
be repeated and post v2 after that.

-- 
Regards,
Niklas Söderlund