From 743f4f798a15091488519cf4bfe94debed8d5f26 Mon Sep 17 00:00:00 2001 From: snoyer Date: Sat, 14 Sep 2024 21:42:20 +0400 Subject: [PATCH] improve options parsing (#1616) First pass on the new options IO. Here we improve parsing for the existing `bool`, `int`, `double`, `ratio_t` types. Also split the IO tests from the options structure tests (the options tests could possibly be reduced in the future as they currently also do some parsing/formatting test). --- library/private/options_tools.h.in | 88 ++++++++++++++++++++++------ library/public/options.h.in | 13 +++- library/testing/CMakeLists.txt | 2 + library/testing/PseudoUnitTest.h | 31 +++++++--- library/testing/TestSDKOptions.cxx | 45 -------------- library/testing/TestSDKOptionsIO.cxx | 75 ++++++++++++++++++++++++ 6 files changed, 180 insertions(+), 74 deletions(-) create mode 100644 library/testing/TestSDKOptionsIO.cxx diff --git a/library/private/options_tools.h.in b/library/private/options_tools.h.in index c4e04150f9..ef64c89db7 100644 --- a/library/private/options_tools.h.in +++ b/library/private/options_tools.h.in @@ -4,6 +4,7 @@ #include "options.h" #include "types.h" +#include #include namespace f3d @@ -23,9 +24,36 @@ std::string trim(std::string_view strv) return std::string(strv); } +//---------------------------------------------------------------------------- +/** Call `std::stod` and throw `std::invalid_argument` if the whole string has not been consumed. */ +double stodStrict(const std::string& str) +{ + std::size_t pos = 0; + const double parsed = std::stod(str, &pos); + if (pos != str.size()) + { + throw std::invalid_argument("partial"); + } + return parsed; +} + +//---------------------------------------------------------------------------- +/** Call `std::stoi` and throw `std::invalid_argument` if the whole string has not been consumed */ +int stoiStrict(const std::string& str) +{ + std::size_t pos = 0; + const int parsed = std::stoi(str, &pos); + if (pos != str.size()) + { + throw std::invalid_argument("partial"); + } + return parsed; +} + //---------------------------------------------------------------------------- /** - * Vector specific templated parse method + * Vector specific templated parse method. + * Splits on `","` and trims chunks before parsing each element */ template struct is_vector : std::false_type @@ -45,25 +73,29 @@ T parse(const std::string& str) std::istringstream split(str); for (std::string each; std::getline(split, each, ',');) { - vec.emplace_back(options_tools::parse(each)); + vec.emplace_back(options_tools::parse(options_tools::trim(each))); } return vec; } //---------------------------------------------------------------------------- /** - * Parse provided string into a bool - * Numerical and boolaplha are both supported + * Parse provided string into a bool. Supports boolapha values as well as "yes/no" and "on/off" */ template<> bool parse(const std::string& str) { - // TODO implement proper parsing, eg "True", "TRUE", "ON", "On", ... - bool b1; - bool b2; - std::istringstream(str) >> b1; - std::istringstream(str) >> std::boolalpha >> b2; - return b1 || b2; + std::string s = str; + std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { return std::tolower(c); }); + if (s == "true" || s == "yes" || s == "on" || s == "1") + { + return true; + } + if (s == "false" || s == "no" || s == "off" || s == "0") + { + return false; + } + throw options::parsing_exception("Cannot parse " + str + " into a bool"); } //---------------------------------------------------------------------------- @@ -75,10 +107,9 @@ bool parse(const std::string& str) template<> int parse(const std::string& str) { - // TODO implement more parsing possibilities ? try { - return std::stoi(str); + return stoiStrict(str); } catch (std::invalid_argument const&) { @@ -100,10 +131,9 @@ int parse(const std::string& str) template<> double parse(const std::string& str) { - // TODO implement more parsing possibilities ? try { - return std::stod(str); + return stodStrict(str); } catch (std::invalid_argument const&) { @@ -118,15 +148,37 @@ double parse(const std::string& str) //---------------------------------------------------------------------------- /** - * Parse provided string into a ratio_t - * Rely on version for parsing + * Parse provided string into a ratio_t. + * Supported formats: number (same as `` version), percentage, fraction * Can throw options::parsing_exception in case of failure to parse */ template<> ratio_t parse(const std::string& str) { - // TODO implement proper ratio parsing - return options_tools::parse(str); + try + { + if (!str.empty() && str.at(str.size() - 1) == '%') + { + return stodStrict(str.substr(0, str.size() - 1)) / 100; + } + + const std::size_t i = str.find('/'); + if (i != std::string::npos) + { + return stodStrict(str.substr(0, i)) / stodStrict(str.substr(i + 1, str.size() - i - 1)); + } + + return stodStrict(str); + } + catch (std::invalid_argument const&) + { + throw options::parsing_exception("Cannot parse " + str + " into a ratio_t"); + } + catch (std::out_of_range const&) + { + throw options::parsing_exception( + "Cannot parse " + str + " into a ratio_t as it would go out of range"); + } } //---------------------------------------------------------------------------- diff --git a/library/public/options.h.in b/library/public/options.h.in index 22efa63c45..b7059e7171 100644 --- a/library/public/options.h.in +++ b/library/public/options.h.in @@ -108,8 +108,17 @@ public: /** * Templated parsing method used internally to parse strings. - * Implemented only for types: bool, int, double, ratio_t, - * as well as vector for each of these types. + * Implemented for: + * - `bool` as `"true"`/`"false"`, `"1"`/`"0"`, `"yes"`/`"no"`, `"on"`/`"off"` + * (case-insensitive) + * - `int`: `"123"`, `"+123"`, `"-456"` + * - `double`: `"123.45"`, `"+123.45"`, `"-67.8"`, `"-1e3"` + * - `ratio_t`: as + * + a number, same a `double` above + * + a percentage: `"12.3%"` => `0.123` + * + a fraction: `"3/4"` => `0.75` + * - `vector` where `T` is one of the above: the input is split on `","` and chunks are trimmed + * before parsing each element. * See parsing documentation for more info. TODO. * Throw an options::parsing_exception if parsing failed. */ diff --git a/library/testing/CMakeLists.txt b/library/testing/CMakeLists.txt index dac8bdb4b1..1e5470e916 100644 --- a/library/testing/CMakeLists.txt +++ b/library/testing/CMakeLists.txt @@ -18,6 +18,7 @@ list(APPEND libf3dSDKTests_list TestSDKMultiColoring.cxx TestSDKMultiOptions.cxx TestSDKOptions.cxx + TestSDKOptionsIO.cxx TestSDKRenderAndInteract.cxx TestSDKRenderFinalShader.cxx TestSDKWindowNative.cxx @@ -87,6 +88,7 @@ endif() list(APPEND libf3dSDKTestsNoRender_list TestSDKEngineExceptions TestSDKOptions + TestSDKOptionsIO TestSDKLog TestSDKLoader) diff --git a/library/testing/PseudoUnitTest.h b/library/testing/PseudoUnitTest.h index 53447d9866..55ebbd7e79 100644 --- a/library/testing/PseudoUnitTest.h +++ b/library/testing/PseudoUnitTest.h @@ -108,6 +108,28 @@ class PseudoUnitTest this->log(success, label, message); } + template + std::string toString(const T& value) + { + std::stringstream ss; + ss << value; + return ss.str(); + } + + template + std::string toString(const std::vector& value) + { + std::stringstream ss; + size_t i = 0; + for (const T& item : value) + { + ss << (i++ ? ", " : "{ ") << this->toString(item); + } + ss << " }"; + return ss.str(); + } + +protected: template std::string comparisonMessage(const T1& actual, const T2& expected, const std::string& comp) { @@ -125,15 +147,6 @@ class PseudoUnitTest } } - template - std::string toString(const T& value) - { - std::stringstream ss; - ss << value; - return ss.str(); - } - -protected: virtual void log(const bool success, const std::string& label, const std::string& message) { const std::string line = message.empty() ? label : (label + ": " + message); diff --git a/library/testing/TestSDKOptions.cxx b/library/testing/TestSDKOptions.cxx index bb809bc6a1..bd88ac51fd 100644 --- a/library/testing/TestSDKOptions.cxx +++ b/library/testing/TestSDKOptions.cxx @@ -138,50 +138,5 @@ int TestSDKOptions(int argc, char* argv[]) test.expect( "inexistent_exception exception on copy", [&]() { opt.copy(opt2, "dummy"); }); - // Test parse - if (f3d::options::parse(std::string("true")) != true) - { - std::cerr << "Options parse method not behaving as expected with bool." << std::endl; - return EXIT_FAILURE; - } - if (f3d::options::parse(std::string("37")) != 37) - { - std::cerr << "Options parse method not behaving as expected with int." << std::endl; - return EXIT_FAILURE; - } - if (f3d::options::parse(std::string("37.2")) != 37.2) - { - std::cerr << "Options parse method not behaving as expected with double." << std::endl; - return EXIT_FAILURE; - } - if (f3d::options::parse(std::string("0.31")) != 0.31) - { - std::cerr << "Options parse method not behaving as expected with ratio_t." << std::endl; - return EXIT_FAILURE; - } - if (f3d::options::parse(std::string(" foobar ")) != "foobar") - { - std::cerr << "Options parse method not behaving as expected with string." << std::endl; - return EXIT_FAILURE; - } - if (f3d::options::parse>(std::string("1, 2, 3")) != - std::vector({ 1, 2, 3 })) - { - std::cerr << "Options parse method not behaving as expected with int vector." << std::endl; - return EXIT_FAILURE; - } - if (f3d::options::parse>(std::string("0.1, 0.2, 0.3")) != - std::vector({ 0.1, 0.2, 0.3 })) - { - std::cerr << "Options parse method not behaving as expected with double vector." << std::endl; - return EXIT_FAILURE; - } - if (f3d::options::parse>(std::string("foo, bar, baz")) != - std::vector({ "foo", "bar", "baz" })) - { - std::cerr << "Options parse method not behaving as expected with string vector." << std::endl; - return EXIT_FAILURE; - } - return EXIT_SUCCESS; } diff --git a/library/testing/TestSDKOptionsIO.cxx b/library/testing/TestSDKOptionsIO.cxx new file mode 100644 index 0000000000..333fb22321 --- /dev/null +++ b/library/testing/TestSDKOptionsIO.cxx @@ -0,0 +1,75 @@ +#include +#include + +#include "PseudoUnitTest.h" + +#include + +class ParsingTest : public PseudoUnitTest +{ +public: + template + void parse(const std::string& label, const std::string& input, const T& expected) + { + PseudoUnitTest::operator()(label + " `" + input + "`", [&]() { + const T actual = f3d::options::parse(input); + if (actual != expected) + { + throw this->comparisonMessage(actual, expected, "!="); + } + }); + } + + template + void parse_expect(const std::string& label, const std::string& input) + { + PseudoUnitTest::expect(label + " `" + input + "`", [&]() { f3d::options::parse(input); }); + } +}; + +int TestSDKOptionsIO(int argc, char* argv[]) +{ + using parsing_exception = f3d::options::parsing_exception; + ParsingTest test; + + test.parse("bool", "true", true); + test.parse("bool", "false", false); + test.parse("bool", "yes", true); + test.parse("bool", "Yes", true); + test.parse("bool", "no", false); + test.parse("bool", "1", true); + test.parse("bool", "0", false); + test.parse_expect("invalid bool", "foo"); + + test.parse("int", "123", 123); + test.parse("int", "-123", -123); + test.parse("int", "+123", +123); + test.parse_expect("invalid int", "1.2"); + test.parse_expect("invalid int", "abc"); + test.parse_expect("invalid int", + "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"); + + test.parse("double", "123", 123.0); + test.parse("double", "-123.45", -123.45); + test.parse("double", "+1e-3", 0.001); + test.parse_expect("invalid double", "1.2.3"); + test.parse_expect("invalid double", "abc"); + + test.parse("std::string", "foobar", "foobar"); + test.parse("std::string", " foobar ", "foobar"); + + test.parse("ratio_t", "0.1234", 0.1234); + test.parse("ratio_t", "12.34%", 0.1234); + test.parse_expect("invalid ratio_t", "12.34&"); + test.parse("ratio_t", "-2/-3.5", 2.0 / 3.5); + test.parse_expect("invalid ratio_t", "1/2/3"); + + test.parse>("std::vector", "1, 2, 3", { 1, 2, 3 }); + + test.parse>("std::vector", " 0.1, 0.2 , 0.3 ", { 0.1, 0.2, 0.3 }); + + test.parse>( + "std::vector", " foo, bar , baz ", { "foo", "bar", "baz" }); + + return test.result(); +}