Commit 8b56b9c2 authored by Dominik Charousset's avatar Dominik Charousset

Merge pull request #1451

parents 87d449a8 4b4d3fcb
......@@ -13,6 +13,9 @@ is based on [Keep a Changelog](https://keepachangelog.com).
classes symmetrical.
- The template class `caf::dictionary` now has new member functions for erasing
elements.
- The CLI argument parser now supports a space separator when using long
argument names, e.g., `--foo bar`. This is in addition to the existing
`--foo=bar` syntax.
### Changed
......
......@@ -19,6 +19,21 @@ namespace caf {
class CAF_CORE_EXPORT config_option {
public:
// -- member types -----------------------------------------------------------
/// An iterator over CLI arguments.
using argument_iterator = std::vector<std::string>::const_iterator;
/// Stores the result of a find operation. The option sets `begin == end`
/// if the operation could not find a match.
struct find_result {
/// The begin of the matched range.
argument_iterator begin;
/// The end of the matched range.
argument_iterator end;
/// The value for the config option.
std::string_view value;
};
/// Custom vtable-like struct for delegating to type-specific functions and
/// storing type-specific information shared by several config options.
......@@ -92,6 +107,10 @@ public:
/// Returns whether the category is optional for CLI options.
bool has_flat_cli_name() const noexcept;
/// Tries to find this option by its long name in `[first, last)`.
find_result find_by_long_name(argument_iterator first,
argument_iterator last) const noexcept;
private:
std::string_view buf_slice(size_t from, size_t to) const noexcept;
......@@ -104,37 +123,4 @@ private:
mutable void* value_;
};
/// Finds `config_option` string with a matching long name in (`first`, `last`],
/// where each entry is a pointer to a string. Returns a `ForwardIterator` to
/// the match and a `string_view` of the option value if the entry is
/// found and a `ForwardIterator` to `last` with an empty `string_view`
/// otherwise.
template <class ForwardIterator, class Sentinel>
std::pair<ForwardIterator, std::string_view>
find_by_long_name(const config_option& x, ForwardIterator first,
Sentinel last) {
auto long_name = x.long_name();
for (; first != last; ++first) {
std::string_view str{*first};
// Make sure this is a long option starting with "--".
if (!starts_with(str, "--"))
continue;
str.remove_prefix(2);
// Skip optional "caf#" prefix.
if (starts_with(str, "caf#"))
str.remove_prefix(4);
// Make sure we are dealing with the right key.
if (!starts_with(str, long_name))
continue;
// Make sure the key is followed by an assignment.
str.remove_prefix(long_name.size());
if (!starts_with(str, "="))
continue;
// Remove leading '=' and return the value.
str.remove_prefix(1);
return {first, str};
}
return {first, std::string_view{}};
}
} // namespace caf
......@@ -462,17 +462,15 @@ std::pair<error, std::string>
actor_system_config::extract_config_file_path(string_list& args) {
auto ptr = custom_options_.qualified_name_lookup("global.config-file");
CAF_ASSERT(ptr != nullptr);
string_list::iterator i;
std::string_view path;
std::tie(i, path) = find_by_long_name(*ptr, args.begin(), args.end());
if (i == args.end()) {
auto [first, last, path] = ptr->find_by_long_name(args.begin(), args.end());
if (first == args.end()) {
return {none, std::string{}};
} else if (path.empty()) {
return {make_error(pec::missing_argument, "no argument to --config-file"),
std::string{}};
} else {
auto path_str = std::string{path.begin(), path.end()};
args.erase(i);
auto path_str = std::string{path};
args.erase(first, last);
config_value val{path_str};
if (auto err = ptr->sync(val); !err) {
put(content, "config-file", std::move(val));
......
......@@ -133,4 +133,36 @@ std::string_view config_option::buf_slice(size_t from,
return {buf_.get() + from, to - from};
}
// TODO: consider using `config_option_set` and deprecating this
config_option::find_result config_option::find_by_long_name(
config_option::argument_iterator first,
config_option::argument_iterator last) const noexcept {
auto argument_name = long_name();
for (; first != last; ++first) {
std::string_view str{*first};
// Make sure this is a long option starting with "--".
if (!starts_with(str, "--"))
continue;
str.remove_prefix(2);
// Make sure we are dealing with the right key.
if (!starts_with(str, argument_name))
continue;
str.remove_prefix(argument_name.size());
// check for flag
if (is_flag() && str.empty()) {
return {first, first + 1, str};
} else if (starts_with(str, "=")) {
// Remove leading '=' and return the value.
str.remove_prefix(1);
return {first, first + 1, str};
} else if (auto val = first + 1; str.empty() && val != last) {
// Get the next argument the value
return {first, first + 2, std::string_view{*val}};
} else {
continue;
}
}
return {first, first, std::string_view{}};
}
} // namespace caf
......@@ -167,23 +167,24 @@ auto config_option_set::parse(settings& config, argument_iterator first,
}
}
};
// We loop over the first N-1 values, because we always consider two
// arguments at once.
for (auto i = first; i != last;) {
if (i->size() < 2)
return {pec::not_an_option, i};
if (*i == "--")
return {pec::success, std::next(first)};
if (i->compare(0, 2, "--") == 0) {
// Long options use the syntax "--<name>=<value>" and consume only a
// single argument.
auto npos = std::string::npos;
// Long options come in three varieties:
// "--<name>", config option is a boolean flag
// "--<name>=<value>", formatted as a single argument with the value,
// "--<name> <value>", formatted as two arguments,
const auto npos = std::string::npos;
auto assign_op = i->find('=');
auto name = assign_op == npos ? i->substr(2)
: i->substr(2, assign_op - 2);
auto opt = cli_long_name_lookup(name);
if (opt == nullptr)
return {pec::not_an_option, i};
if (opt->is_flag() || assign_op != npos) {
auto code
= consume(*opt,
assign_op == npos
......@@ -193,6 +194,16 @@ auto config_option_set::parse(settings& config, argument_iterator first,
if (code != pec::success)
return {code, i};
++i;
} else {
auto j = std::next(i);
if (j == last) {
return {pec::missing_argument, j};
}
auto code = consume(*opt, j->begin(), j->end());
if (code != pec::success)
return {code, i};
std::advance(i, 2);
}
} else if (i->front() == '-') {
// Short options have three possibilities.
auto opt = cli_short_name_lookup((*i)[1]);
......
......@@ -55,7 +55,6 @@ struct fixture {
void parse(const char* file_content, string_list args = {}) {
cfg.clear();
cfg.remainder.clear();
std::istringstream conf{file_content};
if (auto err = cfg.parse(std::move(args), conf))
CAF_FAIL("parse() failed: " << err);
......
......@@ -343,29 +343,29 @@ CAF_TEST(find by long opt) {
auto needle = make_config_option<std::string>("?foo"sv, "bar,b"sv,
"test option"sv);
auto check = [&](std::vector<string> args, bool found_opt, bool has_opt) {
auto res = find_by_long_name(needle, std::begin(args), std::end(args));
CHECK_EQ(res.first != std::end(args), found_opt);
auto res = needle.find_by_long_name(std::begin(args), std::end(args));
CHECK_EQ(res.begin != std::end(args), found_opt);
if (has_opt)
CHECK_EQ(res.second, "val2");
CHECK_EQ(res.value, "val2");
else
CHECK(res.second.empty());
CHECK(res.value.empty());
};
// Well formed, find val2.
check({"--foo=val1", "--bar=val2", "--baz=val3"}, true, true);
check({"--foo=val1", "--bar", "val2", "--baz=val3"}, true, true);
// Dashes missing, no match.
check({"--foo=val1", "bar=val2", "--baz=val3"}, false, false);
// Equal missing.
check({"--fooval1", "--barval2", "--bazval3"}, false, false);
// Option value missing.
check({"--foo=val1", "--bar=", "--baz=val3"}, true, false);
// With prefix 'caf#'.
check({"--caf#foo=val1", "--caf#bar=val2", "--caf#baz=val3"}, true, true);
// Option not included.
check({"--foo=val1", "--b4r=val2", "--baz=val3"}, false, false);
// Option not included, with prefix.
check({"--caf#foo=val1", "--caf#b4r=val2", "--caf#baz=val3"}, false, false);
// No options to look through.
check({}, false, false);
// flag option for booleans
needle = make_config_option<bool>("?foo"sv, "bar,b"sv, "test option"sv);
check({"--foo=val1", "--bar", "--baz=val3"}, true, false);
}
END_FIXTURE_SCOPE()
......@@ -109,9 +109,22 @@ CAF_TEST(parse with ref syncing) {
CHECK_EQ(get_as<int>(cfg, "foo.i"), 42);
}
CAF_TEST(long format for flags) {
auto foo_b = false;
opts.add<bool>(foo_b, "foo", "b,b", "");
settings cfg;
vector<string> args{"--foo.b"};
auto res = opts.parse(cfg, args);
CHECK_EQ(res.first, pec::success);
if (res.second != args.end())
CAF_FAIL("parser stopped at: " << *res.second);
CHECK_EQ(foo_b, true);
}
CAF_TEST(string parameters) {
opts.add<std::string>("value,v", "some value");
CHECK_EQ(read<std::string>({"--value=foobar"}), "foobar");
CHECK_EQ(read<std::string>({"--value", "foobar"}), "foobar");
CHECK_EQ(read<std::string>({"-v", "foobar"}), "foobar");
CHECK_EQ(read<std::string>({"-vfoobar"}), "foobar");
}
......@@ -121,8 +134,11 @@ CAF_TEST(flat CLI options) {
opts.add<std::string>("?foo", "bar,b", "some value");
CHECK(opts.begin()->has_flat_cli_name());
CHECK_EQ(read<std::string>({"-b", "foobar"}), "foobar");
CHECK_EQ(read<std::string>({"-bfoobar"}), "foobar");
CHECK_EQ(read<std::string>({"--bar=foobar"}), "foobar");
CHECK_EQ(read<std::string>({"--foo.bar=foobar"}), "foobar");
CHECK_EQ(read<std::string>({"--bar", "foobar"}), "foobar");
CHECK_EQ(read<std::string>({"--foo.bar", "foobar"}), "foobar");
}
CAF_TEST(flat CLI parsing with nested categories) {
......@@ -130,8 +146,11 @@ CAF_TEST(flat CLI parsing with nested categories) {
opts.add<std::string>("?foo.goo", "bar,b", "some value");
CHECK(opts.begin()->has_flat_cli_name());
CHECK_EQ(read<std::string>({"-b", "foobar"}), "foobar");
CHECK_EQ(read<std::string>({"-bfoobar"}), "foobar");
CHECK_EQ(read<std::string>({"--bar=foobar"}), "foobar");
CHECK_EQ(read<std::string>({"--foo.goo.bar=foobar"}), "foobar");
CHECK_EQ(read<std::string>({"--bar", "foobar"}), "foobar");
CHECK_EQ(read<std::string>({"--foo.goo.bar", "foobar"}), "foobar");
}
CAF_TEST(square brackets are optional on the command line) {
......@@ -146,6 +165,15 @@ CAF_TEST(square brackets are optional on the command line) {
CHECK_EQ(read<int_list>({"--value=1"}), int_list({1}));
CHECK_EQ(read<int_list>({"--value=1,2,3"}), int_list({1, 2, 3}));
CHECK_EQ(read<int_list>({"--value=1, 2 , 3 , "}), int_list({1, 2, 3}));
CHECK_EQ(read<int_list>({"--value", "[1]"}), int_list({1}));
CHECK_EQ(read<int_list>({"--value", "[1,]"}), int_list({1}));
CHECK_EQ(read<int_list>({"--value", "[ 1 , ]"}), int_list({1}));
CHECK_EQ(read<int_list>({"--value", "[1,2]"}), int_list({1, 2}));
CHECK_EQ(read<int_list>({"--value", "[1, 2, 3]"}), int_list({1, 2, 3}));
CHECK_EQ(read<int_list>({"--value", "[1, 2, 3, ]"}), int_list({1, 2, 3}));
CHECK_EQ(read<int_list>({"--value", "1"}), int_list({1}));
CHECK_EQ(read<int_list>({"--value", "1,2,3"}), int_list({1, 2, 3}));
CHECK_EQ(read<int_list>({"--value", "1, 2 , 3 , "}), int_list({1, 2, 3}));
CHECK_EQ(read<int_list>({"-v", "[1]"}), int_list({1}));
CHECK_EQ(read<int_list>({"-v", "[1,]"}), int_list({1}));
CHECK_EQ(read<int_list>({"-v", "[ 1 , ]"}), int_list({1}));
......
find_package(Python COMPONENTS Interpreter)
add_test(
NAME "robot-config-read-config-file"
COMMAND
${Python_EXECUTABLE}
-m robot
--variable BINARY_PATH:$<TARGET_FILE:hello_world>
"${CMAKE_CURRENT_SOURCE_DIR}/config/read-config-file.robot")
add_test(
NAME "robot-config-dump-config"
COMMAND
......
*** Settings ***
Documentation A test suite for --config-file.
Library OperatingSystem
Library Process
*** Variables ***
${BINARY_PATH} /path/to/the/test/binary
*** Test Cases ***
Test --config-file argument
[Documentation] Binary with a --config-file option tries to open the provided file.
[Tags] Config
Run Process ${BINARY_PATH} --config-file\=test-me.cfg stderr=output
${error_output}= Get File output
Should Contain ${error_output} cannot_open_file("test-me.cfg")
Run Process ${BINARY_PATH} --config-file test-me.cfg stderr=output
${error_output}= Get File output
Should Contain ${error_output} cannot_open_file("test-me.cfg")
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment