Store option commodities and namespace and mnemonic instead of pointer.

Protects against crashes caused by the user deleting the commodity and
allows the option to work if a deleted commodity is recreated.

libgnucash/app-utils/gnc-option-impl.cpp

@@ -127,47 +127,18 @@ GncOptionQofInstanceValue::deserialize(const std::string& str) noexcept
 {
     QofInstance* inst{};
     // Commodities are often serialized as Namespace::Mnemonic or just Mnemonic
-    if (m_ui_type == GncOptionUIType::CURRENCY ||
+    try {
-        m_ui_type == GncOptionUIType::COMMODITY)
+        auto guid{static_cast<GncGUID>(gnc::GUID::from_string(str))};
-    {
+        inst = qof_instance_from_guid(&guid, m_ui_type);
-        auto book{gnc_get_current_book()};
-        auto table = gnc_commodity_table_get_table(book);
-        auto sep{str.find(":")};
-        if (sep != std::string::npos)
-        {
-            auto name_space{str.substr(0, sep)};
-            auto mnemonic{str.substr(sep + 1, -1)};
-            inst = QOF_INSTANCE(gnc_commodity_table_lookup(table,
-                                                              name_space.c_str(),
-                                                              mnemonic.c_str()));
-        }
-        if (!inst && m_ui_type == GncOptionUIType::CURRENCY)
-            inst = QOF_INSTANCE(gnc_commodity_table_lookup(table,
-                                                             "CURRENCY",
-                                                             str.c_str()));
         if (inst)
         {
             m_value = make_gnc_item(inst);
             return true;
         }
     }
-
+    catch (const gnc::guid_syntax_exception& err)
-    if (!inst)
     {
-        try {
+        PWARN("Failed to convert %s to a GUID", str.c_str());
-            auto guid{static_cast<GncGUID>(gnc::GUID::from_string(str))};
-            inst = qof_instance_from_guid(&guid, m_ui_type);
-            if (inst)
-            {
-                auto coll{qof_instance_get_collection(inst)};
-                m_value = std::make_pair(qof_collection_get_type(coll), guid);
-                return true;
-            }
-        }
-        catch (const gnc::guid_syntax_exception& err)
-        {
-            PWARN("Failed to convert %s to a GUID", str.c_str());
-        }
     }
     return false;
 }
@@ -200,6 +171,103 @@ GncOptionQofInstanceValue::serialize() const noexcept
     return retval;
 }
 
+static gnc_commodity*
+gnc_commodity_from_namespace_and_mnemonic(std::string_view name_space,
+                                          std::string_view mnemonic)
+{
+    auto book{gnc_get_current_book()};
+    auto table = gnc_commodity_table_get_table(book);
+    return gnc_commodity_table_lookup(table, name_space.data(),
+                                      mnemonic.data());
+}
+
+gnc_commodity*
+GncOptionCommodityValue::get_value() const
+{
+    return gnc_commodity_from_namespace_and_mnemonic(m_namespace, m_mnemonic);
+}
+
+gnc_commodity*
+GncOptionCommodityValue::get_default_value() const
+{
+    return gnc_commodity_from_namespace_and_mnemonic(m_default_namespace,
+                                                     m_default_mnemonic);
+}
+
+void
+GncOptionCommodityValue::set_value(gnc_commodity* value)
+{
+    if (!validate(value))
+        throw std::invalid_argument("Value not a currency when required or not a commodity. Value not set.");
+    m_mnemonic = gnc_commodity_get_mnemonic(value);
+    m_namespace = gnc_commodity_get_namespace(value);
+}
+
+void
+GncOptionCommodityValue::set_default_value(gnc_commodity* value)
+{
+    if (!validate(value))
+        throw std::invalid_argument("Value not a currency when required or not a commodity. Value not set.");
+    m_mnemonic = m_default_mnemonic = gnc_commodity_get_mnemonic(value);
+    m_namespace = m_default_namespace = gnc_commodity_get_namespace(value);
+}
+
+void
+GncOptionCommodityValue::reset_default_value()
+{
+    m_mnemonic = m_default_mnemonic;
+    m_namespace = m_default_namespace;
+}
+
+bool
+GncOptionCommodityValue::is_changed() const noexcept
+{
+    return m_namespace == m_default_namespace && m_mnemonic == m_default_mnemonic;
+}
+
+bool
+GncOptionCommodityValue::validate(gnc_commodity* comm) const noexcept
+{
+    if (!GNC_IS_COMMODITY(comm))
+        return false;
+    if (m_is_currency && !gnc_commodity_is_currency(comm))
+        return false;
+    return true;
+}
+
+std::string
+GncOptionCommodityValue::serialize() const noexcept
+{
+    if (m_is_currency)
+        return m_mnemonic;
+    else
+        return m_namespace + ":" + m_mnemonic;
+}
+
+bool
+GncOptionCommodityValue::deserialize(const std::string& str) noexcept
+{
+   auto sep{str.find(":")};
+    gnc_commodity* comm{};
+    std::string mnemonic, name_space;
+    if (sep != std::string::npos)
+    {
+        name_space = str.substr(0, sep);
+        mnemonic = str.substr(sep + 1, -1);
+    }
+    else
+    {
+        name_space = "CURRENCY";
+        mnemonic = str;
+    }
+    comm = gnc_commodity_from_namespace_and_mnemonic(name_space, mnemonic);
+    if (!validate(comm))
+        return false;
+    m_namespace = std::move(name_space);
+    m_mnemonic = std::move(mnemonic);
+    return true;
+}
+
 bool
 GncOptionAccountListValue::validate(const GncOptionAccountList& values) const
 {
@@ -470,10 +538,6 @@ qof_instance_from_guid(GncGUID* guid, GncOptionUIType type)
     QofIdType qof_type;
     switch(type)
     {
-        case GncOptionUIType::CURRENCY:
-        case GncOptionUIType::COMMODITY:
-            qof_type = "Commodity";
-            break;
         case GncOptionUIType::BUDGET:
             qof_type = "Budget";
             break;
@@ -510,37 +574,13 @@ QofInstance*
 qof_instance_from_string(const std::string& str, GncOptionUIType type)
 {
     QofInstance* retval{nullptr};
-    // Commodities are often serialized as Namespace::Mnemonic or just Mnemonic
+    try {
-    if (type == GncOptionUIType::CURRENCY ||
+        auto guid{static_cast<GncGUID>(gnc::GUID::from_string(str))};
-        type == GncOptionUIType::COMMODITY)
+        retval = qof_instance_from_guid(&guid, type);
-    {
-        auto book{gnc_get_current_book()};
-        auto table = gnc_commodity_table_get_table(book);
-        auto sep{str.find(":")};
-        if (sep != std::string::npos)
-        {
-            auto name_space{str.substr(0, sep)};
-            auto mnemonic{str.substr(sep + 1, -1)};
-            retval = QOF_INSTANCE(gnc_commodity_table_lookup(table,
-                                                             name_space.c_str(),
-                                                             mnemonic.c_str()));
-        }
-        if (!retval && type == GncOptionUIType::CURRENCY)
-            retval = QOF_INSTANCE(gnc_commodity_table_lookup(table,
-                                                             "CURRENCY",
-                                                             str.c_str()));
     }
-
+    catch (const gnc::guid_syntax_exception& err)
-    if (!retval)
     {
-        try {
+        PWARN("Failed to convert %s to a GUID", str.c_str());
-            auto guid{static_cast<GncGUID>(gnc::GUID::from_string(str))};
-            retval = qof_instance_from_guid(&guid, type);
-        }
-        catch (const gnc::guid_syntax_exception& err)
-        {
-            PWARN("Failed to convert %s to a GUID", str.c_str());
-        }
     }
     return retval;
 }
@@ -549,26 +589,8 @@ std::string
 qof_instance_to_string(const QofInstance* inst)
 {
     std::string retval;
-    if (GNC_IS_COMMODITY(inst))
+    gnc::GUID guid{*qof_instance_get_guid(inst)};
-    {
+    retval = guid.to_string();
-        auto commodity{GNC_COMMODITY(inst)};
-        if (!gnc_commodity_is_currency(commodity))
-        {
-            auto name_space{gnc_commodity_get_namespace(GNC_COMMODITY(inst))};
-            if (name_space && *name_space != '\0')
-            {
-                retval = name_space;
-                retval += ":";
-            }
-        }
-        retval += gnc_commodity_get_mnemonic(GNC_COMMODITY(inst));
-        return retval;
-    }
-    else
-    {
-        gnc::GUID guid{*qof_instance_get_guid(inst)};
-        retval = guid.to_string();
-    }
     return retval;
 }
 
@@ -678,41 +700,6 @@ GncOptionValue<SCM>::reset_default_value()
     m_value = m_default_value;
     scm_gc_protect_object(m_value);
 }
-
-template <typename ValueType> std::string
-GncOptionValidatedValue<ValueType>::serialize() const noexcept
-{
-    static const std::string no_value{"No Value"};
-    if constexpr(std::is_same_v<ValueType, const QofInstance*>)
-        return m_value ? qof_instance_to_string(m_value) : no_value;
-    else if constexpr(is_same_decayed_v<ValueType, std::string>)
-        return m_value;
-    else if constexpr(is_same_decayed_v<ValueType, bool>)
-        return m_value ? "True" : "False";
-    else if constexpr(std::is_arithmetic_v<ValueType>)
-        return std::to_string(m_value);
-    else
-        return "Invalid Value Type";
-}
-
-template <typename ValueType> bool
-GncOptionValidatedValue<ValueType>::deserialize(const std::string& str) noexcept
-{
-    if constexpr(is_same_decayed_v<ValueType, std::string>)
-        set_value(str);
-    else if constexpr(is_same_decayed_v<ValueType, bool>)
-        set_value(str == "True");
-    else if constexpr(is_same_decayed_v<ValueType, int>)
-        set_value(stoi(str));
-    else if constexpr(is_same_decayed_v<ValueType, int64_t>)
-        set_value(stoll(str));
-    else if constexpr(is_same_decayed_v<ValueType, double>)
-        set_value(stod(str));
-    else
-        return false;
-    return true;
-}
-
 std::string
 GncOptionAccountListValue::serialize() const noexcept
 {
@@ -881,6 +868,16 @@ GncOptionDateValue::deserialize(const std::string& str) noexcept
     }
 }
 
+std::istream&
+operator>> (std::istream& iss, GncOptionCommodityValue& opt)
+{
+    std::string instr;
+    iss >> instr;
+    if (!opt.deserialize(instr))
+        throw std::invalid_argument("Invalid commodity string in stream.");
+    return iss;
+}
+
 template GncOptionValue<bool>::GncOptionValue(const GncOptionValue<bool>&);
 template GncOptionValue<int>::GncOptionValue(const GncOptionValue<int>&);
 template GncOptionValue<int64_t>::GncOptionValue(const GncOptionValue<int64_t>&);
@@ -944,16 +941,6 @@ template std::string GncOptionValue<std::string>::serialize() const noexcept;
 template std::string GncOptionValue<const QofQuery*>::serialize() const noexcept;
 template std::string GncOptionValue<const GncOwner*>::serialize() const noexcept;
 template std::string GncOptionValue<SCM>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<bool>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<int>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<int64_t>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<double>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<char*>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<const char*>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<std::string>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<const QofInstance*>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<const QofQuery*>::serialize() const noexcept;
-template std::string GncOptionValidatedValue<const GncOwner*>::serialize() const noexcept;
 template std::string GncOptionRangeValue<int>::serialize() const noexcept;
 template std::string GncOptionRangeValue<double>::serialize() const noexcept;
 template bool GncOptionValue<bool>::deserialize(const std::string&) noexcept;
@@ -966,15 +953,5 @@ template bool GncOptionValue<std::string>::deserialize(const std::string&) noexc
 template bool GncOptionValue<const QofQuery*>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<const GncOwner*>::deserialize(const std::string&) noexcept;
 template bool GncOptionValue<SCM>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<bool>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<int>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<int64_t>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<double>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<char*>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<const char*>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<std::string>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<const QofInstance*>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<const QofQuery*>::deserialize(const std::string&) noexcept;
-template bool GncOptionValidatedValue<const GncOwner*>::deserialize(const std::string&) noexcept;
 template bool GncOptionRangeValue<int>::deserialize(const std::string&) noexcept;
 template bool GncOptionRangeValue<double>::deserialize(const std::string&) noexcept;

libgnucash/app-utils/gnc-option-impl.hpp

@@ -172,75 +172,54 @@ private:
     GncItem m_default_value;
 };
 
-/** class GncOptionValidatedValue
+/** class GncOptionCommodityValue
- *  Validated values have an additional member function, provided as a
+ * Commodities are stored with their namespace and mnemonic instead of their gncGIUD
- *  constructor argument, that checks value parameters for some property before
+ * so that they can be correctly retrieved even if they're deleted and recreated.
- *  setting the object's value member. If the function returns false a
+ * Additionally if GncOptionCommodityValue is created with GncOptionUIType::CURRENCY
- *  std::invalid_argument exception is thrown.
+ * it will throw std::invalid_argument if one attempts to set a value that isn't a
+ * currency.
  */
-template <typename ValueType>
+
-class GncOptionValidatedValue : public OptionClassifier
+class GncOptionCommodityValue : public OptionClassifier
 {
 public:
-    GncOptionValidatedValue<ValueType>() = delete;
+    GncOptionCommodityValue() = delete;
-    GncOptionValidatedValue<ValueType>(const char* section, const char* name,
+    GncOptionCommodityValue(const char* section, const char* name,
                                        const char* key, const char* doc_string,
-                                       ValueType value,
+                                       gnc_commodity* value,
-                                       std::function<bool(ValueType)>validator,
+                                       GncOptionUIType ui_type = GncOptionUIType::COMMODITY) :
-                                       GncOptionUIType ui_type = GncOptionUIType::INTERNAL
-        ) :
         OptionClassifier{section, name, key, doc_string},
-        m_ui_type{ui_type}, m_value{value}, m_default_value{value},
+        m_ui_type{ui_type}, m_is_currency{ui_type == GncOptionUIType::CURRENCY},
-        m_validator{validator}
+        m_namespace{gnc_commodity_get_namespace(value)},
-        {
+        m_mnemonic{gnc_commodity_get_mnemonic(value)},
-            if (!this->validate(value))
+        m_default_namespace{gnc_commodity_get_namespace(value)},
-            throw std::invalid_argument("Attempt to create GncValidatedOption with bad value.");
+        m_default_mnemonic{gnc_commodity_get_mnemonic(value)}
-        }
-    GncOptionValidatedValue<ValueType>(const char* section, const char* name,
-                                       const char* key, const char* doc_string,
-                                       ValueType value,
-                                       std::function<bool(ValueType)>validator,
-                                       ValueType val_data) :
-        OptionClassifier{section, name, key, doc_string},
-        m_ui_type{GncOptionUIType::INTERNAL}, m_value{value},
-        m_default_value{value}, m_validator{validator}, m_validation_data{val_data}
     {
-            if (!this->validate(value))
+       if (!validate(value))
-            throw std::invalid_argument("Attempt to create GncValidatedOption with bad value.");
+            throw std::invalid_argument("Attempt to create GncOptionCommodityValue with currency UIType and non-currency value.");
     }
-    GncOptionValidatedValue<ValueType>(const GncOptionValidatedValue<ValueType>&) = default;
+    GncOptionCommodityValue(const GncOptionCommodityValue&) = default;
-    GncOptionValidatedValue<ValueType>(GncOptionValidatedValue<ValueType>&&) = default;
+    GncOptionCommodityValue(GncOptionCommodityValue&&) = default;
-    GncOptionValidatedValue<ValueType>& operator=(const GncOptionValidatedValue<ValueType>&) = default;
+    GncOptionCommodityValue& operator=(const GncOptionCommodityValue&) = default;
-    GncOptionValidatedValue<ValueType>& operator=(GncOptionValidatedValue<ValueType>&&) = default;
+    GncOptionCommodityValue& operator=(GncOptionCommodityValue&&) = default;
-    ValueType get_value() const { return m_value; }
+    gnc_commodity* get_value() const;
-    ValueType get_default_value() const { return m_default_value; }
+    gnc_commodity* get_default_value() const;
-    bool validate(ValueType value) const { return m_validator(value); }
+    bool validate(gnc_commodity*) const noexcept;
-    void set_value(ValueType value)
+    void set_value(gnc_commodity* value);
-    {
+    void set_default_value(gnc_commodity* value);
-        if (this->validate(value))
+    void reset_default_value();
-            m_value = value;
+    bool is_changed() const noexcept;
-        else
-            throw std::invalid_argument("Validation failed, value not set.");
-    }
-    void set_default_value(ValueType value)
-    {
-        if (this->validate(value))
-            m_value = m_default_value = value;
-        else
-            throw std::invalid_argument("Validation failed, value not set.");
-    }
-    void reset_default_value() { m_value = m_default_value; }
-    bool is_changed() const noexcept { return m_value != m_default_value; }
     GncOptionUIType get_ui_type() const noexcept { return m_ui_type; }
     void make_internal() { m_ui_type = GncOptionUIType::INTERNAL; }
     std::string serialize() const noexcept;
     bool deserialize(const std::string& str) noexcept;
 private:
     GncOptionUIType m_ui_type;
-    ValueType m_value;
+    bool m_is_currency;
-    ValueType m_default_value;
+    std::string m_namespace;
-    std::function<bool(ValueType)> m_validator;                         //11
+    std::string m_mnemonic;
-    ValueType m_validation_data;
+    std::string m_default_namespace;
+    std::string m_default_mnemonic;
 };
 
 QofInstance* qof_instance_from_string(const std::string& str,
@@ -252,9 +231,7 @@ template <typename T>
 struct is_QofInstanceValue
 {
     static constexpr bool value =
-         (std::is_same_v<std::decay_t<T>, GncOptionQofInstanceValue> ||
+        std::is_same_v<std::decay_t<T>, GncOptionQofInstanceValue>;
-          std::is_same_v<std::decay_t<T>,
-          GncOptionValidatedValue<const QofInstance*>>);
 };
 
 template <typename T> inline constexpr bool
@@ -264,9 +241,7 @@ template <typename T>
 struct is_QofQueryValue
 {
     static constexpr bool value =
-         (std::is_same_v<std::decay_t<T>, GncOptionValue<const QofQuery*>> ||
+         std::is_same_v<std::decay_t<T>, GncOptionValue<const QofQuery*>>;
-          std::is_same_v<std::decay_t<T>,
-          GncOptionValidatedValue<const QofQuery*>>);
 };
 
 template <typename T> inline constexpr bool
@@ -298,25 +273,20 @@ operator<< <GncOptionValue<bool>>(std::ostream& oss,
     return oss;
 }
 
+inline std::ostream&
+operator<< (std::ostream& oss, const GncOptionCommodityValue& opt)
+{
+    oss << opt.serialize();
+    return oss;
+}
+
 template<class OptType,
          typename std::enable_if_t<is_QofInstanceValue_v<OptType>, int> = 0>
 inline std::ostream&
 operator<< (std::ostream& oss, const OptType& opt)
 {
     auto value = opt.get_value();
-    if (auto type = opt.get_ui_type(); type == GncOptionUIType::COMMODITY ||
+    oss << qof_instance_to_string(value);
-        type == GncOptionUIType::CURRENCY)
-    {
-        if (type == GncOptionUIType::COMMODITY)
-        {
-            oss << gnc_commodity_get_namespace(GNC_COMMODITY(value)) << " ";
-        }
-        oss << gnc_commodity_get_mnemonic(GNC_COMMODITY(value));
-    }
-    else
-    {
-        oss << qof_instance_to_string(value);
-    }
     return oss;
 }
 
@@ -339,35 +309,15 @@ std::istream& operator>>(std::istream& iss, OptType& opt)
     }
 }
 
+std::istream& operator>> (std::istream& iss, GncOptionCommodityValue& opt);
+
 template<class OptType,
          typename std::enable_if_t<is_QofInstanceValue_v<OptType>, int> = 0>
 std::istream&
 operator>> (std::istream& iss, OptType& opt)
 {
     std::string instr;
-    auto type = opt.get_ui_type();
+    iss >> instr;
-    if (type == GncOptionUIType::COMMODITY || type == GncOptionUIType::CURRENCY)
-    {
-        std::string name_space, mnemonic;
-        if (type == GncOptionUIType::COMMODITY)
-            iss >> name_space;
-        else
-            name_space = GNC_COMMODITY_NS_CURRENCY;
-        if (name_space.find(":") == std::string::npos)
-        {
-            iss >> mnemonic;
-            instr = name_space + ":";
-            instr += mnemonic;
-        }
-        else
-        {
-            instr = name_space;
-        }
-     }
-    else
-    {
-        iss >> instr;
-    }
     opt.set_value(qof_instance_from_string(instr, opt.get_ui_type()));
     return iss;
 }

libgnucash/app-utils/gnc-option.cpp

@@ -302,15 +302,15 @@ GncOption::validate(ValueType value) const
     return std::visit(
         [value] (const auto& option) -> bool {
             if constexpr ((is_same_decayed_v<decltype(option),
-                           GncOptionMultichoiceValue> &&
+                                             GncOptionMultichoiceValue> &&
-                           is_same_decayed_v<ValueType,
+                           is_same_decayed_v<ValueType, std::string>) ||
-                           std::string>) ||
                           (is_same_decayed_v<decltype(option),
-                           GncOptionMultichoiceValue> &&
+                                             GncOptionMultichoiceValue> &&
                            is_same_decayed_v<ValueType,
-                           GncMultichoiceOptionIndexVec>) ||
+                                             GncMultichoiceOptionIndexVec>) ||
-                          is_same_decayed_v<decltype(option),
+                          (is_same_decayed_v<decltype(option),
-                          GncOptionValidatedValue<ValueType>>)
+                                             GncOptionCommodityValue> &&
+                           is_same_decayed_v<ValueType, gnc_commodity*>))
                 return option.validate(value);
             else
                 return false;
@@ -448,8 +448,6 @@ gnc_make_SCM_option(const char* section, const char* name,
  */
 
 
-template class GncOptionValidatedValue<const QofInstance*>;
-
 template GncOption::GncOption(const char*, const char*, const char*,
                               const char*, bool, GncOptionUIType);
 //template GncOption::GncOption(const char*, const char*, const char*,
@@ -477,6 +475,7 @@ template size_t GncOption::get_value<size_t>() const;
 template const char* GncOption::get_value<const char*>() const;
 template std::string GncOption::get_value<std::string>() const;
 template const QofInstance* GncOption::get_value<const QofInstance*>() const;
+template gnc_commodity* GncOption::get_value<gnc_commodity*>() const;
 template const Account* GncOption::get_value<const Account*>() const;
 template RelativeDatePeriod GncOption::get_value<RelativeDatePeriod>() const;
 template GncOptionAccountList GncOption::get_value<GncOptionAccountList>() const;
@@ -490,6 +489,7 @@ template double GncOption::get_default_value<double>() const;
 template const char* GncOption::get_default_value<const char*>() const;
 template std::string GncOption::get_default_value<std::string>() const;
 template const QofInstance* GncOption::get_default_value<const QofInstance*>() const;
+template gnc_commodity* GncOption::get_default_value<gnc_commodity*>() const;
 template const Account* GncOption::get_default_value<const Account*>() const;
 template RelativeDatePeriod GncOption::get_default_value<RelativeDatePeriod>() const;
 template GncOptionAccountList GncOption::get_default_value<GncOptionAccountList>() const;
@@ -504,6 +504,7 @@ template void GncOption::set_value(char*);
 template void GncOption::set_value(const char*);
 template void GncOption::set_value(std::string);
 template void GncOption::set_value(const QofInstance*);
+template void GncOption::set_value(gnc_commodity*);
 template void GncOption::set_value(const Account*);
 template void GncOption::set_value(RelativeDatePeriod);
 template void GncOption::set_value(size_t);
@@ -535,6 +536,7 @@ template bool GncOption::validate(double) const;
 template bool GncOption::validate(const char*) const;
 template bool GncOption::validate(std::string) const;
 template bool GncOption::validate(const QofInstance*) const;
+template bool GncOption::validate(gnc_commodity*) const;
 template bool GncOption::validate(const Account*) const;
 template bool GncOption::validate(const QofQuery*) const;
 template bool GncOption::validate(RelativeDatePeriod) const;

libgnucash/app-utils/gnc-option.hpp

@@ -60,7 +60,7 @@ class GncOptionAccountListValue;
 class GncOptionAccountSelValue;
 class GncOptionMultichoiceValue;
 template <typename ValueType> class GncOptionRangeValue;
-template <typename ValueType> class GncOptionValidatedValue;
+class GncOptionCommodityValue;
 class GncOptionDateValue;
 
 template <typename T>
@@ -107,8 +107,7 @@ using GncOptionVariant = std::variant<GncOptionValue<std::string>,
                                       GncOptionMultichoiceValue,
                                       GncOptionRangeValue<int>,
                                       GncOptionRangeValue<double>,
-                                      GncOptionValidatedValue<const QofInstance*>,
+                                      GncOptionCommodityValue,
-                                      GncOptionValidatedValue<const QofQuery*>,
                                       GncOptionDateValue>;
 
 using GncOptionVariantPtr = std::unique_ptr<GncOptionVariant>;

libgnucash/app-utils/gnc-optiondb.cpp

@@ -444,8 +444,6 @@ is_qofinstance_ui_type(GncOptionUIType type)
 {
     switch (type)
     {
-        case GncOptionUIType::CURRENCY:
-        case GncOptionUIType::COMMODITY:
         case GncOptionUIType::ACCOUNT_SEL:
         case GncOptionUIType::BUDGET:
         case GncOptionUIType::OWNER:
@@ -651,8 +649,8 @@ gnc_register_commodity_option(GncOptionDB* db, const char* section,
                               const char* name, const char* key,
                               const char* doc_string, gnc_commodity *value)
 {
-    GncOption option{GncOptionQofInstanceValue{section, name, key, doc_string,
+    GncOption option{GncOptionCommodityValue{section, name, key, doc_string,
-                                               (const QofInstance*)value,
+                                               value,
                                                GncOptionUIType::COMMODITY}};
     db->register_option(section, std::move(option));
 }
@@ -917,14 +915,8 @@ gnc_register_currency_option(GncOptionDB* db, const char* section,
                              const char* name, const char* key,
                              const char* doc_string, gnc_commodity *value)
 {
-    GncOption option{GncOptionValidatedValue<const QofInstance*>{
+    GncOption option{GncOptionCommodityValue{
-        section, name, key, doc_string, (const QofInstance*)value,
+        section, name, key, doc_string, value,GncOptionUIType::CURRENCY
-        [](const QofInstance* new_value) -> bool
-            {
-                return GNC_IS_COMMODITY (new_value) &&
-                    gnc_commodity_is_currency(GNC_COMMODITY(new_value));
-            },
-            GncOptionUIType::CURRENCY
         }};
     db->register_option(section, std::move(option));
 }

libgnucash/app-utils/gnc-optiondb.i

@@ -240,6 +240,14 @@ scm_from_value<const Account*>(const Account* value)
     return scm_from_value<const QofInstance*>(QOF_INSTANCE(value));
 }
 
+template <> inline SCM
+scm_from_value<gnc_commodity*>(gnc_commodity* value)
+{
+    if (!value)
+        return SCM_BOOL_F;
+    return scm_from_value<const QofInstance*>((const QofInstance*)value);
+}
+
 template <> inline SCM
 scm_from_value<QofQuery*>(QofQuery* value)
 {
@@ -334,7 +342,7 @@ scm_to_value<const QofInstance*>(SCM new_value)
 
     auto info = SWIG_PointerType(new_value);
 
-    static const std::array<swig_type_info*, 11> types{
+    static const std::array<swig_type_info*, 10> types{
         SWIGTYPE_p_QofInstance_s, SWIGTYPE_p_gnc_commodity,
         SWIGTYPE_p_budget_s, SWIGTYPE_p__gncInvoice,
         SWIGTYPE_p__gncTaxTable, SWIGTYPE_p_Account,
@@ -352,6 +360,36 @@ scm_to_value<const QofInstance*>(SCM new_value)
     return static_cast<const QofInstance*>(ptr);
 }
 
+template <> inline gnc_commodity*
+scm_to_value<gnc_commodity*>(SCM new_value)
+{
+    auto comm{scm_to_value<const QofInstance*>(new_value)};
+    if (comm)
+        return GNC_COMMODITY(comm);
+    if (scm_is_list(new_value))
+    {
+        auto len{scm_to_uint(scm_length(new_value))};
+        std::string mnemonic{scm_to_utf8_string(scm_list_ref(new_value,
+                                                         scm_from_uint(0)))};
+        std::string name_space{"CURRENCY"};
+        if (len > 1)
+           name_space = scm_to_utf8_string(scm_list_ref(new_value,
+                                                        scm_from_uint(1)));
+        auto book{gnc_get_current_book()};
+        auto table = gnc_commodity_table_get_table(book);
+        return gnc_commodity_table_lookup(table, name_space.c_str(),
+                                          mnemonic.c_str());
+    }
+    if (scm_is_string(new_value))
+    {
+        auto book{gnc_get_current_book()};
+        auto table = gnc_commodity_table_get_table(book);
+        std::string mnemonic{scm_to_utf8_string(new_value)};
+        return gnc_commodity_table_lookup(table, "CURRENCY", mnemonic.c_str());
+    }
+    return nullptr;
+}
+
 template <> inline const Account*
 scm_to_value<const Account*>(SCM new_value)
 {
@@ -431,6 +469,12 @@ gnc_option_test_book_destroy(QofBook* book)
 %ignore GncOptionMultichoiceValue(GncOptionMultichoiceValue&&);
 %ignore GncOptionMultichoiceValue::operator=(const GncOptionMultichoiceValue&);
 %ignore GncOptionMultichoiceValue::operator=(GncOptionMultichoiceValue&&);
+%ignore GncOptionQofInstanceValue(GncOptionQofInstanceValue&&);
+%ignore GncOptionQofInstanceValue::operator=(const GncOptionQofInstanceValue&);
+%ignore GncOptionQofInstanceValue::operator=(GncOptionQofInstanceValue&&);
+%ignore GncOptionCommodityValue(GncOptionCommodityValue&&);
+%ignore GncOptionCommodityValue::operator=(const GncOptionCommodityValue&);
+%ignore GncOptionCommodityValue::operator=(GncOptionCommodityValue&&);
 %ignore GncOptionDateValue(GncOptionDateValue&&);
 %ignore GncOptionDateValue::operator=(const GncOptionDateValue&);
 %ignore GncOptionDateValue::operator=(GncOptionDateValue&&);
@@ -1056,25 +1100,27 @@ inline SCM return_scm_value(ValueType value)
                     if (serial.empty())
                         return no_value;
                     auto value{scm_list_1(scm_from_utf8_string(serial.c_str()))};
-                    if (uitype == GncOptionUIType::CURRENCY)
-                    {
-                        const SCM quoted_format_str{scm_from_utf8_string("\"~a\"")};
-                        return scm_simple_format(SCM_BOOL_F, quoted_format_str, value);
-                    }
-                    else if (uitype == GncOptionUIType::COMMODITY)
-                    {
-                        const SCM commodity_fmt{scm_from_utf8_string("\"~a\" \"~a\"")};
-                        auto comm{GNC_COMMODITY(option.get_value())};
-                        auto name_space{gnc_commodity_get_namespace(comm)};
-                        auto mnemonic{gnc_commodity_get_mnemonic(comm)};
-                        auto commodity_val{scm_list_2(scm_from_utf8_string(name_space),
-                                                      scm_from_utf8_string(mnemonic))};
-                        return scm_simple_format(SCM_BOOL_F, commodity_fmt, commodity_val);
-                    }
-                    else
-                    {
                         return scm_simple_format(SCM_BOOL_F, plain_format_str, value);
-                    }
+                }
+                if constexpr (is_same_decayed_v<decltype(option),
+                              GncOptionCommodityValue>)
+                {
+                     auto comm{option.get_value()};
+                     auto mnemonic{gnc_commodity_get_mnemonic(comm)};
+                     if (gnc_commodity_is_currency(comm))
+                     {
+                         auto value{scm_list_1(scm_from_utf8_string(mnemonic))};
+                         const SCM quoted_format_str{scm_from_utf8_string("~s")};
+                         return scm_simple_format(SCM_BOOL_F, quoted_format_str, value);
+                     }
+                     else
+                     {
+                          const SCM commodity_fmt{scm_from_utf8_string("~s ~s")};
+                          auto name_space{gnc_commodity_get_namespace(comm)};
+                          auto commodity_val{scm_list_2(scm_from_utf8_string(name_space),
+                                                        scm_from_utf8_string(mnemonic))};
+                          return scm_simple_format(SCM_BOOL_F, commodity_fmt, commodity_val);
+                     }
                 }
                 if constexpr (is_same_decayed_v<decltype(option),
                               GncOptionDateValue>)
@@ -1198,6 +1244,34 @@ inline SCM return_scm_value(ValueType value)
                             option.set_value(scm_to_int(new_value));
                         return;
                     }
+                    if constexpr (is_same_decayed_v<decltype(option),
+                                  GncOptionCommodityValue>)
+                    {
+                        if (scm_list_p(new_value) == SCM_BOOL_F)
+                        {
+                            if (scm_is_string(new_value))
+                            {
+                                 auto strval{scm_to_utf8_string(new_value)};
+                                 auto val{qof_instance_from_string(strval, option.get_ui_type())};
+                                 option.set_value(GNC_COMMODITY(val));
+                                 return;
+                            }
+                            option.set_value(scm_to_value<gnc_commodity*>(new_value));
+                            return;
+                        }
+                        auto len{scm_to_uint(scm_length(new_value))};
+                        std::string mnemonic{scm_to_utf8_string(scm_list_ref(new_value, scm_from_uint(0)))};
+                        if (len > 1)
+                        {
+                             std::string name_space{scm_to_utf8_string(scm_list_ref(new_value, scm_from_uint(1)))};
+                             option.deserialize(name_space + ":" + mnemonic);
+                        }
+                        else
+                        {
+                            option.deserialize(mnemonic);
+                        }
+                        return;
+                    }
                     if constexpr (is_QofInstanceValue_v<decltype(option)>)
                     {
                         if (scm_is_string(new_value))
@@ -1288,6 +1362,12 @@ inline SCM return_scm_value(ValueType value)
                             option.set_default_value(scm_to_int(new_value));
                         return;
                     }
+                    if constexpr (is_same_decayed_v<decltype(option),
+                                  GncOptionCommodityValue>)
+                    {
+                       auto comm{scm_to_value<gnc_commodity*>(new_value)};
+                       option.set_default_value(comm);
+                    }
                     if constexpr (is_QofInstanceValue_v<decltype(option)>)
                     {
                         if (scm_is_string(new_value))
@@ -1589,8 +1669,8 @@ inline SCM return_scm_value(ValueType value)
                               const char* key, const char* doc_string,
                               gnc_commodity *value)
     {
-        return new GncOption{GncOptionQofInstanceValue{
+        return new GncOption{GncOptionCommodityValue{
-                section, name, key, doc_string, (const QofInstance*)value,
+                section, name, key, doc_string, value,
                     GncOptionUIType::COMMODITY}};
     }
 
@@ -1603,15 +1683,17 @@ inline SCM return_scm_value(ValueType value)
         const auto book{qof_session_get_book(gnc_get_current_session())};
         const auto commodity_table{gnc_commodity_table_get_table(book)};
         const auto namespaces{gnc_commodity_table_get_namespaces(commodity_table)};
-        for (auto node = namespaces; node && commodity == nullptr; node = g_list_next(node))
+        for (auto node = namespaces; node && commodity == nullptr;
+             node = g_list_next(node))
+        {
             commodity = gnc_commodity_table_lookup(commodity_table,
                                                    (const char*)(node->data),
                                                    value);
 
-        if (commodity)
+            if (commodity)
-            return gnc_make_commodity_option(section, name, key, doc_string,
+                return gnc_make_commodity_option(section, name, key, doc_string,
-                                            commodity);
+                                                 commodity);
-
+        }
         return nullptr;
     }
 
@@ -1622,16 +1704,9 @@ inline SCM return_scm_value(ValueType value)
     {
         try
         {
-            return new GncOption{GncOptionValidatedValue<const QofInstance*>{
+            return new GncOption{GncOptionCommodityValue{
-                    section, name, key, doc_string, (const QofInstance*)value,
+                    section, name, key, doc_string, value,
-                        [](const QofInstance* new_value) -> bool
+                    GncOptionUIType::CURRENCY}};
-                    {
-                        return GNC_IS_COMMODITY (new_value) &&
-                            gnc_commodity_is_currency(GNC_COMMODITY(new_value));
-                    },
-                        GncOptionUIType::CURRENCY
-                            }
-            };
         }
         catch (const std::exception& err)
         {

libgnucash/app-utils/test/gtest-gnc-option.cpp

@@ -243,13 +243,9 @@ make_currency_option (const char* section, const char* name,
                       const char* key, const char* doc_string,
                       gnc_commodity *value, bool is_currency=false)
 {
-    GncOption option{GncOptionValidatedValue<const QofInstance*>{
+    GncOption option{GncOptionCommodityValue{
-        section, name, key, doc_string, (const QofInstance*)value,
+        section, name, key, doc_string, value,
-        [](const QofInstance* new_value) -> bool
+        is_currency ? GncOptionUIType::CURRENCY : GncOptionUIType::COMMODITY}
-            {
-                return GNC_IS_COMMODITY (new_value) &&
-                    gnc_commodity_is_currency(GNC_COMMODITY(new_value));
-            }, is_currency ? GncOptionUIType::CURRENCY : GncOptionUIType::COMMODITY}
     };
     return option;
 }
@@ -258,7 +254,7 @@ TEST_F(GncOptionCommodityTest, test_currency_ctor)
 {
     EXPECT_THROW({
             auto option = make_currency_option("foo", "bar", "baz",
-                                               "Phony Option", m_hpe, false);
+                                               "Phony Option", m_hpe, true);
         }, std::invalid_argument);
     EXPECT_NO_THROW({
             auto option = make_currency_option("foo", "bar", "baz",
@@ -275,23 +271,23 @@ TEST_F(GncOptionCommodityTest, test_currency_setter)
     auto option = make_currency_option("foo", "bar", "baz", "Phony Option",
                                       m_eur, true);
     EXPECT_NO_THROW({
-            option.set_value((const QofInstance*)m_usd);
+            option.set_value(m_usd);
         });
     EXPECT_PRED2(gnc_commodity_equal, m_usd,
-                 GNC_COMMODITY(option.get_value<const QofInstance*>()));
+                 GNC_COMMODITY(option.get_value<gnc_commodity*>()));
     EXPECT_THROW({
-            option.set_value((const QofInstance*)m_hpe);
+            option.set_value(m_hpe);
         }, std::invalid_argument);
     EXPECT_PRED2(gnc_commodity_equal, m_usd,
-                 GNC_COMMODITY(option.get_value<const QofInstance*>()));
+                 GNC_COMMODITY(option.get_value<gnc_commodity *>()));
 }
 
 TEST_F(GncOptionCommodityTest, test_currency_validator)
 {
     auto option = make_currency_option("foo", "bar", "baz", "Phony Option",
                                       m_eur, true);
-    EXPECT_TRUE(option.validate((const QofInstance*)m_usd));
+    EXPECT_TRUE(option.validate(m_usd));
-    EXPECT_FALSE(option.validate((const QofInstance*)m_aapl));
+    EXPECT_FALSE(option.validate(m_aapl));
 }
 
 static inline std::string make_currency_str(gnc_commodity* cur)
@@ -344,19 +340,19 @@ TEST_F(GncOptionCommodityTest, test_currency_in)
             std::string usd_str{make_currency_str(m_usd)};
             std::istringstream iss{usd_str};
             iss >> option;
-            EXPECT_EQ(QOF_INSTANCE(m_usd), option.get_value<const QofInstance*>());
+            EXPECT_EQ(m_usd, option.get_value<gnc_commodity*>());
         });
 }
 
 TEST_F(GncOptionCommodityTest, test_commodity_in)
 {
-    GncOption option{GncOptionQofInstanceValue{"foo", "bar", "baz", "Phony Option", (const QofInstance*)m_aapl,
+    GncOption option{GncOptionCommodityValue{"foo", "bar", "baz", "Phony Option", m_aapl,
                      GncOptionUIType::COMMODITY}};
 
     std::string hpe_str{make_commodity_str(m_hpe)};
     std::istringstream iss{hpe_str};
     iss >> option;
-    EXPECT_EQ(QOF_INSTANCE(m_hpe), option.get_value<const QofInstance*>());
+    EXPECT_EQ(m_hpe, option.get_value<gnc_commodity*>());
 }
 
 class GncUIType

libgnucash/app-utils/test/test-gnc-option-scheme-output.scm

@@ -87,19 +87,19 @@
 (let ((option (gnc:lookup-option options
                                  \"foo\"
                                  \"bar\")))
-  ((lambda (o) (if o (gnc:option-set-value o \"~a\"))) option))
+  ((lambda (o) (if o (gnc:option-set-value o ~s))) option))
 
 " value))
 
 (define (test-commodity-output-template value)
   (let ((value-parts (string-split value #\:)))
-    (format #f "
+       (format #f "
 ; Section: foo
 
 (let ((option (gnc:lookup-option options
                                  \"foo\"
                                  \"bar\")))
-  ((lambda (o) (if o (gnc:option-set-value o \"~a\" \"~a\"))) option))
+  ((lambda (o) (if o (gnc:option-set-value o ~s ~s))) option))
 
 " (car value-parts) (cadr value-parts))))
 
@@ -208,6 +208,8 @@ veritatis et quasi architecto beatae vitae dicta sunt, explicabo.")
          (comm-tbl (gnc-commodity-table-get-table book))
          (AAPL (gnc-commodity-new book "Apple" "NASDAQ" "AAPL" "" 1))
          (FMAGX (gnc-commodity-new book "Fidelity Magellan Fund" "FUND" "FMAGX" "" 1000)))
+         (gnc-commodity-table-insert comm-tbl AAPL)
+         (gnc-commodity-table-insert comm-tbl FMAGX)
     (test-option-scheme-output "commodity"
                                gnc:make-commodity-option GncOption-serialize
                                test-commodity-output-template