From 2b9e12a1f92093ff7121792c9a20e4d07a82c8ed Mon Sep 17 00:00:00 2001 From: Yogesh Rathod Date: Sat, 28 Sep 2024 23:36:09 +0530 Subject: [PATCH 1/7] added an api to f3d::options to determine whether an option is optional --- cmake/f3dOptions.cmake | 4 ++++ library/private/options_tools.h.in | 13 +++++++++++++ library/public/options.h.in | 6 ++++++ library/src/options.cxx | 5 +++++ 4 files changed, 28 insertions(+) diff --git a/cmake/f3dOptions.cmake b/cmake/f3dOptions.cmake index 605f97d9b9..da66fe9605 100644 --- a/cmake/f3dOptions.cmake +++ b/cmake/f3dOptions.cmake @@ -74,6 +74,7 @@ function (f3d_generate_options) list(JOIN _options_string_setter ";\n else " _options_string_setter) list(JOIN _options_string_getter ";\n else " _options_string_getter) list(JOIN _options_lister ",\n " _options_lister) + list(JOIN _options_is_optional ";\n else " _options_is_optional) configure_file( "${_f3d_generate_options_INPUT_PUBLIC_HEADER}" @@ -135,10 +136,12 @@ function(_parse_json_option _top_json) # Use default_value string(APPEND _options_struct "${_option_indent} ${_option_actual_type} ${_member_name} = ${_option_default_value_start}${_option_default_value}${_option_default_value_end};\n") set(_optional_getter "") + list(APPEND _options_is_optional "if (name == \"${_option_name}\") return false") else() # No default_value, it is an std::optional string(APPEND _options_struct "${_option_indent} std::optional<${_option_actual_type}> ${_member_name};\n") set(_optional_getter ".value()") + list(APPEND _options_is_optional "if (name == \"${_option_name}\") return true") endif() list(APPEND _options_setter "if (name == \"${_option_name}\") opt.${_option_name} = std::get<${_option_variant_type}>(value)") @@ -168,4 +171,5 @@ function(_parse_json_option _top_json) set(_options_string_setter ${_options_string_setter} PARENT_SCOPE) set(_options_string_getter ${_options_string_getter} PARENT_SCOPE) set(_options_lister ${_options_lister} PARENT_SCOPE) + set(_options_is_optional ${_options_is_optional} PARENT_SCOPE) endfunction() diff --git a/library/private/options_tools.h.in b/library/private/options_tools.h.in index c82444b887..4bb2efb242 100644 --- a/library/private/options_tools.h.in +++ b/library/private/options_tools.h.in @@ -348,6 +348,19 @@ std::string getAsString(const options& opt, const std::string& name) throw options::no_value_exception("Trying to get " + name + " before it was set"); } } + +//---------------------------------------------------------------------------- +/** + * Generated method, see `options::getAsString` + */ +bool IsOptional(const std::string& name) +{ + // clang-format off + ${_options_is_optional}; + // clang-format on + else throw options::inexistent_exception("Option " + name + " does not exist"); +} + } // option_tools } // f3d #endif // f3d_options_tools_h diff --git a/library/public/options.h.in b/library/public/options.h.in index 13dad70f5a..2f265f0a45 100644 --- a/library/public/options.h.in +++ b/library/public/options.h.in @@ -114,6 +114,12 @@ public: */ std::pair getClosestOption(const std::string& option) const; + /** + * Returns true if the option is optional else returns false. + * Throws an options::inexistent_exception if option does not exist. + */ + bool IsOptional(const std::string& option) const; + /** * Templated parsing method used internally to parse strings. * Implemented for: diff --git a/library/src/options.cxx b/library/src/options.cxx index d39e14b7ff..1a88e27055 100644 --- a/library/src/options.cxx +++ b/library/src/options.cxx @@ -136,6 +136,11 @@ std::pair options::getClosestOption(const std::string return ret; } +bool options::IsOptional(const std::string& option) const +{ + return options_tools::IsOptional(option); +} + //---------------------------------------------------------------------------- template T options::parse(const std::string& str) From fb15bedcfee2d09f2676fa0530f0133fb330c01c Mon Sep 17 00:00:00 2001 From: Yogesh Rathod Date: Sat, 28 Sep 2024 23:47:49 +0530 Subject: [PATCH 2/7] renamed IsOptional to isOptional --- library/private/options_tools.h.in | 4 ++-- library/public/options.h.in | 2 +- library/src/options.cxx | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/private/options_tools.h.in b/library/private/options_tools.h.in index 4bb2efb242..4869fd2265 100644 --- a/library/private/options_tools.h.in +++ b/library/private/options_tools.h.in @@ -351,9 +351,9 @@ std::string getAsString(const options& opt, const std::string& name) //---------------------------------------------------------------------------- /** - * Generated method, see `options::getAsString` + * Generated method, see `options::isOptional` */ -bool IsOptional(const std::string& name) +bool isOptional(const std::string& name) { // clang-format off ${_options_is_optional}; diff --git a/library/public/options.h.in b/library/public/options.h.in index 2f265f0a45..228fe03a14 100644 --- a/library/public/options.h.in +++ b/library/public/options.h.in @@ -118,7 +118,7 @@ public: * Returns true if the option is optional else returns false. * Throws an options::inexistent_exception if option does not exist. */ - bool IsOptional(const std::string& option) const; + bool isOptional(const std::string& option) const; /** * Templated parsing method used internally to parse strings. diff --git a/library/src/options.cxx b/library/src/options.cxx index 1a88e27055..01b52c7cd2 100644 --- a/library/src/options.cxx +++ b/library/src/options.cxx @@ -136,9 +136,9 @@ std::pair options::getClosestOption(const std::string return ret; } -bool options::IsOptional(const std::string& option) const +bool options::isOptional(const std::string& option) const { - return options_tools::IsOptional(option); + return options_tools::isOptional(option); } //---------------------------------------------------------------------------- From f6d8ba43a490f340e85913c4e00ca35cc995010a Mon Sep 17 00:00:00 2001 From: Yogesh Rathod Date: Sun, 29 Sep 2024 01:02:06 +0530 Subject: [PATCH 3/7] added an api to f3d::options to remove an optional value and reset option to default value --- cmake/f3dOptions.cmake | 7 +++++++ library/private/options_tools.h.in | 24 ++++++++++++++++++++++++ library/public/options.h.in | 12 ++++++++++++ library/src/options.cxx | 10 ++++++++++ 4 files changed, 53 insertions(+) diff --git a/cmake/f3dOptions.cmake b/cmake/f3dOptions.cmake index da66fe9605..29d72b7ebc 100644 --- a/cmake/f3dOptions.cmake +++ b/cmake/f3dOptions.cmake @@ -75,6 +75,8 @@ function (f3d_generate_options) list(JOIN _options_string_getter ";\n else " _options_string_getter) list(JOIN _options_lister ",\n " _options_lister) list(JOIN _options_is_optional ";\n else " _options_is_optional) + list(JOIN _options_reset ";\n else " _options_reset) + list(JOIN _options_remove_value ";\n else " _options_remove_value) configure_file( "${_f3d_generate_options_INPUT_PUBLIC_HEADER}" @@ -137,11 +139,14 @@ function(_parse_json_option _top_json) string(APPEND _options_struct "${_option_indent} ${_option_actual_type} ${_member_name} = ${_option_default_value_start}${_option_default_value}${_option_default_value_end};\n") set(_optional_getter "") list(APPEND _options_is_optional "if (name == \"${_option_name}\") return false") + list(APPEND _options_reset "if (name == \"${_option_name}\") opt.${_option_name} = ${_option_default_value_start}${_option_default_value}${_option_default_value_end}") else() # No default_value, it is an std::optional string(APPEND _options_struct "${_option_indent} std::optional<${_option_actual_type}> ${_member_name};\n") set(_optional_getter ".value()") list(APPEND _options_is_optional "if (name == \"${_option_name}\") return true") + list(APPEND _options_reset "if (name == \"${_option_name}\") opt.${_option_name}.reset()") + list(APPEND _options_remove_value "if (name == \"${_option_name}\") opt.${_option_name}.reset()") endif() list(APPEND _options_setter "if (name == \"${_option_name}\") opt.${_option_name} = std::get<${_option_variant_type}>(value)") @@ -172,4 +177,6 @@ function(_parse_json_option _top_json) set(_options_string_getter ${_options_string_getter} PARENT_SCOPE) set(_options_lister ${_options_lister} PARENT_SCOPE) set(_options_is_optional ${_options_is_optional} PARENT_SCOPE) + set(_options_reset ${_options_reset} PARENT_SCOPE) + set(_options_remove_value ${_options_remove_value} PARENT_SCOPE) endfunction() diff --git a/library/private/options_tools.h.in b/library/private/options_tools.h.in index 4869fd2265..0e0a5fe189 100644 --- a/library/private/options_tools.h.in +++ b/library/private/options_tools.h.in @@ -361,6 +361,30 @@ bool isOptional(const std::string& name) else throw options::inexistent_exception("Option " + name + " does not exist"); } +//---------------------------------------------------------------------------- +/** + * Generated method, see `options::isOptional` + */ +void reset(options& opt, const std::string& name) +{ + // clang-format off + ${_options_reset}; + // clang-format on + else throw options::inexistent_exception("Option " + name + " does not exist"); +} + +//---------------------------------------------------------------------------- +/** + * Generated method, see `options::removeValue` + */ +void removeValue(options& opt, const std::string& name) +{ + // clang-format off + ${_options_remove_value}; + // clang-format on + else throw options::inexistent_exception("Option " + name + " does not exist or is not not optional"); +} + } // option_tools } // f3d #endif // f3d_options_tools_h diff --git a/library/public/options.h.in b/library/public/options.h.in index 228fe03a14..ced6fe2423 100644 --- a/library/public/options.h.in +++ b/library/public/options.h.in @@ -120,6 +120,18 @@ public: */ bool isOptional(const std::string& option) const; + /** + * Resets the option to default value. + * Throws an options::inexistent_exception if option does not exist. + */ + void reset(options& opt, const std::string& name); + + /** + * Unset the option if it is optional. + * Throws an options::inexistent_exception if option does not exist. + */ + void removeValue(options& opt, const std::string& name); + /** * Templated parsing method used internally to parse strings. * Implemented for: diff --git a/library/src/options.cxx b/library/src/options.cxx index 01b52c7cd2..ec2c21edc8 100644 --- a/library/src/options.cxx +++ b/library/src/options.cxx @@ -141,6 +141,16 @@ bool options::isOptional(const std::string& option) const return options_tools::isOptional(option); } +void options::reset(options& opt, const std::string& name) +{ + options_tools::reset(*this, name); +} + +void options::removeValue(options& opt, const std::string& name) +{ + options_tools::removeValue(*this, name); +} + //---------------------------------------------------------------------------- template T options::parse(const std::string& str) From b805100eade57e967f6003dfbf217e53bc6e5050 Mon Sep 17 00:00:00 2001 From: Yogesh Rathod Date: Sun, 29 Sep 2024 10:30:50 +0530 Subject: [PATCH 4/7] refactored options_tool::removeValue --- cmake/f3dOptions.cmake | 8 +++----- library/private/options_tools.h.in | 15 +++++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cmake/f3dOptions.cmake b/cmake/f3dOptions.cmake index 29d72b7ebc..e7c5e5924c 100644 --- a/cmake/f3dOptions.cmake +++ b/cmake/f3dOptions.cmake @@ -76,7 +76,6 @@ function (f3d_generate_options) list(JOIN _options_lister ",\n " _options_lister) list(JOIN _options_is_optional ";\n else " _options_is_optional) list(JOIN _options_reset ";\n else " _options_reset) - list(JOIN _options_remove_value ";\n else " _options_remove_value) configure_file( "${_f3d_generate_options_INPUT_PUBLIC_HEADER}" @@ -136,17 +135,17 @@ function(_parse_json_option _top_json) if(_default_value_error STREQUAL "NOTFOUND") # Use default_value - string(APPEND _options_struct "${_option_indent} ${_option_actual_type} ${_member_name} = ${_option_default_value_start}${_option_default_value}${_option_default_value_end};\n") + set(_optional_default_value_initialize "${_option_default_value_start}${_option_default_value}${_option_default_value_end}") + string(APPEND _options_struct "${_option_indent} ${_option_actual_type} ${_member_name} = ${_optional_default_value_initialize};\n") set(_optional_getter "") list(APPEND _options_is_optional "if (name == \"${_option_name}\") return false") - list(APPEND _options_reset "if (name == \"${_option_name}\") opt.${_option_name} = ${_option_default_value_start}${_option_default_value}${_option_default_value_end}") + list(APPEND _options_reset "if (name == \"${_option_name}\") opt.${_option_name} = ${_optional_default_value_initialize}") else() # No default_value, it is an std::optional string(APPEND _options_struct "${_option_indent} std::optional<${_option_actual_type}> ${_member_name};\n") set(_optional_getter ".value()") list(APPEND _options_is_optional "if (name == \"${_option_name}\") return true") list(APPEND _options_reset "if (name == \"${_option_name}\") opt.${_option_name}.reset()") - list(APPEND _options_remove_value "if (name == \"${_option_name}\") opt.${_option_name}.reset()") endif() list(APPEND _options_setter "if (name == \"${_option_name}\") opt.${_option_name} = std::get<${_option_variant_type}>(value)") @@ -178,5 +177,4 @@ function(_parse_json_option _top_json) set(_options_lister ${_options_lister} PARENT_SCOPE) set(_options_is_optional ${_options_is_optional} PARENT_SCOPE) set(_options_reset ${_options_reset} PARENT_SCOPE) - set(_options_remove_value ${_options_remove_value} PARENT_SCOPE) endfunction() diff --git a/library/private/options_tools.h.in b/library/private/options_tools.h.in index 0e0a5fe189..a5e8286193 100644 --- a/library/private/options_tools.h.in +++ b/library/private/options_tools.h.in @@ -117,8 +117,7 @@ int parse(const std::string& str) } catch (std::out_of_range const&) { - throw options::parsing_exception( - "Cannot parse " + str + " into an int as it would go out of range"); + throw options::parsing_exception("Cannot parse " + str + " into an int as it would go out of range"); } } @@ -379,10 +378,14 @@ void reset(options& opt, const std::string& name) */ void removeValue(options& opt, const std::string& name) { - // clang-format off - ${_options_remove_value}; - // clang-format on - else throw options::inexistent_exception("Option " + name + " does not exist or is not not optional"); + if (isOptional(name)) + { + reset(opt, name); + } + else + { + throw options::incompatible_exception("Option " + name + " is not not optional"); + } } } // option_tools From e7d8d2c831f85cb727fcfe61cd685c2f53fd9e85 Mon Sep 17 00:00:00 2001 From: Yogesh Rathod Date: Sun, 29 Sep 2024 19:01:59 +0530 Subject: [PATCH 5/7] Added test for options::reset, options::isOptional, options::removeValue --- library/private/options_tools.h.in | 3 +- library/public/options.h.in | 4 +-- library/src/options.cxx | 4 +-- library/testing/TestSDKOptions.cxx | 47 ++++++++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/library/private/options_tools.h.in b/library/private/options_tools.h.in index a5e8286193..0c4d8f9f6c 100644 --- a/library/private/options_tools.h.in +++ b/library/private/options_tools.h.in @@ -117,7 +117,8 @@ int parse(const std::string& str) } catch (std::out_of_range const&) { - throw options::parsing_exception("Cannot parse " + str + " into an int as it would go out of range"); + throw options::parsing_exception( + "Cannot parse " + str + " into an int as it would go out of range"); } } diff --git a/library/public/options.h.in b/library/public/options.h.in index ced6fe2423..682479170c 100644 --- a/library/public/options.h.in +++ b/library/public/options.h.in @@ -124,13 +124,13 @@ public: * Resets the option to default value. * Throws an options::inexistent_exception if option does not exist. */ - void reset(options& opt, const std::string& name); + void reset(const std::string& name); /** * Unset the option if it is optional. * Throws an options::inexistent_exception if option does not exist. */ - void removeValue(options& opt, const std::string& name); + void removeValue(const std::string& name); /** * Templated parsing method used internally to parse strings. diff --git a/library/src/options.cxx b/library/src/options.cxx index ec2c21edc8..f54b115477 100644 --- a/library/src/options.cxx +++ b/library/src/options.cxx @@ -141,12 +141,12 @@ bool options::isOptional(const std::string& option) const return options_tools::isOptional(option); } -void options::reset(options& opt, const std::string& name) +void options::reset(const std::string& name) { options_tools::reset(*this, name); } -void options::removeValue(options& opt, const std::string& name) +void options::removeValue(const std::string& name) { options_tools::removeValue(*this, name); } diff --git a/library/testing/TestSDKOptions.cxx b/library/testing/TestSDKOptions.cxx index 6d2213aed4..162e8196ee 100644 --- a/library/testing/TestSDKOptions.cxx +++ b/library/testing/TestSDKOptions.cxx @@ -195,5 +195,52 @@ int TestSDKOptions(int argc, char* argv[]) test.expect("no_value_exception exception on getAsString", [&]() { opt.getAsString("scene.animation.time"); }); + f3d::options opt6{}; + + // Test isOptional optional values + test("isOptional with optional value", opt6.isOptional("model.scivis.array_name")); + test("isOptional with optional value", opt6.isOptional("model.scivis.range")); + + // Test isOptional non-optional values + test("isOptional with non-optional value", opt6.isOptional("model.scivis.cells") == false); + test("isOptional with non-optional value", opt6.isOptional("model.scivis.enable") == false); + + // Test isOptional non-existent options + test.expect( + "isOptional with non-existent option", [&]() { opt6.isOptional("dummy"); }); + + f3d::options opt7{}; + + // Test reset non-optional values + opt7.model.color.opacity = 0.5; + opt7.reset("model.color.opacity"); + test("reset non-optional values", opt7.model.color.opacity == 1.0); + + // Test reset optional values + opt7.model.scivis.array_name = "dummy"; + opt7.reset("model.scivis.array_name"); + test.expect( + "reset non-optional values", [&]() { opt7.get("model.scivis.array_name"); }); + + // Test reset non-existent option + test.expect( + "reset with non-existent option", [&]() { opt7.reset("dummy"); }); + + f3d::options opt8{}; + + // Test removeValue optional values + opt8.model.scivis.array_name = "dummy"; + opt8.removeValue("model.scivis.array_name"); + test.expect( + "removeValue optional values", [&]() { opt8.get("model.scivis.array_name"); }); + + // Test removeValue non-optional values + test.expect( + "removeValue non-optional values", [&]() { opt8.removeValue("model.scivis.cells"); }); + + // Test removeValue non-optional values + test.expect( + "removeValue non-existent option", [&]() { opt8.removeValue("dummy"); }); + return EXIT_SUCCESS; } From 945c9da8365b79f2ce91ab35d12a58d82d384f1a Mon Sep 17 00:00:00 2001 From: Yogesh Rathod Date: Sun, 29 Sep 2024 20:07:56 +0530 Subject: [PATCH 6/7] Added comments and removed options_tools::removeValue --- library/private/options_tools.h.in | 18 +----------------- library/public/options.h.in | 2 +- library/src/options.cxx | 12 +++++++++++- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/library/private/options_tools.h.in b/library/private/options_tools.h.in index 0c4d8f9f6c..221dc674b9 100644 --- a/library/private/options_tools.h.in +++ b/library/private/options_tools.h.in @@ -363,7 +363,7 @@ bool isOptional(const std::string& name) //---------------------------------------------------------------------------- /** - * Generated method, see `options::isOptional` + * Generated method, see `options::reset` */ void reset(options& opt, const std::string& name) { @@ -373,22 +373,6 @@ void reset(options& opt, const std::string& name) else throw options::inexistent_exception("Option " + name + " does not exist"); } -//---------------------------------------------------------------------------- -/** - * Generated method, see `options::removeValue` - */ -void removeValue(options& opt, const std::string& name) -{ - if (isOptional(name)) - { - reset(opt, name); - } - else - { - throw options::incompatible_exception("Option " + name + " is not not optional"); - } -} - } // option_tools } // f3d #endif // f3d_options_tools_h diff --git a/library/public/options.h.in b/library/public/options.h.in index 682479170c..ab600d749b 100644 --- a/library/public/options.h.in +++ b/library/public/options.h.in @@ -127,7 +127,7 @@ public: void reset(const std::string& name); /** - * Unset the option if it is optional. + * Unset the option if it is optional else throws options::incompatible_exception. * Throws an options::inexistent_exception if option does not exist. */ void removeValue(const std::string& name); diff --git a/library/src/options.cxx b/library/src/options.cxx index f54b115477..fb33c0685d 100644 --- a/library/src/options.cxx +++ b/library/src/options.cxx @@ -136,19 +136,29 @@ std::pair options::getClosestOption(const std::string return ret; } +//---------------------------------------------------------------------------- bool options::isOptional(const std::string& option) const { return options_tools::isOptional(option); } +//---------------------------------------------------------------------------- void options::reset(const std::string& name) { options_tools::reset(*this, name); } +//---------------------------------------------------------------------------- void options::removeValue(const std::string& name) { - options_tools::removeValue(*this, name); + if (isOptional(name)) + { + reset(name); + } + else + { + throw options::incompatible_exception("Option " + name + " is not not optional"); + } } //---------------------------------------------------------------------------- From f29d0f3d83277de882aa1ff708471eee4fee224c Mon Sep 17 00:00:00 2001 From: Yogesh Rathod Date: Sun, 29 Sep 2024 22:23:49 +0530 Subject: [PATCH 7/7] refactored options_tool::removeValue --- library/src/options.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/src/options.cxx b/library/src/options.cxx index fb33c0685d..23174d6799 100644 --- a/library/src/options.cxx +++ b/library/src/options.cxx @@ -151,9 +151,9 @@ void options::reset(const std::string& name) //---------------------------------------------------------------------------- void options::removeValue(const std::string& name) { - if (isOptional(name)) + if (this->isOptional(name)) { - reset(name); + this->reset(name); } else {