From d0f6f93bb70a4e73cedd83c6f8ec0385a8deac45 Mon Sep 17 00:00:00 2001 From: Marcos Bento Date: Tue, 19 Sep 2023 16:43:12 +0100 Subject: [PATCH] ECKIT-620 Enable multi-value options in CmdArgs In some ECFLOW cases, command line options are expected to collect multiple values, as if these values where positional relative to the option e.g. --option /path/to/one /path/to/two ... --next. The logic mandates that values are collected until: 1) there are no more values to collect, either because the end of the list of values was reached or because the next option was found; or, 2) a mandatory+optional number of values has been collected. This implies changes to the way Options are collected by CmdArgs, and the current change enables each kind of option (SimpleOption, VectorOption, etc) to define how and how many argv entries are collected. --- src/eckit/option/CMakeLists.txt | 2 + src/eckit/option/CmdArgs.cc | 96 ++-- src/eckit/option/FactoryOption.cc | 24 +- src/eckit/option/FactoryOption.h | 14 +- src/eckit/option/MultiValueOption.cc | 89 ++++ src/eckit/option/MultiValueOption.h | 47 ++ src/eckit/option/Option.cc | 87 +--- src/eckit/option/Option.h | 77 ++- src/eckit/option/Separator.cc | 4 - src/eckit/option/Separator.h | 2 +- src/eckit/option/SimpleOption.cc | 95 ++-- src/eckit/option/SimpleOption.h | 15 +- src/eckit/option/VectorOption.cc | 35 +- src/eckit/option/VectorOption.h | 14 +- tests/option/CMakeLists.txt | 2 +- tests/option/eckit_test_option_cmdargs.cc | 547 ++++++++++++++++++++++ 16 files changed, 929 insertions(+), 221 deletions(-) create mode 100644 src/eckit/option/MultiValueOption.cc create mode 100644 src/eckit/option/MultiValueOption.h diff --git a/src/eckit/option/CMakeLists.txt b/src/eckit/option/CMakeLists.txt index 750a51d6e..96e4fc0ed 100644 --- a/src/eckit/option/CMakeLists.txt +++ b/src/eckit/option/CMakeLists.txt @@ -7,6 +7,8 @@ ecbuild_add_library( TARGET eckit_option TYPE SHARED SOURCES FactoryOption.cc FactoryOption.h + MultiValueOption.cc + MultiValueOption.h Option.cc Option.h Separator.cc diff --git a/src/eckit/option/CmdArgs.cc b/src/eckit/option/CmdArgs.cc index b332ac427..9668578bd 100644 --- a/src/eckit/option/CmdArgs.cc +++ b/src/eckit/option/CmdArgs.cc @@ -46,58 +46,71 @@ CmdArgs::CmdArgs(std::function usage, std::vector usage, int args_count, int minimum_args, bool throw_on_error) { - Main& ctx = Main::instance(); - tool_ = ctx.name(); - int argc = ctx.argc(); - bool error = false; - - std::map opts; - - for (std::vector::const_iterator j = options_.begin(); j != options_.end(); ++j) { - if ((*j)->active()) { - ASSERT(opts.find((*j)->name()) == opts.end()); - opts[(*j)->name()] = *j; - keys_.insert((*j)->name()); - (*j)->setDefault(*this); + const Main& ctx = Main::instance(); + tool_ = ctx.name(); + int argc = ctx.argc(); + bool error = false; + + using options_map_t = std::map; + + options_map_t opts; + + // Fill in 'keys_' and prepare options map + for (Option* j : options_) { + if (j->active()) { + ASSERT(opts.find(j->name()) == opts.end()); + keys_.insert(j->name()); + opts[j->name()] = j; + j->setDefault(*this); } } + const static std::string prefix = "--"; Tokenizer parse("="); + // Process all options/values in argv, letting each Option collect the necessary entries for (int i = 1; i < argc; ++i) { std::string a = ctx.argv(i); - if (a.size() > 2 && a[0] == '-' && a[1] == '-') { - std::vector v; - parse(a.substr(2), v); + if (a.substr(0, prefix.size()) == prefix) { // An Option 'a' is found (starts with '--')! - std::map::const_iterator j = opts.find(v[0]); - if (j != opts.end()) { + // The Option might be formatted as --=, so we tokenize and take the + std::vector tokens; + parse(a.substr(2), tokens); + + std::string name = tokens[0]; + tokens.erase(tokens.begin()); + + if (auto found = opts.find(name); found != opts.end()) { try { - if (v.size() == 1) { - (*j).second->set(*this); + const Option* option = found->second; + // Given the applicable Option, we prepare the argv tokens (including the ) + std::vector remaining; + remaining.reserve(tokens.size() + argc); + for (const auto& token : tokens) { + remaining.push_back(token); } - else { - std::vector::const_iterator b = v.begin(); - ++b; - std::vector::const_iterator e = v.end(); - (*j).second->set(StringTools::join("=", b, e), *this); + for (int j = i + 1; j < argc; ++j) { + remaining.push_back(ctx.argv(j)); } + // ... allow the Option to set itself based on all remaining argv tokens + size_t consumed = option->set(*this, std::begin(remaining), std::end(remaining)); + // ... and, disregard the number of consumed tokens. + i += static_cast(consumed - tokens.size()); } catch (UserError&) { - Log::info() << "Invalid value for option --" << v[0] << std::endl; + Log::info() << "Invalid value for option --" << tokens[0] << std::endl; error = true; } } else { - Log::info() << "Invalid option --" << v[0] << std::endl; + Log::info() << "Invalid option --" << name << std::endl; error = true; } } - else { + else { // Position argument 'a' is found! args_.push_back(a); } } - if (args_count >= 0) { if (args_.size() != size_t(args_count)) { Log::info() << tool_ << ": invalid argument count: expected " << args_count << ", got: " << args_.size() @@ -121,16 +134,16 @@ void CmdArgs::init(std::function usage, int args_count Log::info() << "Options are:" << std::endl; Log::info() << "===========:" << std::endl << std::endl; - for (std::vector::const_iterator j = options_.begin(); j != options_.end(); ++j) { - Log::info() << *(*j) << std::endl + for (const Option* j : options_) { + Log::info() << *j << std::endl << std::endl; } Log::info() << std::endl; } if (throw_on_error) { - for (std::vector::iterator j = options_.begin(); j != options_.end(); ++j) { - delete (*j); + for (const Option* j : options_) { + delete j; } throw UserError("An error occurred in argument parsing", Here()); } @@ -140,15 +153,15 @@ void CmdArgs::init(std::function usage, int args_count CmdArgs::~CmdArgs() { - for (std::vector::iterator j = options_.begin(); j != options_.end(); ++j) { - delete (*j); + for (const Option* j : options_) { + delete j; } } void CmdArgs::configure(Configured& c) const { - for (std::vector::const_iterator j = options_.begin(); j != options_.end(); ++j) { - (*j)->copy(*this, c); + for (const Option* j : options_) { + j->copy(*this, c); } } @@ -159,14 +172,6 @@ void CmdArgs::print(std::ostream& out) const { out << "]"; } -// const std::set& CmdArgs::keys() const { -// return keys_; -// } - -// const std::vector& CmdArgs::args() const { -// return args_; -// } - const std::string& CmdArgs::operator()(size_t i) const { ASSERT(i < args_.size()); return args_[i]; @@ -179,6 +184,7 @@ size_t CmdArgs::count() const { const std::string& CmdArgs::tool() const { return tool_; } + //---------------------------------------------------------------------------------------------------------------------- } // namespace eckit::option diff --git a/src/eckit/option/FactoryOption.cc b/src/eckit/option/FactoryOption.cc index d3d172a9d..5d0fc88fe 100644 --- a/src/eckit/option/FactoryOption.cc +++ b/src/eckit/option/FactoryOption.cc @@ -27,22 +27,34 @@ namespace option { template FactoryOption::FactoryOption(const std::string& name, const std::string& description) : - Option(name, description) {} + base_t(name, description) {} template -FactoryOption::~FactoryOption() {} +FactoryOption::FactoryOption(const std::string& name, const std::string& description, std::string default_value) : + base_t(name, description, std::move(default_value)) {} + +template +size_t FactoryOption::set(Configured& parametrisation, args_t::const_iterator begin, args_t::const_iterator end) const { + if (begin == end) { + throw UserError("No option value found for FactoryOption, where 1 was expected"); + } + set(*begin, parametrisation); + return 1; +} template void FactoryOption::set(const std::string& value, Configured& parametrisation) const { + set_value(value, parametrisation); +} + +template +void FactoryOption::set_value(const std::string& value, Configured& parametrisation) const { parametrisation.set(name_, value); } template void FactoryOption::copy(const Configuration& from, Configured& to) const { - std::string v; - if (from.get(name_, v)) { - to.set(name_, v); - } + Option::copy(name_, from, to); } template diff --git a/src/eckit/option/FactoryOption.h b/src/eckit/option/FactoryOption.h index 856e3c7ea..4e7cba3cd 100644 --- a/src/eckit/option/FactoryOption.h +++ b/src/eckit/option/FactoryOption.h @@ -27,19 +27,25 @@ namespace eckit::option { /// the validity of input received, and just returns the appropriate string template -class FactoryOption : public Option { +class FactoryOption : public BaseOption { public: + using base_t = BaseOption; + FactoryOption(const std::string& name, const std::string& description); + FactoryOption(const std::string& name, const std::string& description, std::string default_value); - ~FactoryOption() override; // Change to virtual if base class + ~FactoryOption() override = default; protected: - void print(std::ostream&) const override; // Change to virtual if base class + void print(std::ostream&) const override; private: using Option::set; - void set(const std::string& value, Configured&) const override; + size_t set(Configured&, args_t::const_iterator begin, args_t::const_iterator end) const override; + void set(const std::string& value, Configured&) const; + void set_value(const std::string& value, Configured&) const override; + void copy(const Configuration& from, Configured& to) const override; }; diff --git a/src/eckit/option/MultiValueOption.cc b/src/eckit/option/MultiValueOption.cc new file mode 100644 index 000000000..04974508b --- /dev/null +++ b/src/eckit/option/MultiValueOption.cc @@ -0,0 +1,89 @@ +/* + * (C) Copyright 1996- ECMWF. + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + * In applying this licence, ECMWF does not waive the privileges and immunities + * granted to it by virtue of its status as an intergovernmental organisation nor + * does it submit to any jurisdiction. + */ + +#include "eckit/option/MultiValueOption.h" + +#include +#include + +#include "eckit/config/Configuration.h" +#include "eckit/config/Configured.h" + +#include "eckit/exception/Exceptions.h" + +namespace eckit::option { + +MultiValueOption::MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values) : + MultiValueOption(name, description, n_mandatory_values, 0) {} + +MultiValueOption::MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values, + size_t n_optional_values) : + base_t(name, description), + n_mandatory_values_{n_mandatory_values}, + n_optional_values_{n_optional_values}, + values_{} { + ASSERT_MSG(n_mandatory_values >= 1, "At least 1 mandatory value is expected."); +} + +MultiValueOption::MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values, + const values_t& default_values) : + MultiValueOption(name, description, n_mandatory_values, 0, default_values) {} + +MultiValueOption::MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values, + size_t n_maximum_values, const values_t& default_values) : + base_t(name, description, default_values), + n_mandatory_values_{n_mandatory_values}, + n_optional_values_{n_maximum_values}, + values_{} { + ASSERT_MSG(n_mandatory_values >= 1, "At least 1 mandatory value is expected."); +} + +size_t MultiValueOption::set(Configured& parametrisation, args_t::const_iterator begin, + args_t::const_iterator end) const { + if (std::distance(begin, end) < n_mandatory_values_) { + throw UserError("Not enough option values found for MultiValueOption, where at least " + + std::to_string(n_mandatory_values_) + " were expected"); + } + + // Collect n_mandatory_values_ mandatory values from the range [begin, end) + values_t values; + std::copy_n(begin, n_mandatory_values_, std::back_inserter(values)); + begin = begin + n_mandatory_values_; + + // Collect up to n_optional_values from the range [(updated-)begin, end) + // - n.b. collection must stop when either end is reached or when an option (i.e. "--*") is found + for (size_t i = 0; i < n_optional_values_; ++i) { + if ((begin == end) || (begin->substr(0, 2) == "--")) { + break; + } + + values.push_back(*begin); + ++begin; + } + + // Store the collected values + set_value(values, parametrisation); + return values.size(); +} + +void MultiValueOption::set_value(const values_t& values, Configured& parametrisation) const { + parametrisation.set(this->name(), values); +} + +void MultiValueOption::copy(const Configuration& from, Configured& to) const { + Option::copy(this->name(), from, to); +} + +void MultiValueOption::print(std::ostream& out) const { + out << " --" << this->name() << "[=] [" << n_mandatory_values_ << " * values]([" << n_optional_values_ + << " * values]) (" << this->description() << ")"; +} + +} // namespace eckit::option diff --git a/src/eckit/option/MultiValueOption.h b/src/eckit/option/MultiValueOption.h new file mode 100644 index 000000000..61f4f4185 --- /dev/null +++ b/src/eckit/option/MultiValueOption.h @@ -0,0 +1,47 @@ +/* + * (C) Copyright 1996- ECMWF. + * + * This software is licensed under the terms of the Apache Licence Version 2.0 + * which can be obtained at http://www.apache.org/licenses/LICENSE-2.0. + * In applying this licence, ECMWF does not waive the privileges and immunities + * granted to it by virtue of its status as an intergovernmental organisation nor + * does it submit to any jurisdiction. + */ + +#ifndef eckit_option_MultiValueOption_H +#define eckit_option_MultiValueOption_H + +#include + +#include "eckit/option/Option.h" + +namespace eckit::option { + +class MultiValueOption : public BaseOption> { +public: + using base_t = BaseOption>; + using values_t = std::vector; + + MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values); + MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values, size_t n_optional_values); + MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values, const values_t& default_values_); + MultiValueOption(const std::string& name, const std::string& description, size_t n_mandatory_values, size_t n_maximum_values, const values_t& default_values_); + ~MultiValueOption() override = default; + + void print(std::ostream&) const override; + +private: + size_t set(Configured&, args_t::const_iterator begin, args_t::const_iterator end) const override; + + void set_value(const values_t& values, Configured&) const override; + + void copy(const Configuration& from, Configured& to) const override; + + size_t n_mandatory_values_; + size_t n_optional_values_; + values_t values_; +}; + +} // namespace eckit::option + +#endif diff --git a/src/eckit/option/Option.cc b/src/eckit/option/Option.cc index affd104cb..558604e4a 100644 --- a/src/eckit/option/Option.cc +++ b/src/eckit/option/Option.cc @@ -21,89 +21,12 @@ namespace eckit::option { Option::Option(const std::string& name, const std::string& description) : - name_(name), description_(description), hasDefault_(false) {} + name_(name), description_(description) {} -Option::~Option() {} - -const std::string& Option::name() const { - return name_; -} - - -bool Option::active() const { - return true; -} - -void Option::set(Configured&) const { - std::ostringstream os; - os << "Option::set() not implemented for " << *this; - throw eckit::SeriousBug(os.str()); -} - -Option* Option::defaultValue(const std::string& value) { - hasDefault_ = true; - default_ = value; - return this; -} - -void Option::setDefault(Configured& configured) const { - if (hasDefault_) { - set(default_, configured); - } -} - - -template <> -const char* Title::operator()() const { - return "ordinal"; -} - -template <> -const char* Title::operator()() const { - return "integer"; -} - -template <> -const char* Title::operator()() const { - return "real"; -} - -template <> -const char* Title::operator()() const { - return "0/1"; -} - -template <> -const char* Title::operator()() const { - return "string"; -} - -template <> -const char* Title::operator()() const { - return "path"; -} - -template <> -void SimpleOption::set(Configured& parametrisation) const { - parametrisation.set(name_, true); -} - -template <> -void SimpleOption::set(const std::string& value, Configured& parametrisation) const { - parametrisation.set(name_, value); -} - -template <> -void SimpleOption::copy(const Configuration& from, Configured& to) const { - std::string v; - if (from.get(name_, v)) { - to.set(name_, v); - } -} - -template <> -void SimpleOption::print(std::ostream& out) const { - out << " --" << name_ << " (" << description_ << ")"; +std::ostream& operator<<(std::ostream& s, const Option& p) +{ + p.print(s); + return s; } } // namespace eckit::option diff --git a/src/eckit/option/Option.h b/src/eckit/option/Option.h index 01fb3a362..adcb54b63 100644 --- a/src/eckit/option/Option.h +++ b/src/eckit/option/Option.h @@ -17,52 +17,81 @@ #define Option_H #include +#include #include +#include +#include "eckit/config/Configuration.h" +#include "eckit/config/Configured.h" #include "eckit/memory/NonCopyable.h" -namespace eckit { - -class Configuration; -class Configured; - -namespace option { - +namespace eckit::option { class Option : private eckit::NonCopyable { +public: + using args_t = std::vector; + public: // methods Option(const std::string& name, const std::string& description); + virtual ~Option() = default; - virtual ~Option(); // Change to virtual if base class + [[nodiscard]] const std::string& name() const { return name_; }; + [[nodiscard]] const std::string& description() const { return description_; } - virtual Option* defaultValue(const std::string&); + [[nodiscard]] virtual bool active() const { return true; }; - const std::string& name() const; + /** + * Set the value of the option into `parameter`, taking as many values as necessary from the range `[begin, end)`. + * + * Return: number of items consumed from the range `[begin, end)`. + */ + virtual size_t set(Configured& parameter, args_t::const_iterator begin, args_t::const_iterator end) const = 0; - virtual bool active() const; - - virtual void set(Configured&) const; - virtual void set(const std::string& value, Configured&) const = 0; virtual void copy(const Configuration& from, Configured& to) const = 0; - virtual void setDefault(Configured&) const; + + virtual void setDefault(Configured&) const = 0; + + template + static void copy(const std::string& name, const Configuration& from, Configured& to) { + // This is so generic that could probably be provided by Configuration or Configured + T value; + if (from.get(name, value)) { + to.set(name, value); + } + } protected: // members std::string name_; std::string description_; - bool hasDefault_; - std::string default_; - - virtual void print(std::ostream&) const = 0; // Change to virtual if base class + virtual void print(std::ostream&) const = 0; private: - friend std::ostream& operator<<(std::ostream& s, const Option& p) { - p.print(s); - return s; + friend std::ostream& operator<<(std::ostream& s, const Option& p); +}; + +template +class BaseOption : public Option { +public: + BaseOption(const std::string& name, const std::string& description) : + Option(name, description), default_value_{std::nullopt} {}; + BaseOption(const std::string& name, const std::string& description, const T& default_value_) : + Option(name, description), default_value_{std::make_optional(default_value_)} {}; + ~BaseOption() override = default; + + void setDefault(Configured& parametrisation) const final { + if (default_value_) { + set_value(default_value_.value(), parametrisation); + } } + + virtual void set_value(const T& value, Configured& parametrisation) const = 0; + +private: + std::optional default_value_; }; -} // namespace option -} // namespace eckit + +} // namespace eckit::option #endif diff --git a/src/eckit/option/Separator.cc b/src/eckit/option/Separator.cc index 576989af7..3fce7d9e9 100644 --- a/src/eckit/option/Separator.cc +++ b/src/eckit/option/Separator.cc @@ -27,10 +27,6 @@ Separator::Separator(const std::string& description) : Separator::~Separator() {} -void Separator::set(const std::string& value, Configured& parametrisation) const { - NOTIMP; -} - void Separator::copy(const Configuration& from, Configured& to) const { ; } diff --git a/src/eckit/option/Separator.h b/src/eckit/option/Separator.h index d30edc3f1..74508c15b 100644 --- a/src/eckit/option/Separator.h +++ b/src/eckit/option/Separator.h @@ -85,7 +85,7 @@ class Separator : public Option { // -- Overridden methods using Option::set; - void set(const std::string& value, Configured&) const override; + bool active() const override; void copy(const Configuration& from, Configured& to) const override; diff --git a/src/eckit/option/SimpleOption.cc b/src/eckit/option/SimpleOption.cc index d9f0fc452..09bb59eba 100644 --- a/src/eckit/option/SimpleOption.cc +++ b/src/eckit/option/SimpleOption.cc @@ -22,9 +22,9 @@ #include "eckit/option/SimpleOption.h" #include "eckit/utils/Translator.h" -namespace eckit { +namespace eckit::option { -namespace option { +namespace { template struct Title { @@ -32,67 +32,100 @@ struct Title { }; template <> -const char* Title::operator()() const; +inline const char* Title::operator()() const { + return "ordinal"; +} template <> -const char* Title::operator()() const; +inline const char* Title::operator()() const { + return "integer"; +} template <> -const char* Title::operator()() const; +inline const char* Title::operator()() const { + return "real"; +} template <> -const char* Title::operator()() const; +inline const char* Title::operator()() const { + return "0/1"; +} template <> -const char* Title::operator()() const; +inline const char* Title::operator()() const { + return "string"; +} template <> -const char* Title::operator()() const; +inline const char* Title::operator()() const { + return "path"; +} + +} // namespace + +template +SimpleOption::SimpleOption(const std::string& name, const std::string& description) : base_t(name, description) {} template -SimpleOption::SimpleOption(const std::string& name, const std::string& description) : - Option(name, description) {} +SimpleOption::SimpleOption(const std::string& name, const std::string& description, const T& default_value) : + base_t(name, description, default_value) {} + +template <> +inline size_t SimpleOption::set(Configured& parametrisation, args_t::const_iterator begin [[maybe_unused]], + args_t::const_iterator end [[maybe_unused]]) const { + // When handling bool, there is no actual value in the range [begin, end), thus the need for this specialization + set_value(true, parametrisation); + return 0; +} template -SimpleOption::~SimpleOption() {} +size_t SimpleOption::set(Configured& parametrisation, args_t::const_iterator begin, + args_t::const_iterator end) const { + if (begin == end) { + throw UserError("No option value found for SimpleOption, where 1 was expected"); + } + set(*begin, parametrisation); + return 1; +} + +template <> +inline void SimpleOption::set(const std::string& value, Configured& parametrisation) const { + parametrisation.set(name_, value); +} + template void SimpleOption::set(const std::string& value, Configured& parametrisation) const { T v = eckit::Translator()(value); - parametrisation.set(name_, v); + set_value(v, parametrisation); } template -void SimpleOption::set(Configured& parametrisation) const { - Option::set(parametrisation); +void SimpleOption::set_value(const T& value, Configured& parametrisation) const { + parametrisation.set(this->name(), value); } - -template -void SimpleOption::copy(const Configuration& from, Configured& to) const { - T v; +template <> +inline void SimpleOption::copy(const Configuration& from, Configured& to) const { + std::string v; if (from.get(name_, v)) { to.set(name_, v); } } -template <> -void SimpleOption::set(const std::string& value, Configured& parametrisation) const; - -template <> -void SimpleOption::copy(const Configuration& from, Configured& to) const; +template +void SimpleOption::copy(const Configuration& from, Configured& to) const { + Option::copy(this->name(), from, to); +} template <> -void SimpleOption::print(std::ostream& out) const; +inline void SimpleOption::print(std::ostream& out) const { + out << " --" << name_ << " (" << description_ << ")"; +} template void SimpleOption::print(std::ostream& out) const { - out << " --" << name_ << "=" << Title()() << " (" << description_ << ")"; + out << " --" << this->name() << "=" << Title()() << " (" << this->description() << ")"; } -template <> -void SimpleOption::set(Configured& parametrisation) const; - -} // namespace option - -} // namespace eckit +} // namespace eckit::option diff --git a/src/eckit/option/SimpleOption.h b/src/eckit/option/SimpleOption.h index dcd2e5593..67bc685a8 100644 --- a/src/eckit/option/SimpleOption.h +++ b/src/eckit/option/SimpleOption.h @@ -23,18 +23,25 @@ namespace eckit::option { template -class SimpleOption : public Option { +class SimpleOption : public BaseOption { public: + using base_t = BaseOption; + using args_t = Option::args_t; + SimpleOption(const std::string& name, const std::string& description); + SimpleOption(const std::string& name, const std::string& description, const T& default_value); - ~SimpleOption() override; + ~SimpleOption() override = default; protected: void print(std::ostream&) const override; private: - void set(Configured&) const override; - void set(const std::string& value, Configured&) const override; + size_t set(Configured&, args_t::const_iterator begin, args_t::const_iterator end) const override; + + void set(const std::string& value, Configured&) const; + void set_value(const T& value, Configured&) const override; + void copy(const Configuration& from, Configured& to) const override; }; diff --git a/src/eckit/option/VectorOption.cc b/src/eckit/option/VectorOption.cc index 89e807d35..c03ba2d2f 100644 --- a/src/eckit/option/VectorOption.cc +++ b/src/eckit/option/VectorOption.cc @@ -32,18 +32,23 @@ namespace option { template VectorOption::VectorOption(const std::string& name, const std::string& description, size_t size, const char* separator) : - Option(name, description), size_(size), separator_(separator) {} - + base_t(name, description), size_(size), separator_(separator) {} template -VectorOption::~VectorOption() {} +VectorOption::VectorOption(const std::string& name, const std::string& description, size_t size, + const std::vector& default_value, const char* separator) : + base_t(Option(name, description, default_value)), size_(size), separator_(separator) {} + template -void VectorOption::set(Configured& parametrisation) const { - set(std::string{}, parametrisation); +size_t VectorOption::set(Configured& parametrisation, args_t::const_iterator begin, args_t::const_iterator end) const { + if (begin == end) { + throw UserError("No option value found for VectorOption, where 1 was expected"); + } + set(*begin, parametrisation); + return 1; } - template void VectorOption::set(const std::string& value, Configured& parametrisation) const { eckit::Translator t; @@ -59,15 +64,20 @@ void VectorOption::set(const std::string& value, Configured& parametrisation) if (size_) { if (values.size() != size_) - throw UserError(std::string("Size of supplied vector \"") + name_ + "\" incorrect", Here()); + throw UserError(std::string("Size of supplied vector \"") + this->name() + "\" incorrect", Here()); } - parametrisation.set(name_, values); + set_value(values, parametrisation); +} + +template +void VectorOption::set_value(const std::vector& value, Configured& parametrisation) const { + parametrisation.set(this->name(), value); } template void VectorOption::print(std::ostream& out) const { - out << " --" << name_; + out << " --" << this->name(); const char* sep = "="; for (size_t i = 0; i < (size_ ? size_ : 2); i++) { @@ -79,16 +89,13 @@ void VectorOption::print(std::ostream& out) const { out << sep << "..."; } - out << " (" << description_ << ")"; + out << " (" << this->description() << ")"; } template void VectorOption::copy(const Configuration& from, Configured& to) const { - std::vector v; - if (from.get(name_, v)) { - to.set(name_, v); - } + Option::copy(this->name(), from, to); } diff --git a/src/eckit/option/VectorOption.h b/src/eckit/option/VectorOption.h index a9cb0f51f..f4abfa529 100644 --- a/src/eckit/option/VectorOption.h +++ b/src/eckit/option/VectorOption.h @@ -24,18 +24,21 @@ namespace eckit::option { template -class VectorOption : public Option { +class VectorOption : public BaseOption> { public: + using base_t = BaseOption>; + using args_t = Option::args_t; // -- Exceptions // None // -- Contructors VectorOption(const std::string& name, const std::string& description, size_t size, const char* separator = "/"); + VectorOption(const std::string& name, const std::string& description, size_t size, const std::vector& default_value, const char* separator = "/"); // -- Destructor - ~VectorOption() override; // Change to virtual if base class + ~VectorOption() override = default; // -- Convertors // None @@ -44,7 +47,7 @@ class VectorOption : public Option { // None // -- Methods - // None + void set_value(const std::vector& value, Configured&) const override; // -- Overridden methods @@ -88,8 +91,9 @@ class VectorOption : public Option { // -- Overridden methods - void set(Configured&) const override; - void set(const std::string& value, Configured&) const override; + size_t set(Configured&, args_t::const_iterator begin, args_t::const_iterator end) const override; + void set(const std::string& value, Configured&) const; + void copy(const Configuration& from, Configured& to) const override; // -- Class members diff --git a/tests/option/CMakeLists.txt b/tests/option/CMakeLists.txt index 9e5c91176..b3ef3070b 100644 --- a/tests/option/CMakeLists.txt +++ b/tests/option/CMakeLists.txt @@ -1,4 +1,4 @@ -foreach( TESTCASE RANGE 1 12 ) +foreach( TESTCASE RANGE 1 28 ) ecbuild_add_test( TARGET eckit_test_option_cmdargs_${TESTCASE} SOURCES eckit_test_option_cmdargs.cc diff --git a/tests/option/eckit_test_option_cmdargs.cc b/tests/option/eckit_test_option_cmdargs.cc index d219103b0..adda07481 100644 --- a/tests/option/eckit_test_option_cmdargs.cc +++ b/tests/option/eckit_test_option_cmdargs.cc @@ -9,6 +9,7 @@ */ #include "eckit/option/CmdArgs.h" +#include "eckit/option/MultiValueOption.h" #include "eckit/option/SimpleOption.h" #include "eckit/option/VectorOption.h" #include "eckit/runtime/Main.h" @@ -236,6 +237,552 @@ CASE("test_eckit_option_cmdargs_vector_size_check") { //---------------------------------------------------------------------------------------------------------------------- +#if TESTCASE == 13 +CASE("test_eckit_option__string_with_default_value") { + std::vector options; + options.push_back(new SimpleOption("arg1", "", "default-value")); + + std::vector input = {"exe"}; + Main::initialise(input.size(), const_cast(&input[0]), nullptr); + + CmdArgs args(&usage, options, 0, 0, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + EXPECT(args.has("arg1")); + auto arg1_actual = args.getString("arg1"); + EXPECT_EQUAL(arg1_actual, "default-value"); +} +#endif + +#if TESTCASE == 14 +CASE("test_eckit_option__string_no_default_value") { + std::vector options; + options.push_back(new SimpleOption("arg1", "")); + + std::vector input = {"exe"}; + Main::initialise(input.size(), const_cast(&input[0]), nullptr); + + CmdArgs args(&usage, options, 0, 0, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + EXPECT(!args.has("arg1")); +} +#endif + +#if TESTCASE == 15 +CASE("test_eckit_option__bool_plus_separated_string") { + std::vector options; + options.push_back(new SimpleOption("arg1", "")); + options.push_back(new SimpleOption("arg2", "")); + + std::vector input = {"exe", "--arg2", "--arg1", "value"}; + Main::initialise(input.size(), const_cast(&input[0]), nullptr); + + CmdArgs args(&usage, options, 0, 0, true); + EXPECT(args.has("arg1")); + EXPECT(args.has("arg2")); + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + { + std::string s; + args.get("arg1", s); + EXPECT_EQUAL(s, "value"); + auto arg1_actual = args.getString("arg1"); + EXPECT_EQUAL(arg1_actual, "value"); + } + { + bool flag; + args.get("arg2", flag); + EXPECT_EQUAL(flag, true); + auto arg2_actual = args.getBool("arg2"); + EXPECT_EQUAL(arg2_actual, true); + } +} +#endif + +#if TESTCASE == 16 +CASE("test_eckit_option__bool_plus_joint_string") { + std::vector options; + options.push_back(new SimpleOption("arg1", "")); + options.push_back(new SimpleOption("arg2", "")); + + std::vector input = {"exe", "--arg2", "--arg1=value"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, 0, 0, true); + EXPECT(args.has("arg1")); + EXPECT(args.has("arg2")); + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + { + std::string s; + args.get("arg1", s); + EXPECT_EQUAL(s, "value"); + auto arg1_actual = args.getString("arg1"); + EXPECT_EQUAL(arg1_actual, "value"); + } + { + bool flag; + args.get("arg2", flag); + EXPECT_EQUAL(flag, true); + auto arg2_actual = args.getBool("arg2"); + EXPECT_EQUAL(arg2_actual, true); + } +} +#endif + +#if TESTCASE == 17 +CASE("test_eckit_option__long_plus_joint_string") { + std::vector options; + options.push_back(new SimpleOption("arg1", "")); + options.push_back(new SimpleOption("arg2", "")); + + std::vector input = {"exe", "--arg2=12345", "--arg1=value"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, 0, 0, true); + EXPECT(args.has("arg1")); + EXPECT(args.has("arg2")); + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + { + std::string s; + args.get("arg1", s); + EXPECT_EQUAL(s, "value"); + auto arg1_actual = args.getString("arg1"); + EXPECT_EQUAL(arg1_actual, "value"); + } + + { + long i; + args.get("arg2", i); + EXPECT_EQUAL(i, 12345); + auto arg2_actual = args.getLong("arg2"); + EXPECT_EQUAL(arg2_actual, 12345L); + } +} +#endif + +#if TESTCASE == 18 +CASE("test_eckit_option__long_plus_joint_and_separated_string") { + std::vector options; + options.push_back(new SimpleOption("arg1", "")); + options.push_back(new SimpleOption("arg2", "")); + options.push_back(new SimpleOption("arg3", "")); + + std::vector input + = {"exe", "--arg1", "value1", "pos1", "pos2", "pos3", "--arg2=value2", "pos4", "--arg3=1234", "pos5"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 4, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 5); + auto pos1_actual = args(0); + EXPECT_EQUAL(pos1_actual, "pos1"); + auto pos2_actual = args(1); + EXPECT_EQUAL(pos2_actual, "pos2"); + auto pos3_actual = args(2); + EXPECT_EQUAL(pos3_actual, "pos3"); + auto pos4_actual = args(3); + EXPECT_EQUAL(pos4_actual, "pos4"); + auto pos5_actual = args(4); + EXPECT_EQUAL(pos5_actual, "pos5"); + + { + EXPECT(args.has("arg1")); + std::string s; + args.get("arg1", s); + EXPECT_EQUAL(s, "value1"); + auto arg1_actual = args.getString("arg1"); + EXPECT_EQUAL(arg1_actual, "value1"); + } + + { + EXPECT(args.has("arg2")); + std::string s; + args.get("arg2", s); + EXPECT_EQUAL(s, "value2"); + auto arg2_actual = args.getString("arg2"); + EXPECT_EQUAL(arg2_actual, "value2"); + } + + { + EXPECT(args.has("arg3")); + long i; + args.get("arg3", i); + EXPECT_EQUAL(i, 1234L); + auto arg3_actual = args.getLong("arg3"); + EXPECT_EQUAL(arg3_actual, 1234L); + } +} +#endif + +#if TESTCASE == 19 +CASE("test_eckit_option__multi_separate_value_alone") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2)); + + std::vector input = {"exe", "--arg1", "value1", "value2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, 0, 0, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + } +} +#endif + +#if TESTCASE == 20 +CASE("test_eckit_option__multi_joint_value_alone") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2)); + + std::vector input = {"exe", "--arg1=value1", "value2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, 0, 0, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + } +} +#endif + +#if TESTCASE == 21 +CASE("test_eckit_option__multi_value_no_default_value") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2)); + + std::vector input = {"exe", "pos1", "pos2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 2, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 2); + + EXPECT(!args.has("arg1")); +} +#endif + +#if TESTCASE == 22 +CASE("test_eckit_option__multi_value_with_default_value") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2, MultiValueOption::values_t{"value1", "value2"})); + + std::vector input = {"exe", "pos1", "pos2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 2, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 2); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + } +} +#endif + +#if TESTCASE == 23 +CASE("test_eckit_option__multi_value_with_1_mandatory_0_optional") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 1, 0)); + + std::vector input = {"exe", "--arg1", "value1", "pos1", "pos2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 2, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 2); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 1); + EXPECT_EQUAL(v[0], "value1"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 1); + EXPECT_EQUAL(v[0], "value1"); + } + } +} +#endif + +#if TESTCASE == 24 +CASE("test_eckit_option__multi_value_with_2_mandatory_0_optional") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2, 0)); + + std::vector input = {"exe", "--arg1", "value1", "value2", "pos1", "pos2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 2, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 2); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + } + } +} +#endif + +#if TESTCASE == 25 +CASE("test_eckit_option__multi_value_with_2_mandatory_2_optional_2_provided") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2, 2)); + options.push_back(new SimpleOption("arg2", "")); + + std::vector input + = {"exe", "--arg1", "value1", "value2", "optional1", "optional2", "--arg2", "pos1", "pos2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 2, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 2); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 4); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + EXPECT_EQUAL(v[2], "optional1"); + EXPECT_EQUAL(v[3], "optional2"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 4); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + EXPECT_EQUAL(v[2], "optional1"); + EXPECT_EQUAL(v[3], "optional2"); + } + } + + EXPECT(args.has("arg2")); +} +#endif + +#if TESTCASE == 26 +CASE("test_eckit_option__multi_value_with_2_mandatory_2_optional_1_provided_followed_by_another_option") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2, 2)); + options.push_back(new SimpleOption("arg2", "")); + + std::vector input = {"exe", "--arg1", "value1", "value2", "optional1", "--arg2", "pos1", "pos2"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 2, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 2); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 3); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + EXPECT_EQUAL(v[2], "optional1"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 3); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + EXPECT_EQUAL(v[2], "optional1"); + } + } + + EXPECT(args.has("arg2")); +} +#endif + +#if TESTCASE == 27 +CASE("test_eckit_option__multi_value_with_2_mandatory_2_optional_1_provided_at_end") { + std::vector options; + options.push_back(new MultiValueOption("arg1", "", 2, 2)); + + std::vector input = {"exe", "--arg1", "value1", "value2", "optional1"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 0, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 0); + + { + EXPECT(args.has("arg1")); + { + std::vector v; + args.get("arg1", v); + EXPECT_EQUAL(v.size(), 3); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + EXPECT_EQUAL(v[2], "optional1"); + } + { + std::vector v = args.getStringVector("arg1"); + EXPECT_EQUAL(v.size(), 3); + EXPECT_EQUAL(v[0], "value1"); + EXPECT_EQUAL(v[1], "value2"); + EXPECT_EQUAL(v[2], "optional1"); + } + } +} +#endif + +#if TESTCASE == 28 +CASE("test_eckit_option__multi_value_with_others") { + std::vector options; + options.push_back(new SimpleOption("arg1", "")); + options.push_back(new SimpleOption("arg2", "")); + options.push_back(new MultiValueOption("label", "", 2)); + + std::vector input = {"exe", + "--arg1", + "value1", + "pos1", + "pos2", + "pos3", + "--arg2=value2", + "pos4", + "--label=somelabel", + "--a-label-value--", + "pos5"}; + Main::initialise(input.size(), const_cast(&input[0])); + + CmdArgs args(&usage, options, -1, 4, true); + + auto args_count = args.count(); + EXPECT_EQUAL(args_count, 5); + auto pos1_actual = args(0); + EXPECT_EQUAL(pos1_actual, "pos1"); + auto pos2_actual = args(1); + EXPECT_EQUAL(pos2_actual, "pos2"); + auto pos3_actual = args(2); + EXPECT_EQUAL(pos3_actual, "pos3"); + auto pos4_actual = args(3); + EXPECT_EQUAL(pos4_actual, "pos4"); + auto pos5_actual = args(4); + EXPECT_EQUAL(pos5_actual, "pos5"); + + { + EXPECT(args.has("arg1")); + std::string s; + args.get("arg1", s); + EXPECT_EQUAL(s, "value1"); + auto arg1_actual = args.getString("arg1"); + EXPECT_EQUAL(arg1_actual, "value1"); + } + + { + EXPECT(args.has("arg2")); + std::string s; + args.get("arg2", s); + EXPECT_EQUAL(s, "value2"); + auto arg2_actual = args.getString("arg2"); + EXPECT_EQUAL(arg2_actual, "value2"); + } + + { + EXPECT(args.has("label")); + { + std::vector v; + args.get("label", v); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "somelabel"); + EXPECT_EQUAL(v[1], "--a-label-value--"); + } + { + std::vector v = args.getStringVector("label"); + EXPECT_EQUAL(v.size(), 2); + EXPECT_EQUAL(v[0], "somelabel"); + EXPECT_EQUAL(v[1], "--a-label-value--"); + } + } +} +#endif + } // namespace eckit::test int main(int argc, char** argv) {