[libcamera-devel] [PATCH 5/6] cam: options: add a key=value parser
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jan 31 11:53:31 CET 2019
Hi Niklas,
Thank you for the patch.
On Mon, Jan 28, 2019 at 01:41:08AM +0100, Niklas Söderlund wrote:
> Some options passed to the cam utility needs to be complex and specify a
s/needs/need/
> list of key=value pairs, add a new parser to deal white these options.
s/, add/. Add/
s/white/with/
> The new parser is integrated into the existing OptionsParser and the cam
> application can fully describe all its options in one location and
> perform full parsing of all arguments in one go. The new key=value
> parsers also integrates itself with the usage() printing of the
> OptionsParser.
>
> The new parser can also be used on it's own to parse key=value pairs
s/it's/its/
> from different data sources then the command line options.
s/then/than/
I would have split this patch in two, with one patch to add the
key-value parser, and another patch to integrate it in the options
parser.
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/cam/options.cpp | 159 +++++++++++++++++++++++++++++++++++++++++++-
> src/cam/options.h | 35 ++++++++++
> 2 files changed, 193 insertions(+), 1 deletion(-)
>
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index d3bff1cd897a5cfb..f63fd3b495e29986 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -30,11 +30,22 @@ bool OptionsParser::addOption(int opt, const char *help, const char *name,
> if (optionsMap_.find(opt) != optionsMap_.end())
> return false;
>
> - optionsMap_[opt] = Option({ opt, name, argument, argumentName, help });
> + optionsMap_[opt] = Option({ opt, name, argument, argumentName, help,
> + nullptr });
>
> return true;
> }
>
> +void OptionsParser::addOption(int opt, const char *help, KeyValueParser &parser,
> + const char *name)
> +{
> + if (!addOption(opt, help, name, ArgumentRequired,
> + "key=value[,key=value,...]"))
> + return;
> +
> + optionsMap_[opt].keyValueParser = &parser;
> +}
> +
> OptionsParser::Options OptionsParser::parse(int argc, char **argv)
> {
> OptionsParser::Options options;
> @@ -103,6 +114,16 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
> }
>
> options.values_[c] = optarg ? optarg : "";
> +
> + /* Parse argument as key=values if needed. */
> + if (optionsMap_[c].keyValueParser) {
> + options.keyValues_[c] = optionsMap_[c].keyValueParser->parse(options.values_[c].c_str());
> + if (!options.keyValues_[c].valid()) {
> + usage();
> + options.clear();
> + break;
> + }
> + }
> }
>
> return options;
> @@ -169,5 +190,141 @@ void OptionsParser::usage()
> std::cerr << help << std::endl;
> }
> }
> +
> + if (option.keyValueParser)
> + option.keyValueParser->usage(indent);
> + }
> +}
> +
> +bool OptionsParser::Options::isKeyValue(int opt)
> +{
> + return keyValues_.find(opt) != keyValues_.end();
> +}
> +
> +KeyValueParser::Options OptionsParser::Options::keyValues(int opt)
> +{
> + return keyValues_.find(opt)->second;
> +}
> +
> +void KeyValueParser::addOption(const char *name,
> + const char *help,
> + OptionArgument argument,
> + const char *argumentName)
> +{
> + if (!name)
> + return;
> + if (!help || help[0] == '\0')
> + return;
> + if (argument != ArgumentNone && !argumentName)
> + return;
> +
> + /* Reject duplicate options. */
> + if (optionsMap_.find(name) != optionsMap_.end())
> + return;
> +
> + optionsMap_[name] = Option({ name, help, argument, argumentName });
> +}
> +
> +KeyValueParser::Options KeyValueParser::parse(const char *arguments)
> +{
> + Options options;
> +
> + for (const char *pair = arguments; *arguments != '\0'; pair = arguments) {
> + const char *comma = strchrnul(arguments, ',');
> + size_t len = comma - pair;
> +
> + /* Skip over the comma. */
> + arguments = *comma == ',' ? comma + 1 : comma;
> +
> + /* Skip to the next pair if the pair is empty. */
> + if (!len)
> + continue;
> +
> + std::string key;
> + std::string value;
> +
> + const char *separator = static_cast<const char *>(memchr(pair, '=', len));
> + if (!separator) {
> + key = std::string(pair, len);
> + value = "";
> + } else {
> + key = std::string(pair, separator - pair);
> + value = std::string(separator + 1, comma - separator - 1);
> + }
> +
> + /* The key is mandatory, the value might be optional. */
> + if (key.empty())
> + continue;
> +
> + if (optionsMap_.find(key) == optionsMap_.end()) {
> + std::cerr << "Invalid option " << key << std::endl;
> + options.clear();
> + break;
> + }
> +
> + if (value == "" && optionsMap_[key].argument == ArgumentRequired) {
> + std::cerr << "Missing argument for option " << key << std::endl;
> + options.clear();
> + break;
> + }
> +
> + if (value != "" && optionsMap_[key].argument == ArgumentNone) {
> + std::cerr << "Argument specified for option " << key << std::endl;
> + options.clear();
> + break;
> + }
How about
OptionArgument arg = optionsMap_[key].argument;
if (value.empty() && arg == ArgumentRequired) {
std::cerr << "Option " << key << " requires an argument" << std::endl;
options.clear();
break;
} else if (!value.empty() && arg == ArgumentNone) {
std::cerr << "Option " << key << " doesn't take an argument" << std::endl;
options.clear();
break;
}
> + options.values_[key] = value;
> + }
> +
> + return options;
> +}
> +
> +void KeyValueParser::usage(int indent)
> +{
> + unsigned int space = 0;
> +
> + for (auto const &iter : optionsMap_) {
> + const Option &option = iter.second;
> + unsigned int length = 14;
> + if (option.argument != ArgumentNone)
> + length += 1 + strlen(option.argumentName);
> + if (option.argument == ArgumentOptional)
> + length += 2;
> +
> + if (length > space)
> + space = length;
> + }
> +
> + space = (space + 7) / 8 * 8;
> +
> + for (auto const &iter : optionsMap_) {
> + const Option &option = iter.second;
> +
No need for a blank line.
> + std::string argument = option.name;
> +
> + if (option.argument != ArgumentNone) {
> + if (option.argument == ArgumentOptional)
> + argument += "[=";
> + else
> + argument += "=";
> + argument += option.argumentName;
> + if (option.argument == ArgumentOptional)
> + argument += "]";
> + }
> +
> + std::cerr << std::setw(indent) << std::right << " "
> + << std::setw(space) << std::left << argument;
> +
> + for (const char *help = option.help, *end = help; end;) {
> + end = strchr(help, '\n');
> + if (end) {
> + std::cerr << std::string(help, end - help + 1);
> + std::cerr << std::setw(indent + space) << " ";
> + help = end + 1;
> + } else {
> + std::cerr << help << std::endl;
> + }
> + }
> }
> }
> diff --git a/src/cam/options.h b/src/cam/options.h
> index cb7286a0a8005579..91a78406a601d737 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -27,20 +27,54 @@ public:
>
> private:
> friend class OptionsParser;
> + friend class KeyValueParser;
I'd sort there alphabetically.
> std::map<T, std::string> values_;
> void clear() { values_.clear(); };
> };
>
> +class KeyValueParser
> +{
> +public:
> + class Options : public OptionsBase<std::string>
> + {
> + };
> +
> + void addOption(const char *name, const char *help,
> + OptionArgument argument = ArgumentNone,
> + const char *argumentName = nullptr);
> +
> + Options parse(const char *arguments);
> + void usage(int indent);
> +
> +private:
> + struct Option {
> + const char *name;
> + const char *help;
> + OptionArgument argument;
> + const char *argumentName;
> + };
> +
> + std::map<std::string, Option> optionsMap_;
Maybe just options_ ?
> +};
> +
> class OptionsParser
> {
> public:
> class Options : public OptionsBase<int>
> {
> + public:
> + bool isKeyValue(int opt);
> + KeyValueParser::Options keyValues(int opt);
> + private:
> + friend class OptionsParser;
> + std::map<unsigned int, KeyValueParser::Options> keyValues_;
> };
>
> bool addOption(int opt, const char *help, const char *name = nullptr,
> OptionArgument argument = ArgumentNone,
> const char *argumentName = nullptr);
> + void addOption(int opt, const char *help, KeyValueParser &parser,
> + const char *name = nullptr);
Shouldn't this return a bool like the other addOption() ?
> Options parse(int argc, char *argv[]);
> void usage();
> @@ -52,6 +86,7 @@ private:
> OptionArgument argument;
> const char *argumentName;
> const char *help;
> + KeyValueParser *keyValueParser;
>
> bool hasShortOption() const { return isalnum(opt); }
> bool hasLongOption() const { return name != nullptr; }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list