[libcamera-devel] [PATCH] cam: options: optional arguments needs to be specified as --foo=bar

[Laurent Pinchart]

Hi Niklas,

On Fri, Jan 25, 2019 at 04:49:36PM +0100, Niklas Söderlund wrote:
> On 2019-01-25 16:44:41 +0200, Laurent Pinchart wrote:
> > On Fri, Jan 25, 2019 at 12:23:25PM +0100, Niklas Söderlund wrote:
> >> On 2019-01-25 13:01:34 +0200, Laurent Pinchart wrote:
> >>> On Fri, Jan 25, 2019 at 12:23:11AM +0100, Niklas Söderlund wrote:
> >>>> It's not state in the documentation but optional arguments needs to be
> >>>> specified using as '--foo=bar' instead of '--foo bar', otherwise the
> >>>> value is not propagated to optarg during argument parsing. Update the
> >>>> usage printing helper to reflect this requirement.
> >>>> 
> >>>> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >>>> ---
> >>>>  src/cam/options.cpp | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>> 
> >>>> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> >>>> index 82acff9bbeea476d..73d81d0bc0ec6d38 100644
> >>>> --- a/src/cam/options.cpp
> >>>> +++ b/src/cam/options.cpp
> >>>> @@ -143,7 +143,8 @@ void OptionsParser::usage()
> >>>>  		};
> >>>>  
> >>>>  		if (option.argument != ArgumentNone) {
> >>>> -			argument += std::string(" ");
> >>>> +			argument += option.argument == ArgumentOptional ?
> >>>> +				"=" : " ";
> >>>>  			if (option.argument == ArgumentOptional)
> >>>>  				argument += "[";
> >>>>  			argument += option.argumentName;
> >>> 
> >>> This will output
> >>> 
> >>> 	-f, --foo value
> >>> 
> >>> for mandatory arguments, and
> >>> 
> >>> 	-f, --foo=[value]
> >>> 
> >>> for optional arguments. If we want to print the =, shouldn't it be
> >>> --foo[=value] ?
> >> 
> >> It should of course be --foo[=value], will fix.
> >> 
> >>> And how should we handle the case where no long option
> >>> is available, with this patch -f=[value] would be printed, which isn't
> >>> correct I think.
> >> 
> >> Good point, for optional short arguments the syntax would be -fvalue.  
> >> Would it make sens to print both short and long syntax in the usage?
> >> 
> >>     -f[value], --foo[=value]
> >> 
> >> Let me know what you think and I send a v2.
> > 
> > That would make sense, but then we should also write
> > 
> >     -f value, --foo value
> > 
> > and we will lose the nice alignment of all long arguments :-( I'm not
> > sure what's best.
> 
> To move forward with this I would suggest,
> 
>     -f, --foo[=value]
> 
> Rational being that if you use -f you get the default behavior and if 
> you want to specify the optional argument you can use the long option. I 
> would however not go out of the way to make -fvalue fail argument 
> parsing. Then when we create a man page for the tool we can specify both 
> versions in detail.
> 
> Would this work for everyone?

Works for me.

-- 
Regards,

Laurent Pinchart