[libcamera-devel] [PATCH 05/30] cam: options: Add optionName() function to Option structure

[Kieran Bingham]

On 07/07/2021 03:19, Laurent Pinchart wrote:
> Add an Option::optionName() function that returns a string describing
> the option name, with leading dashes. As a result,
> OptionsParser::parseValueError() function becomes a single-line function
> and can be inlined in its caller.
> 

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/cam/options.cpp | 32 ++++++++++++++++++--------------
>  src/cam/options.h   |  2 --
>  2 files changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index da218c4644d3..2f71860d0466 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -100,6 +100,7 @@ struct Option {
>  	bool hasShortOption() const { return isalnum(opt); }
>  	bool hasLongOption() const { return name != nullptr; }
>  	const char *typeName() const;
> +	std::string optionName() const;
>  };
>  
>  /**
> @@ -125,6 +126,20 @@ const char *Option::typeName() const
>  	return "unknown";
>  }
>  
> +/**
> + * \brief Retrieve a string describing the option name, with leading dashes
> + * \return A string describing the option name, as a long option identifier
> + * (double dash) if the option has a name, or a short option identifier (single
> + * dash) otherwise
> + */
> +std::string Option::optionName() const
> +{
> +	if (name)
> +		return "--" + std::string(name);
> +	else
> +		return "-" + std::string(1, opt);
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * OptionBase<T>
>   */
> @@ -866,7 +881,9 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
>  
>  		const Option &option = *optionsMap_[c];
>  		if (!options.parseValue(c, option, optarg)) {
> -			parseValueError(option);
> +			std::cerr << "Can't parse " << option.typeName()
> +				  << " argument for option " << option.optionName()
> +				  << std::endl;
>  			usage();
>  			return options;
>  		}
> @@ -953,16 +970,3 @@ void OptionsParser::usage()
>  			option.keyValueParser->usage(indent);
>  	}
>  }
> -
> -void OptionsParser::parseValueError(const Option &option)
> -{
> -	std::string optionName;
> -
> -	if (option.name)
> -		optionName = "--" + std::string(option.name);
> -	else
> -		optionName = "-" + std::string(1, option.opt);
> -
> -	std::cerr << "Can't parse " << option.typeName()
> -		  << " argument for option " << optionName << std::endl;
> -}
> diff --git a/src/cam/options.h b/src/cam/options.h
> index a3b18bb6c4ea..0b8bb172d184 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -95,8 +95,6 @@ public:
>  	void usage();
>  
>  private:
> -	void parseValueError(const Option &option);
> -
>  	std::list<Option> options_;
>  	std::map<unsigned int, Option *> optionsMap_;
>  };
>