Skip to content

Commit

Permalink
ECKIT-620 Enable multi-value options in CmdArgs
Browse files Browse the repository at this point in the history
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 <command> <name> <value> /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.
  • Loading branch information
marcosbento committed Sep 20, 2023
1 parent ffdee81 commit d0f6f93
Show file tree
Hide file tree
Showing 16 changed files with 929 additions and 221 deletions.
2 changes: 2 additions & 0 deletions src/eckit/option/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
96 changes: 51 additions & 45 deletions src/eckit/option/CmdArgs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,58 +46,71 @@ CmdArgs::CmdArgs(std::function<void(const std::string&)> usage, std::vector<Opti

void CmdArgs::init(std::function<void(const std::string&)> 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<std::string, const option::Option*> opts;

for (std::vector<option::Option*>::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<std::string, const option::Option*>;

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<std::string> v;
parse(a.substr(2), v);
if (a.substr(0, prefix.size()) == prefix) { // An Option 'a' is found (starts with '--')!

std::map<std::string, const option::Option*>::const_iterator j = opts.find(v[0]);
if (j != opts.end()) {
// The Option might be formatted as --<name>=<value>, so we tokenize and take the <name>
std::vector<std::string> 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 <value>)
std::vector<std::string> remaining;
remaining.reserve(tokens.size() + argc);
for (const auto& token : tokens) {
remaining.push_back(token);
}
else {
std::vector<std::string>::const_iterator b = v.begin();
++b;
std::vector<std::string>::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<int>(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()
Expand All @@ -121,16 +134,16 @@ void CmdArgs::init(std::function<void(const std::string&)> usage, int args_count
Log::info() << "Options are:" << std::endl;
Log::info() << "===========:" << std::endl
<< std::endl;
for (std::vector<option::Option*>::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<option::Option*>::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());
}
Expand All @@ -140,15 +153,15 @@ void CmdArgs::init(std::function<void(const std::string&)> usage, int args_count


CmdArgs::~CmdArgs() {
for (std::vector<option::Option*>::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<option::Option*>::const_iterator j = options_.begin(); j != options_.end(); ++j) {
(*j)->copy(*this, c);
for (const Option* j : options_) {
j->copy(*this, c);
}
}

Expand All @@ -159,14 +172,6 @@ void CmdArgs::print(std::ostream& out) const {
out << "]";
}

// const std::set<std::string>& CmdArgs::keys() const {
// return keys_;
// }

// const std::vector<std::string>& CmdArgs::args() const {
// return args_;
// }

const std::string& CmdArgs::operator()(size_t i) const {
ASSERT(i < args_.size());
return args_[i];
Expand All @@ -179,6 +184,7 @@ size_t CmdArgs::count() const {
const std::string& CmdArgs::tool() const {
return tool_;
}

//----------------------------------------------------------------------------------------------------------------------

} // namespace eckit::option
24 changes: 18 additions & 6 deletions src/eckit/option/FactoryOption.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,34 @@ namespace option {

template <class T>
FactoryOption<T>::FactoryOption(const std::string& name, const std::string& description) :
Option(name, description) {}
base_t(name, description) {}

template <class T>
FactoryOption<T>::~FactoryOption() {}
FactoryOption<T>::FactoryOption(const std::string& name, const std::string& description, std::string default_value) :
base_t(name, description, std::move(default_value)) {}

template <class T>
size_t FactoryOption<T>::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 <class T>
void FactoryOption<T>::set(const std::string& value, Configured& parametrisation) const {
set_value(value, parametrisation);
}

template <class T>
void FactoryOption<T>::set_value(const std::string& value, Configured& parametrisation) const {
parametrisation.set(name_, value);
}

template <class T>
void FactoryOption<T>::copy(const Configuration& from, Configured& to) const {
std::string v;
if (from.get(name_, v)) {
to.set(name_, v);
}
Option::copy<std::string>(name_, from, to);
}

template <class T>
Expand Down
14 changes: 10 additions & 4 deletions src/eckit/option/FactoryOption.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,25 @@ namespace eckit::option {
/// the validity of input received, and just returns the appropriate string

template <class T>
class FactoryOption : public Option {
class FactoryOption : public BaseOption<std::string> {
public:
using base_t = BaseOption<std::string>;

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;
};

Expand Down
89 changes: 89 additions & 0 deletions src/eckit/option/MultiValueOption.cc
Original file line number Diff line number Diff line change
@@ -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 <algorithm>
#include <iostream>

#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<values_t>(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
47 changes: 47 additions & 0 deletions src/eckit/option/MultiValueOption.h
Original file line number Diff line number Diff line change
@@ -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 <iosfwd>

#include "eckit/option/Option.h"

namespace eckit::option {

class MultiValueOption : public BaseOption<std::vector<std::string>> {
public:
using base_t = BaseOption<std::vector<std::string>>;
using values_t = std::vector<std::string>;

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
Loading

0 comments on commit d0f6f93

Please sign in to comment.