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

[Niklas Söderlund]

Hi Kieran,

On 2019-01-25 11:39:04 +0000, Kieran Bingham wrote:
> Heya,
> 
> On 25/01/2019 11:23, Niklas Söderlund wrote:
> > Hi Laurent,
> > 
> > On 2019-01-25 13:01:34 +0200, Laurent Pinchart wrote:
> >> Hi Niklas,
> >>
> >> 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.
> 
> Is it difficult to accept a space or an = separator on the arguments?

I think so, this is the default behavior of getopt_long() so without 
reimplementing that I think we are stuck with it. Maybe we can look into 
this once we are a bit further along and use the default behavior for 
now?

> 
> 
> 
> -- 
> Regards
> --
> Kieran

-- 
Regards,
Niklas Söderlund