From daff67b1502282facd79ab6d8486c2619b26da04 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 22 Dec 2011 21:12:15 +0000 Subject: [PATCH] Simply log errors and continue when type errors occur while loading the user config file, rather than only loading the portion of the file before the error Originally committed to SVN as r6017. --- aegisub/libaegisub/common/option.cpp | 6 +- aegisub/libaegisub/common/option_visit.cpp | 132 +++++++++++------- aegisub/libaegisub/common/option_visit.h | 11 +- .../libaegisub/include/libaegisub/option.h | 3 +- 4 files changed, 95 insertions(+), 57 deletions(-) diff --git a/aegisub/libaegisub/common/option.cpp b/aegisub/libaegisub/common/option.cpp index ec300d15b..ef96580cc 100644 --- a/aegisub/libaegisub/common/option.cpp +++ b/aegisub/libaegisub/common/option.cpp @@ -71,10 +71,10 @@ void Options::ConfigUser() { } /// @todo Handle other errors such as parsing and notifying the user. - LoadConfig(*stream); + LoadConfig(*stream, true); } -void Options::LoadConfig(std::istream& stream) { +void Options::LoadConfig(std::istream& stream, bool ignore_errors) { /// @todo Store all previously loaded configs in an array for bug report purposes, /// this is just a temp stub. json::UnknownElement config_root; @@ -88,7 +88,7 @@ void Options::LoadConfig(std::istream& stream) { LOG_E("option/load") << "json::Exception: " << e.what(); } - ConfigVisitor config_visitor(values, ""); + ConfigVisitor config_visitor(values, "", ignore_errors); config_root.Accept(config_visitor); } diff --git a/aegisub/libaegisub/common/option_visit.cpp b/aegisub/libaegisub/common/option_visit.cpp index 22ef6ce46..7205255eb 100644 --- a/aegisub/libaegisub/common/option_visit.cpp +++ b/aegisub/libaegisub/common/option_visit.cpp @@ -19,23 +19,34 @@ /// @see option_visit.h /// @ingroup libaegisub +#include "option_visit.h" + #ifndef LAGI_PRE #include #include #endif #include +#include #include -#include "option_visit.h" namespace agi { -ConfigVisitor::ConfigVisitor(OptionValueMap &val, const std::string &member_name) +ConfigVisitor::ConfigVisitor(OptionValueMap &val, const std::string &member_name, bool ignore_errors) : values(val) , name(member_name) +, ignore_errors(ignore_errors) { } +template +void ConfigVisitor::Error(const char *message) { + if (ignore_errors) + LOG_E("option/load/config_visitor") << "Error loading option from user configuration: " << message; + else + throw ErrorType(message); +} + void ConfigVisitor::Visit(const json::Object& object) { json::Object::const_iterator index(object.begin()), index_end(object.end()); @@ -43,7 +54,7 @@ void ConfigVisitor::Visit(const json::Object& object) { name += "/"; for (; index != index_end; ++index) { - ConfigVisitor config_visitor(values, name + index->first); + ConfigVisitor config_visitor(values, name + index->first, ignore_errors); index->second.Accept(config_visitor); } } @@ -59,17 +70,21 @@ inline int64_t convert_unknown(json::UnknownElement const& ue) { } template -static OptionValue *read_array(json::Array const& src, std::string const& array_type, std::string const& name, void (OptionValueType::*set_list)(const std::vector&)) { +OptionValue *ConfigVisitor::ReadArray(json::Array const& src, std::string const& array_type, void (OptionValueType::*set_list)(const std::vector&)) { std::vector arr; arr.reserve(src.size()); for (json::Array::const_iterator it = src.begin(); it != src.end(); ++it) { json::Object const& obj = *it; - if (obj.size() != 1) - throw OptionJsonValueArray("Invalid array member"); - if (obj.begin()->first != array_type) - throw OptionJsonValueArray("Attempt to insert value into array of wrong type"); + if (obj.size() != 1) { + Error("Invalid array member"); + return 0; + } + if (obj.begin()->first != array_type) { + Error("Attempt to insert value into array of wrong type"); + return 0; + } arr.push_back(convert_unknown(obj.begin()->second)); } @@ -80,27 +95,31 @@ static OptionValue *read_array(json::Array const& src, std::string const& array_ } void ConfigVisitor::Visit(const json::Array& array) { - if (array.empty()) - throw OptionJsonValueArray("Cannot infer the type of an empty array"); + if (array.empty()) { + Error("Cannot infer the type of an empty array"); + return; + } json::Object const& front = array.front(); - if (front.size() != 1) - throw OptionJsonValueArray("Invalid array member"); + if (front.size() != 1) { + Error("Invalid array member"); + return; + } const std::string& array_type = front.begin()->first; if (array_type == "string") - AddOptionValue(read_array(array, array_type, name, &OptionValueListString::SetListString)); + AddOptionValue(ReadArray(array, array_type, &OptionValueListString::SetListString)); else if (array_type == "int") - AddOptionValue(read_array(array, array_type, name, &OptionValueListInt::SetListInt)); + AddOptionValue(ReadArray(array, array_type, &OptionValueListInt::SetListInt)); else if (array_type == "double") - AddOptionValue(read_array(array, array_type, name, &OptionValueListDouble::SetListDouble)); + AddOptionValue(ReadArray(array, array_type, &OptionValueListDouble::SetListDouble)); else if (array_type == "bool") - AddOptionValue(read_array(array, array_type, name, &OptionValueListBool::SetListBool)); + AddOptionValue(ReadArray(array, array_type, &OptionValueListBool::SetListBool)); else if (array_type == "colour") - AddOptionValue(read_array(array, array_type, name, &OptionValueListColour::SetListColour)); + AddOptionValue(ReadArray(array, array_type, &OptionValueListColour::SetListColour)); else - throw OptionJsonValueArray("Array type not handled"); + Error("Array type not handled"); } @@ -125,10 +144,15 @@ void ConfigVisitor::Visit(const json::Boolean& boolean) { } void ConfigVisitor::Visit(const json::Null& null) { - throw OptionJsonValueNull("Attempt to read null value"); + Error("Attempt to read null value"); } void ConfigVisitor::AddOptionValue(OptionValue* opt) { + if (!opt) { + assert(ignore_errors); + return; + } + if (!values.count(name)) { values[name] = opt; return; @@ -138,46 +162,54 @@ void ConfigVisitor::AddOptionValue(OptionValue* opt) { // method throws std::auto_ptr auto_opt(opt); - switch (opt->GetType()) { - case OptionValue::Type_String: - values[name]->SetString(opt->GetString()); - break; + try { + switch (opt->GetType()) { + case OptionValue::Type_String: + values[name]->SetString(opt->GetString()); + break; - case OptionValue::Type_Int: - values[name]->SetInt(opt->GetInt()); - break; + case OptionValue::Type_Int: + values[name]->SetInt(opt->GetInt()); + break; - case OptionValue::Type_Double: - values[name]->SetDouble(opt->GetDouble()); - break; + case OptionValue::Type_Double: + values[name]->SetDouble(opt->GetDouble()); + break; - case OptionValue::Type_Colour: - values[name]->SetColour(opt->GetColour()); - break; + case OptionValue::Type_Colour: + values[name]->SetColour(opt->GetColour()); + break; - case OptionValue::Type_Bool: - values[name]->SetBool(opt->GetBool()); - break; + case OptionValue::Type_Bool: + values[name]->SetBool(opt->GetBool()); + break; - case OptionValue::Type_List_String: - values[name]->SetListString(opt->GetListString()); - break; + case OptionValue::Type_List_String: + values[name]->SetListString(opt->GetListString()); + break; - case OptionValue::Type_List_Int: - values[name]->SetListInt(opt->GetListInt()); - break; + case OptionValue::Type_List_Int: + values[name]->SetListInt(opt->GetListInt()); + break; - case OptionValue::Type_List_Double: - values[name]->SetListDouble(opt->GetListDouble()); - break; + case OptionValue::Type_List_Double: + values[name]->SetListDouble(opt->GetListDouble()); + break; - case OptionValue::Type_List_Colour: - values[name]->SetListColour(opt->GetListColour()); - break; + case OptionValue::Type_List_Colour: + values[name]->SetListColour(opt->GetListColour()); + break; - case OptionValue::Type_List_Bool: - values[name]->SetListBool(opt->GetListBool()); - break; + case OptionValue::Type_List_Bool: + values[name]->SetListBool(opt->GetListBool()); + break; + } + } + catch (agi::OptionValueError const& e) { + if (ignore_errors) + LOG_E("option/load/config_visitor") << "Error loading option from user configuration: " << e.GetChainedMessage(); + else + throw; } } } // namespace agi diff --git a/aegisub/libaegisub/common/option_visit.h b/aegisub/libaegisub/common/option_visit.h index 67e92a4b8..588293b40 100644 --- a/aegisub/libaegisub/common/option_visit.h +++ b/aegisub/libaegisub/common/option_visit.h @@ -33,12 +33,17 @@ DEFINE_SIMPLE_EXCEPTION_NOINNER(OptionJsonValueNull, OptionJsonValueError, "opti class ConfigVisitor : public json::ConstVisitor { OptionValueMap &values; std::string name; - typedef std::pair OptionValuePair; + bool ignore_errors; + + template + void Error(const char *message); + + template + OptionValue *ReadArray(json::Array const& src, std::string const& array_type, void (OptionValueType::*set_list)(const std::vector&)); void AddOptionValue(OptionValue* opt); - public: - ConfigVisitor(OptionValueMap &val, const std::string &member_name); + ConfigVisitor(OptionValueMap &val, const std::string &member_name, bool ignore_errors = false); void Visit(const json::Array& array); void Visit(const json::Object& object); diff --git a/aegisub/libaegisub/include/libaegisub/option.h b/aegisub/libaegisub/include/libaegisub/option.h index 857722a46..fdc2a51bf 100644 --- a/aegisub/libaegisub/include/libaegisub/option.h +++ b/aegisub/libaegisub/include/libaegisub/option.h @@ -75,7 +75,8 @@ private: /// @brief Load a config file into the Options object. /// @param config Config to load. - void LoadConfig(std::istream& stream); + /// @param ignore_errors Log invalid entires in the option file and continue rather than throwing an exception + void LoadConfig(std::istream& stream, bool ignore_errors = false); /// @brief Write an option to file. /// @param[out] obj Parent object