Commit 5b17abf22ff42aaa94c7399e4cae022470b48d43
Committed by
Henry Schreiner
1 parent
c67ab9dd
default_val option call (#387)
* Fix invalid callback calls for default_val Option function. the update adds a flag variable to control it, makes default_val exception safe and a template to convert from actual value types. * update readme and fix some compilation issues on older compilers * revert README.md with mistake erasures * Update README.md Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
Showing
6 changed files
with
90 additions
and
12 deletions
README.md
| ... | ... | @@ -331,6 +331,8 @@ Before parsing, you can set the following options: |
| 331 | 331 | `->capture_default_str()`: ๐ Store the current value attached and display it in the help string. |
| 332 | 332 | - `->default_function(std::string())`: ๐ Advanced: Change the function that `capture_default_str()` uses. |
| 333 | 333 | - `->always_capture_default()`: ๐ Always run `capture_default_str()` when creating new options. Only useful on an App's `option_defaults`. |
| 334 | +- `default_str(string)`: Set the default string directly. This string will also be used as a default value if no arguments are passed and the value is requested. | |
| 335 | +- `default_val(value)`: ๐ง Generate the default string from a value and validate that the value is also valid. For options that assign directly to a value type the value in that type is also updated. Value must be convertible to a string(one of known types or a stream operator). | |
| 334 | 336 | |
| 335 | 337 | |
| 336 | 338 | These options return the `Option` pointer, so you can chain them together, and even skip storing the pointer entirely. The `each` function takes any function that has the signature `void(const std::string&)`; it should throw a `ValidationError` when validation fails. The help message will have the name of the parent option prepended. Since `each`, `check` and `transform` use the same underlying mechanism, you can chain as many as you want, and they will be executed in order. Operations added through `transform` are executed first in reverse order of addition, and `check` and `each` are run following the transform functions in order of addition. If you just want to see the unconverted values, use `.results()` to get the `std::vector<std::string>` of results. |
| ... | ... | @@ -990,4 +992,3 @@ CLI11 was developed at the [University of Cincinnati][] to support of the [GooFi |
| 990 | 992 | [hunter]: https://docs.hunter.sh/en/latest/packages/pkg/CLI11.html |
| 991 | 993 | [standard readme style]: https://github.com/RichardLitt/standard-readme |
| 992 | 994 | [argparse]: https://github.com/p-ranav/argparse |
| 993 | - | ... | ... |
include/CLI/App.hpp
| ... | ... | @@ -601,12 +601,13 @@ class App { |
| 601 | 601 | return CLI::detail::checked_to_string<AssignTo, ConvertTo>(variable); |
| 602 | 602 | }); |
| 603 | 603 | opt->type_name(detail::type_name<ConvertTo>()); |
| 604 | - // these must be actual variables since (std::max) sometimes is defined in terms of references and references | |
| 604 | + // these must be actual lvalues since (std::max) sometimes is defined in terms of references and references | |
| 605 | 605 | // to structs used in the evaluation can be temporary so that would cause issues. |
| 606 | 606 | auto Tcount = detail::type_count<AssignTo>::value; |
| 607 | 607 | auto XCcount = detail::type_count<ConvertTo>::value; |
| 608 | 608 | opt->type_size((std::max)(Tcount, XCcount)); |
| 609 | 609 | opt->expected(detail::expected_count<ConvertTo>::value); |
| 610 | + opt->run_callback_for_default(); | |
| 610 | 611 | return opt; |
| 611 | 612 | } |
| 612 | 613 | |
| ... | ... | @@ -754,7 +755,7 @@ class App { |
| 754 | 755 | CLI::callback_t fun = [&flag_result](const CLI::results_t &res) { |
| 755 | 756 | return CLI::detail::lexical_cast(res[0], flag_result); |
| 756 | 757 | }; |
| 757 | - return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description)); | |
| 758 | + return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description))->run_callback_for_default(); | |
| 758 | 759 | } |
| 759 | 760 | |
| 760 | 761 | /// Vector version to capture multiple flags. |
| ... | ... | @@ -772,7 +773,8 @@ class App { |
| 772 | 773 | return retval; |
| 773 | 774 | }; |
| 774 | 775 | return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description)) |
| 775 | - ->multi_option_policy(MultiOptionPolicy::TakeAll); | |
| 776 | + ->multi_option_policy(MultiOptionPolicy::TakeAll) | |
| 777 | + ->run_callback_for_default(); | |
| 776 | 778 | } |
| 777 | 779 | |
| 778 | 780 | /// Add option for callback that is triggered with a true flag and takes no arguments |
| ... | ... | @@ -911,7 +913,7 @@ class App { |
| 911 | 913 | CLI::Option *opt = |
| 912 | 914 | add_option(option_name, std::move(fun), std::move(option_description), defaulted, default_function); |
| 913 | 915 | |
| 914 | - opt->type_name(label)->type_size(1, 2)->delimiter('+'); | |
| 916 | + opt->type_name(label)->type_size(1, 2)->delimiter('+')->run_callback_for_default(); | |
| 915 | 917 | return opt; |
| 916 | 918 | } |
| 917 | 919 | ... | ... |
include/CLI/Option.hpp
| ... | ... | @@ -324,6 +324,8 @@ class Option : public OptionBase<Option> { |
| 324 | 324 | bool allow_extra_args_{false}; |
| 325 | 325 | /// Specify that the option should act like a flag vs regular option |
| 326 | 326 | bool flag_like_{false}; |
| 327 | + /// Control option to run the callback to set the default | |
| 328 | + bool run_callback_for_default_{false}; | |
| 327 | 329 | ///@} |
| 328 | 330 | |
| 329 | 331 | /// Making an option by hand is not defined, it must be made by the App class |
| ... | ... | @@ -408,6 +410,15 @@ class Option : public OptionBase<Option> { |
| 408 | 410 | /// Get the current value of allow extra args |
| 409 | 411 | bool get_allow_extra_args() const { return allow_extra_args_; } |
| 410 | 412 | |
| 413 | + /// Set the value of run_callback_for_default which controls whether the callback function should be called to set | |
| 414 | + /// the default This is controlled automatically but could be manipulated by the user. | |
| 415 | + Option *run_callback_for_default(bool value = true) { | |
| 416 | + run_callback_for_default_ = value; | |
| 417 | + return this; | |
| 418 | + } | |
| 419 | + /// Get the current value of run_callback_for_default | |
| 420 | + bool get_run_callback_for_default() const { return run_callback_for_default_; } | |
| 421 | + | |
| 411 | 422 | /// Adds a Validator with a built in type name |
| 412 | 423 | Option *check(Validator validator, const std::string &validator_name = "") { |
| 413 | 424 | validator.non_modifying(); |
| ... | ... | @@ -1066,15 +1077,30 @@ class Option : public OptionBase<Option> { |
| 1066 | 1077 | return this; |
| 1067 | 1078 | } |
| 1068 | 1079 | |
| 1069 | - /// Set the default value string representation and evaluate into the bound value | |
| 1070 | - Option *default_val(const std::string &val) { | |
| 1071 | - default_str(val); | |
| 1072 | - auto old_results = results_; | |
| 1080 | + /// Set the default value and validate the results and run the callback if appropriate to set the value into the | |
| 1081 | + /// bound value only available for types that can be converted to a string | |
| 1082 | + template <typename X> Option *default_val(const X &val) { | |
| 1083 | + std::string val_str = detail::to_string(val); | |
| 1084 | + auto old_option_state = current_option_state_; | |
| 1085 | + results_t old_results{std::move(results_)}; | |
| 1073 | 1086 | results_.clear(); |
| 1074 | - add_result(val); | |
| 1075 | - run_callback(); | |
| 1087 | + try { | |
| 1088 | + add_result(val_str); | |
| 1089 | + if(run_callback_for_default_) { | |
| 1090 | + run_callback(); // run callback sets the state we need to reset it again | |
| 1091 | + current_option_state_ = option_state::parsing; | |
| 1092 | + } else { | |
| 1093 | + _validate_results(results_); | |
| 1094 | + current_option_state_ = old_option_state; | |
| 1095 | + } | |
| 1096 | + } catch(const CLI::Error &) { | |
| 1097 | + // this should be done | |
| 1098 | + results_ = std::move(old_results); | |
| 1099 | + current_option_state_ = old_option_state; | |
| 1100 | + throw; | |
| 1101 | + } | |
| 1076 | 1102 | results_ = std::move(old_results); |
| 1077 | - current_option_state_ = option_state::parsing; | |
| 1103 | + default_str_ = std::move(val_str); | |
| 1078 | 1104 | return this; |
| 1079 | 1105 | } |
| 1080 | 1106 | ... | ... |
include/CLI/TypeTools.hpp
| ... | ... | @@ -46,6 +46,9 @@ template <typename T> struct is_vector : std::false_type {}; |
| 46 | 46 | /// Check to see if something is a vector (true if actually a vector) |
| 47 | 47 | template <class T, class A> struct is_vector<std::vector<T, A>> : std::true_type {}; |
| 48 | 48 | |
| 49 | +/// Check to see if something is a vector (true if actually a const vector) | |
| 50 | +template <class T, class A> struct is_vector<const std::vector<T, A>> : std::true_type {}; | |
| 51 | + | |
| 49 | 52 | /// Check to see if something is bool (fail check by default) |
| 50 | 53 | template <typename T> struct is_bool : std::false_type {}; |
| 51 | 54 | ... | ... |
tests/AppTest.cpp
| ... | ... | @@ -425,6 +425,10 @@ TEST_F(TApp, OneIntFlagLike) { |
| 425 | 425 | opt->default_str("7"); |
| 426 | 426 | run(); |
| 427 | 427 | EXPECT_EQ(val, 7); |
| 428 | + | |
| 429 | + opt->default_val(9); | |
| 430 | + run(); | |
| 431 | + EXPECT_EQ(val, 9); | |
| 428 | 432 | } |
| 429 | 433 | |
| 430 | 434 | TEST_F(TApp, TogetherInt) { |
| ... | ... | @@ -514,6 +518,33 @@ TEST_F(TApp, doubleVectorFunctionFail) { |
| 514 | 518 | EXPECT_EQ(strvec.size(), 3u); |
| 515 | 519 | } |
| 516 | 520 | |
| 521 | +TEST_F(TApp, doubleVectorFunctionRunCallbackOnDefault) { | |
| 522 | + std::vector<double> res; | |
| 523 | + auto opt = app.add_option_function<std::vector<double>>("--val", [&res](const std::vector<double> &val) { | |
| 524 | + res = val; | |
| 525 | + std::transform(res.begin(), res.end(), res.begin(), [](double v) { return v + 5.0; }); | |
| 526 | + }); | |
| 527 | + args = {"--val", "5", "--val", "6", "--val", "7"}; | |
| 528 | + run(); | |
| 529 | + EXPECT_EQ(res.size(), 3u); | |
| 530 | + EXPECT_EQ(res[0], 10.0); | |
| 531 | + EXPECT_EQ(res[2], 12.0); | |
| 532 | + EXPECT_FALSE(opt->get_run_callback_for_default()); | |
| 533 | + opt->run_callback_for_default(); | |
| 534 | + opt->default_val(std::vector<int>{2, 1, -2}); | |
| 535 | + EXPECT_EQ(res[0], 7.0); | |
| 536 | + EXPECT_EQ(res[2], 3.0); | |
| 537 | + | |
| 538 | + EXPECT_THROW(opt->default_val("this is a string"), CLI::ConversionError); | |
| 539 | + auto vec = opt->as<std::vector<double>>(); | |
| 540 | + ASSERT_EQ(vec.size(), 3U); | |
| 541 | + EXPECT_EQ(vec[0], 5.0); | |
| 542 | + EXPECT_EQ(vec[2], 7.0); | |
| 543 | + opt->check(CLI::Number); | |
| 544 | + opt->run_callback_for_default(false); | |
| 545 | + EXPECT_THROW(opt->default_val("this is a string"), CLI::ValidationError); | |
| 546 | +} | |
| 547 | + | |
| 517 | 548 | TEST_F(TApp, DefaultStringAgain) { |
| 518 | 549 | std::string str = "previous"; |
| 519 | 550 | app.add_option("-s,--string", str); | ... | ... |
tests/HelpTest.cpp
| ... | ... | @@ -397,6 +397,21 @@ TEST(THelp, ManualSetters) { |
| 397 | 397 | EXPECT_EQ(x, 14); |
| 398 | 398 | help = app.help(); |
| 399 | 399 | EXPECT_THAT(help, HasSubstr("=14")); |
| 400 | + | |
| 401 | + op1->default_val(12); | |
| 402 | + EXPECT_EQ(x, 12); | |
| 403 | + help = app.help(); | |
| 404 | + EXPECT_THAT(help, HasSubstr("=12")); | |
| 405 | + | |
| 406 | + EXPECT_TRUE(op1->get_run_callback_for_default()); | |
| 407 | + op1->run_callback_for_default(false); | |
| 408 | + EXPECT_FALSE(op1->get_run_callback_for_default()); | |
| 409 | + | |
| 410 | + op1->default_val(18); | |
| 411 | + // x should not be modified in this case | |
| 412 | + EXPECT_EQ(x, 12); | |
| 413 | + help = app.help(); | |
| 414 | + EXPECT_THAT(help, HasSubstr("=18")); | |
| 400 | 415 | } |
| 401 | 416 | |
| 402 | 417 | TEST(THelp, ManualSetterOverFunction) { | ... | ... |