Commit f31c918c authored by Dominik Charousset's avatar Dominik Charousset

Fix CLI handling of string and atom_value values

Fix parsing of CLI arguments for strings and atom values. Other types
still fall back to the INI parser, which we need to imrpove eventually.
For example, URIs require surrounding "<>", a list of strings still
requires each string to be quoted, etc.
parent a887399c
...@@ -40,10 +40,14 @@ public: ...@@ -40,10 +40,14 @@ public:
/// Stores a value in the given location. /// Stores a value in the given location.
void (*store)(void*, const config_value&); void (*store)(void*, const config_value&);
/// Tries to extrac a value from the given location. Exists for backward /// Tries to extract a value from the given location. Exists for backward
/// compatibility only and will get removed with CAF 0.17. /// compatibility only and will get removed with CAF 0.17.
config_value (*get)(const void*); config_value (*get)(const void*);
/// Tries to parse an input string. Stores and returns the parsed value on
/// success, returns an error otherwise.
expected<config_value> (*parse)(void*, string_view);
/// Human-readable name of the option's type. /// Human-readable name of the option's type.
std::string type_name; std::string type_name;
}; };
...@@ -96,6 +100,10 @@ public: ...@@ -96,6 +100,10 @@ public:
/// Returns whether this config option stores a boolean flag. /// Returns whether this config option stores a boolean flag.
bool is_flag() const noexcept; bool is_flag() const noexcept;
/// Tries to parse an input string. Stores and returns the parsed value on
/// success, returns an error otherwise.
expected<config_value> parse(string_view input) const;
/// @private /// @private
// TODO: remove with CAF 0.17 // TODO: remove with CAF 0.17
optional<config_value> get() const; optional<config_value> get() const;
...@@ -113,4 +121,3 @@ private: ...@@ -113,4 +121,3 @@ private:
}; };
} // namespace caf } // namespace caf
...@@ -24,25 +24,44 @@ ...@@ -24,25 +24,44 @@
#include "caf/config_value.hpp" #include "caf/config_value.hpp"
#include "caf/detail/type_name.hpp" #include "caf/detail/type_name.hpp"
#include "caf/error.hpp" #include "caf/error.hpp"
#include "caf/expected.hpp"
#include "caf/fwd.hpp" #include "caf/fwd.hpp"
#include "caf/pec.hpp" #include "caf/pec.hpp"
#include "caf/string_view.hpp"
namespace caf { namespace caf {
namespace detail { namespace detail {
template <class T>
error default_config_option_check(const config_value& x) {
if (holds_alternative<T>(x))
return none;
return make_error(pec::type_mismatch);
}
template <class T>
void default_config_option_store(void* ptr, const config_value& x) {
*static_cast<T*>(ptr) = get<T>(x);
}
template <class T>
expected<config_value> default_config_option_parse(void* ptr, string_view str) {
auto result = config_value::parse(str.begin(), str.end());
if (result) {
if (!holds_alternative<T>(*result))
return make_error(pec::type_mismatch);
if (ptr != nullptr)
*static_cast<T*>(ptr) = get<T>(*result);
}
return result;
}
template <class T> template <class T>
config_option::meta_state* option_meta_state_instance() { config_option::meta_state* option_meta_state_instance() {
static config_option::meta_state obj{ static config_option::meta_state obj{
[](const config_value& x) -> error { default_config_option_check<T>, default_config_option_store<T>, nullptr,
if (holds_alternative<T>(x)) default_config_option_parse<T>, detail::type_name<T>()};
return none;
return make_error(pec::type_mismatch);
},
[](void* ptr, const config_value& x) {
*static_cast<T*>(ptr) = get<T>(x);
},
nullptr, detail::type_name<T>()};
return &obj; return &obj;
} }
......
...@@ -25,6 +25,7 @@ ...@@ -25,6 +25,7 @@
#include "caf/config.hpp" #include "caf/config.hpp"
#include "caf/config_value.hpp" #include "caf/config_value.hpp"
#include "caf/error.hpp" #include "caf/error.hpp"
#include "caf/expected.hpp"
#include "caf/optional.hpp" #include "caf/optional.hpp"
using std::move; using std::move;
...@@ -147,6 +148,10 @@ bool config_option::is_flag() const noexcept { ...@@ -147,6 +148,10 @@ bool config_option::is_flag() const noexcept {
return type_name() == "boolean"; return type_name() == "boolean";
} }
expected<config_value> config_option::parse(string_view input) const {
return meta_->parse(value_, input);
}
optional<config_value> config_option::get() const { optional<config_value> config_option::get() const {
if (value_ != nullptr && meta_->get != nullptr) if (value_ != nullptr && meta_->get != nullptr)
return meta_->get(value_); return meta_->get(value_);
......
...@@ -138,24 +138,13 @@ auto config_option_set::parse(settings& config, argument_iterator first, ...@@ -138,24 +138,13 @@ auto config_option_set::parse(settings& config, argument_iterator first,
return pec::missing_argument; return pec::missing_argument;
auto slice_size = static_cast<size_t>(std::distance(arg_begin, arg_end)); auto slice_size = static_cast<size_t>(std::distance(arg_begin, arg_end));
string_view slice{&*arg_begin, slice_size}; string_view slice{&*arg_begin, slice_size};
auto val = config_value::parse(slice); auto val = opt.parse(slice);
if (!val) if (!val) {
auto& err = val.error();
if (err.category() == atom("parser"))
return static_cast<pec>(err.code());
return pec::illegal_argument; return pec::illegal_argument;
if (opt.check(*val) != none) {
// The parser defaults to unescaped strings. For example, --foo=bar
// will interpret `bar` as a string. Hence, we'll get a type mismatch
// if `foo` expects an atom. We check this special case here to avoid
// the clumsy --foo="'bar'" notation on the command line.
if (holds_alternative<std::string>(*val)
&& opt.type_name() == "atom"
&& slice.substr(0, 1) != "\""
&& slice.size() <= 10) {
*val = atom_from_string(std::string{slice.begin(), slice.end()});
} else {
return pec::type_mismatch;
}
} }
opt.store(*val);
entry[opt_name] = std::move(*val); entry[opt_name] = std::move(*val);
} }
return pec::success; return pec::success;
......
...@@ -18,14 +18,15 @@ ...@@ -18,14 +18,15 @@
#include "caf/make_config_option.hpp" #include "caf/make_config_option.hpp"
#include <ctype.h>
#include "caf/config_value.hpp" #include "caf/config_value.hpp"
#include "caf/optional.hpp" #include "caf/optional.hpp"
#define DEFAULT_META(type) \ #define DEFAULT_META(type, parse_fun) \
config_option::meta_state type##_meta_state { \ config_option::meta_state type##_meta_state{ \
check_impl<type>, store_impl<type>, get_impl<type>, \ default_config_option_check<type>, default_config_option_store<type>, \
detail::type_name<type>() \ get_impl<type>, parse_fun, detail::type_name<type>()};
}; \
using std::string; using std::string;
...@@ -35,19 +36,6 @@ namespace { ...@@ -35,19 +36,6 @@ namespace {
using meta_state = config_option::meta_state; using meta_state = config_option::meta_state;
template <class T>
error check_impl(const config_value& x) {
if (holds_alternative<T>(x))
return none;
return make_error(pec::type_mismatch);
}
template <class T>
void store_impl(void* ptr, const config_value& x) {
CAF_ASSERT(ptr != nullptr);
*static_cast<T*>(ptr) = get<T>(x);
}
template <class T> template <class T>
config_value get_impl(const void* ptr) { config_value get_impl(const void* ptr) {
CAF_ASSERT(ptr != nullptr); CAF_ASSERT(ptr != nullptr);
...@@ -76,7 +64,7 @@ config_value bool_get_neg(const void* ptr) { ...@@ -76,7 +64,7 @@ config_value bool_get_neg(const void* ptr) {
return config_value{!*static_cast<const bool*>(ptr)}; return config_value{!*static_cast<const bool*>(ptr)};
} }
meta_state bool_neg_meta{bool_check, bool_store_neg, bool_get_neg, meta_state bool_neg_meta{bool_check, bool_store_neg, bool_get_neg, nullptr,
detail::type_name<bool>()}; detail::type_name<bool>()};
meta_state us_res_meta{ meta_state us_res_meta{
...@@ -94,6 +82,7 @@ meta_state us_res_meta{ ...@@ -94,6 +82,7 @@ meta_state us_res_meta{
timespan val{ival * 1000}; timespan val{ival * 1000};
return config_value{val}; return config_value{val};
}, },
nullptr,
detail::type_name<timespan>() detail::type_name<timespan>()
}; };
...@@ -112,21 +101,44 @@ meta_state ms_res_meta{ ...@@ -112,21 +101,44 @@ meta_state ms_res_meta{
timespan val{ival * 1000000}; timespan val{ival * 1000000};
return config_value{val}; return config_value{val};
}, },
nullptr,
detail::type_name<timespan>()}; detail::type_name<timespan>()};
expected<config_value> parse_atom(void* ptr, string_view str) {
if (str.size() > 10)
return make_error(pec::too_many_characters);
auto is_illegal = [](char c) {
return !(isalnum(c) || c == '_' || c == ' ');
};
auto i = std::find_if(str.begin(), str.end(), is_illegal);
if (i != str.end())
return make_error(pec::unexpected_character,
std::string{"invalid character: "} + *i);
auto result = atom_from_string(str);
if (ptr != nullptr)
*reinterpret_cast<atom_value*>(ptr) = result;
return config_value{result};
}
expected<config_value> parse_string(void* ptr, string_view str) {
std::string result{str.begin(), str.end()};
if (ptr != nullptr)
*reinterpret_cast<std::string*>(ptr) = result;
return config_value{std::move(result)};
}
} // namespace } // namespace
namespace detail { namespace detail {
DEFAULT_META(atom_value) DEFAULT_META(atom_value, parse_atom)
DEFAULT_META(size_t) DEFAULT_META(size_t, default_config_option_parse<size_t>)
DEFAULT_META(string) DEFAULT_META(string, parse_string)
config_option::meta_state bool_meta_state{ config_option::meta_state bool_meta_state{bool_check, bool_store, bool_get,
bool_check, bool_store, bool_get, detail::type_name<bool>() nullptr, detail::type_name<bool>()};
};
} // namespace detail } // namespace detail
......
...@@ -37,6 +37,18 @@ namespace { ...@@ -37,6 +37,18 @@ namespace {
struct fixture { struct fixture {
config_option_set opts; config_option_set opts;
template <class T>
expected<T> read(std::vector<std::string> args) {
settings cfg;
auto res = opts.parse(cfg, std::move(args));
if (res.first != pec::success)
return res.first;
auto x = get_if<T>(&cfg, "value");
if (x == none)
return sec::invalid_argument;
return *x;
}
}; };
} // namespace <anonymous> } // namespace <anonymous>
...@@ -103,23 +115,30 @@ CAF_TEST(parse with ref syncing) { ...@@ -103,23 +115,30 @@ CAF_TEST(parse with ref syncing) {
CAF_TEST(atom parameters) { CAF_TEST(atom parameters) {
opts.add<atom_value>("value,v", "some value"); opts.add<atom_value>("value,v", "some value");
CAF_MESSAGE("test atom option without quotes"); CAF_CHECK_EQUAL(read<atom_value>({"-v", "foobar"}), atom("foobar"));
auto parse_args = [&](std::vector<std::string> args) -> expected<atom_value> { CAF_CHECK_EQUAL(read<atom_value>({"-vfoobar"}), atom("foobar"));
settings cfg; CAF_CHECK_EQUAL(read<atom_value>({"--value=foobar"}), atom("foobar"));
auto res = opts.parse(cfg, std::move(args)); }
if (res.first != pec::success)
return res.first; CAF_TEST(string parameters) {
auto atm = get_if<atom_value>(&cfg, "value"); opts.add<std::string>("value,v", "some value");
if (atm == none) CAF_MESSAGE("test string option with and without quotes");
return sec::invalid_argument; CAF_CHECK_EQUAL(read<std::string>({"--value=\"foobar\""}), "\"foobar\"");
return *atm; CAF_CHECK_EQUAL(read<std::string>({"--value=foobar"}), "foobar");
}; CAF_CHECK_EQUAL(read<std::string>({"-v", "\"foobar\""}), "\"foobar\"");
CAF_CHECK_EQUAL(parse_args({"-v", "'foobar'"}), atom("foobar")); CAF_CHECK_EQUAL(read<std::string>({"-v", "foobar"}), "foobar");
CAF_CHECK_EQUAL(parse_args({"-v'foobar'"}), atom("foobar")); CAF_CHECK_EQUAL(read<std::string>({"-v\"foobar\""}), "\"foobar\"");
CAF_CHECK_EQUAL(parse_args({"--value='foobar'"}), atom("foobar")); CAF_CHECK_EQUAL(read<std::string>({"-vfoobar"}), "foobar");
CAF_CHECK_EQUAL(parse_args({"-v", "foobar"}), atom("foobar")); CAF_CHECK_EQUAL(read<std::string>({"--value=\"'abc'\""}), "\"'abc'\"");
CAF_CHECK_EQUAL(parse_args({"-vfoobar"}), atom("foobar")); CAF_CHECK_EQUAL(read<std::string>({"--value='abc'"}), "'abc'");
CAF_CHECK_EQUAL(parse_args({"--value=foobar"}), atom("foobar")); CAF_CHECK_EQUAL(read<std::string>({"-v", "\"'abc'\""}), "\"'abc'\"");
CAF_CHECK_EQUAL(read<std::string>({"-v", "'abc'"}), "'abc'");
CAF_CHECK_EQUAL(read<std::string>({"-v'abc'"}), "'abc'");
CAF_CHECK_EQUAL(read<std::string>({"--value=\"123\""}), "\"123\"");
CAF_CHECK_EQUAL(read<std::string>({"--value=123"}), "123");
CAF_CHECK_EQUAL(read<std::string>({"-v", "\"123\""}), "\"123\"");
CAF_CHECK_EQUAL(read<std::string>({"-v", "123"}), "123");
CAF_CHECK_EQUAL(read<std::string>({"-v123"}), "123");
} }
CAF_TEST_FIXTURE_SCOPE_END() CAF_TEST_FIXTURE_SCOPE_END()
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