Fix invalid callback calls for default_val Option function. the updat…

…e adds a flag variable to control it, makes default_val exception safe and a template to convert from actual value types.

include/CLI/App.hpp

@@ -601,12 +601,13 @@ class App {
             return CLI::detail::checked_to_string<AssignTo, ConvertTo>(variable);
         });
         opt->type_name(detail::type_name<ConvertTo>());
-        // these must be actual variables since (std::max) sometimes is defined in terms of references and references
+        // these must be actual lvalues since (std::max) sometimes is defined in terms of references and references
         // to structs used in the evaluation can be temporary so that would cause issues.
         auto Tcount = detail::type_count<AssignTo>::value;
         auto XCcount = detail::type_count<ConvertTo>::value;
         opt->type_size((std::max)(Tcount, XCcount));
         opt->expected(detail::expected_count<ConvertTo>::value);
+        opt->run_callback_for_default();
         return opt;
     }
 
@@ -754,7 +755,7 @@ class App {
         CLI::callback_t fun = [&flag_result](const CLI::results_t &res) {
             return CLI::detail::lexical_cast(res[0], flag_result);
         };
-        return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description));
+        return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description))->run_callback_for_default();
     }
 
     /// Vector version to capture multiple flags.
@@ -772,7 +773,8 @@ class App {
             return retval;
         };
         return _add_flag_internal(flag_name, std::move(fun), std::move(flag_description))
-            ->multi_option_policy(MultiOptionPolicy::TakeAll);
+            ->multi_option_policy(MultiOptionPolicy::TakeAll)
+            ->run_callback_for_default();
     }
 
     /// Add option for callback that is triggered with a true flag and takes no arguments
@@ -911,7 +913,7 @@ class App {
         CLI::Option *opt =
             add_option(option_name, std::move(fun), std::move(option_description), defaulted, default_function);
 
-        opt->type_name(label)->type_size(1, 2)->delimiter('+');
+        opt->type_name(label)->type_size(1, 2)->delimiter('+')->run_callback_for_default();
         return opt;
     }
 

include/CLI/Option.hpp

@@ -324,6 +324,8 @@ class Option : public OptionBase<Option> {
     bool allow_extra_args_{false};
     /// Specify that the option should act like a flag vs regular option
     bool flag_like_{false};
+    /// Control option to run the callback to set the default
+    bool run_callback_for_default_{false};
     ///@}
 
     /// 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> {
     /// Get the current value of allow extra args
     bool get_allow_extra_args() const { return allow_extra_args_; }
 
+    /// Set the value of run_callback_for_default which controls whether the callback function should be called to set
+    /// the default This is controlled automatically but could be manipulated by the user.
+    Option *run_callback_for_default(bool value = true) {
+        run_callback_for_default_ = value;
+        return this;
+    }
+    /// Get the current value of run_callback_for_default
+    bool get_run_callback_for_default() const { return run_callback_for_default_; }
+
     /// Adds a Validator with a built in type name
     Option *check(Validator validator, const std::string &validator_name = "") {
         validator.non_modifying();
@@ -1066,15 +1077,31 @@ class Option : public OptionBase<Option> {
         return this;
     }
 
-    /// Set the default value string representation and evaluate into the bound value
-    Option *default_val(const std::string &val) {
-        default_str(val);
-        auto old_results = results_;
+    /// Set the default value and validate the results and run the callback if appropriate to set the value into the
+    /// bound value only available for types that can be converted to a string
+    template <typename X> Option *default_val(const X &val) {
+        std::string val_str = detail::to_string(val);
+        auto old_option_state = current_option_state_;
+        auto old_results{std::move(results_)};
         results_.clear();
-        add_result(val);
-        run_callback();
+        try {
+            add_result(val_str);
+            if(run_callback_for_default_ && !val_str.empty()) {
+                current_option_state_ = option_state::parsing;
+                run_callback(); // run callback sets the state we need to reset it again
+                current_option_state_ = option_state::parsing;
+            } else {
+                _validate_results(results_);
+                current_option_state_ = old_option_state;
+            }
+        } catch(const CLI::Error &) {
+            // this should be done
+            results_ = std::move(old_results);
+            current_option_state_ = old_option_state;
+            throw;
+        }
         results_ = std::move(old_results);
-        current_option_state_ = option_state::parsing;
+        default_str_ = std::move(val_str);
         return this;
     }
 

include/CLI/TypeTools.hpp

@@ -46,6 +46,9 @@ template <typename T> struct is_vector : std::false_type {};
 /// Check to see if something is a vector (true if actually a vector)
 template <class T, class A> struct is_vector<std::vector<T, A>> : std::true_type {};
 
+/// Check to see if something is a vector (true if actually a const vector)
+template <class T, class A> struct is_vector<const std::vector<T, A>> : std::true_type {};
+
 /// Check to see if something is bool (fail check by default)
 template <typename T> struct is_bool : std::false_type {};
 

tests/AppTest.cpp

@@ -425,6 +425,10 @@ TEST_F(TApp, OneIntFlagLike) {
     opt->default_str("7");
     run();
     EXPECT_EQ(val, 7);
+
+    opt->default_val(9);
+    run();
+    EXPECT_EQ(val, 9);
 }
 
 TEST_F(TApp, TogetherInt) {
@@ -514,6 +518,33 @@ TEST_F(TApp, doubleVectorFunctionFail) {
     EXPECT_EQ(strvec.size(), 3u);
 }
 
+TEST_F(TApp, doubleVectorFunctionRunCallbackOnDefault) {
+    std::vector<double> res;
+    auto opt = app.add_option_function<std::vector<double>>("--val", [&res](const std::vector<double> &val) {
+        res = val;
+        std::transform(res.begin(), res.end(), res.begin(), [](double v) { return v + 5.0; });
+    });
+    args = {"--val", "5", "--val", "6", "--val", "7"};
+    run();
+    EXPECT_EQ(res.size(), 3u);
+    EXPECT_EQ(res[0], 10.0);
+    EXPECT_EQ(res[2], 12.0);
+    EXPECT_FALSE(opt->get_run_callback_for_default());
+    opt->run_callback_for_default();
+    opt->default_val(std::vector<int>{2, 1, -2});
+    EXPECT_EQ(res[0], 7.0);
+    EXPECT_EQ(res[2], 3.0);
+
+    EXPECT_THROW(opt->default_val("this is a string"), CLI::ConversionError);
+    auto vec = opt->as<std::vector<double>>();
+    ASSERT_EQ(vec.size(), 3U);
+    EXPECT_EQ(vec[0], 5.0);
+    EXPECT_EQ(vec[2], 7.0);
+    opt->check(CLI::Number);
+    opt->run_callback_for_default(false);
+    EXPECT_THROW(opt->default_val("this is a string"), CLI::ValidationError);
+}
+
 TEST_F(TApp, DefaultStringAgain) {
     std::string str = "previous";
     app.add_option("-s,--string", str);

tests/HelpTest.cpp

@@ -397,6 +397,21 @@ TEST(THelp, ManualSetters) {
     EXPECT_EQ(x, 14);
     help = app.help();
     EXPECT_THAT(help, HasSubstr("=14"));
+
+    op1->default_val(12);
+    EXPECT_EQ(x, 12);
+    help = app.help();
+    EXPECT_THAT(help, HasSubstr("=12"));
+
+    EXPECT_TRUE(op1->get_run_callback_for_default());
+    op1->run_callback_for_default(false);
+    EXPECT_FALSE(op1->get_run_callback_for_default());
+
+    op1->default_val(18);
+    // x should not be modified in this case
+    EXPECT_EQ(x, 12);
+    help = app.help();
+    EXPECT_THAT(help, HasSubstr("=18"));
 }
 
 TEST(THelp, ManualSetterOverFunction) {