From a7930073e807607dc7823399c9a9f4fbc0335957 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 29 Sep 2019 16:16:19 +0200 Subject: [PATCH 1/3] Move settings parser declaration to separate header --- components/CMakeLists.txt | 2 +- components/settings/categories.hpp | 16 ++ components/settings/parser.cpp | 290 ++++++++++++++++++++++++++++ components/settings/parser.hpp | 30 +++ components/settings/settings.cpp | 299 +---------------------------- components/settings/settings.hpp | 6 +- 6 files changed, 340 insertions(+), 303 deletions(-) create mode 100644 components/settings/categories.hpp create mode 100644 components/settings/parser.cpp create mode 100644 components/settings/parser.hpp diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index ed52c8111f..412717aaad 100644 --- a/components/CMakeLists.txt +++ b/components/CMakeLists.txt @@ -29,7 +29,7 @@ endif (GIT_CHECKOUT) # source files add_component_dir (settings - settings + settings parser ) add_component_dir (bsa diff --git a/components/settings/categories.hpp b/components/settings/categories.hpp new file mode 100644 index 0000000000..d6cd042f61 --- /dev/null +++ b/components/settings/categories.hpp @@ -0,0 +1,16 @@ +#ifndef COMPONENTS_SETTINGS_CATEGORIES_H +#define COMPONENTS_SETTINGS_CATEGORIES_H + +#include +#include +#include +#include + +namespace Settings +{ + using CategorySetting = std::pair; + using CategorySettingVector = std::set>; + using CategorySettingValueMap = std::map; +} + +#endif // COMPONENTS_SETTINGS_CATEGORIES_H diff --git a/components/settings/parser.cpp b/components/settings/parser.cpp new file mode 100644 index 0000000000..38b1659739 --- /dev/null +++ b/components/settings/parser.cpp @@ -0,0 +1,290 @@ +#include "parser.hpp" + +#include + +#include + +#include +#include +#include + +void Settings::SettingsFileParser::loadSettingsFile(const std::string& file, CategorySettingValueMap& settings) +{ + mFile = file; + boost::filesystem::ifstream stream; + stream.open(boost::filesystem::path(file)); + Log(Debug::Info) << "Loading settings file: " << file; + std::string currentCategory; + mLine = 0; + while (!stream.eof() && !stream.fail()) + { + ++mLine; + std::string line; + std::getline( stream, line ); + + size_t i = 0; + if (!skipWhiteSpace(i, line)) + continue; + + if (line[i] == '#') // skip comment + continue; + + if (line[i] == '[') + { + size_t end = line.find(']', i); + if (end == std::string::npos) + fail("unterminated category"); + + currentCategory = line.substr(i+1, end - (i+1)); + boost::algorithm::trim(currentCategory); + i = end+1; + } + + if (!skipWhiteSpace(i, line)) + continue; + + if (currentCategory.empty()) + fail("empty category name"); + + size_t settingEnd = line.find('=', i); + if (settingEnd == std::string::npos) + fail("unterminated setting name"); + + std::string setting = line.substr(i, (settingEnd-i)); + boost::algorithm::trim(setting); + + size_t valueBegin = settingEnd+1; + std::string value = line.substr(valueBegin); + boost::algorithm::trim(value); + + if (settings.insert(std::make_pair(std::make_pair(currentCategory, setting), value)).second == false) + fail(std::string("duplicate setting: [" + currentCategory + "] " + setting)); + } +} + +void Settings::SettingsFileParser::saveSettingsFile(const std::string& file, CategorySettingValueMap& settings) +{ + using CategorySettingStatusMap = std::map; + + // No options have been written to the file yet. + CategorySettingStatusMap written; + for (CategorySettingValueMap::iterator it = settings.begin(); it != settings.end(); ++it) { + written[it->first] = false; + } + + // Have we substantively changed the settings file? + bool changed = false; + + // Were there any lines at all in the file? + bool existing = false; + + // The category/section we're currently in. + std::string currentCategory; + + // Open the existing settings.cfg file to copy comments. This might not be the same file + // as the output file if we're copying the setting from the default settings.cfg for the + // first time. A minor change in API to pass the source file might be in order here. + boost::filesystem::ifstream istream; + boost::filesystem::path ipath(file); + istream.open(ipath); + + // Create a new string stream to write the current settings to. It's likely that the + // input file and the output file are the same, so this stream serves as a temporary file + // of sorts. The setting files aren't very large so there's no performance issue. + std::stringstream ostream; + + // For every line in the input file... + while (!istream.eof() && !istream.fail()) { + std::string line; + std::getline(istream, line); + + // The current character position in the line. + size_t i = 0; + + // Don't add additional newlines at the end of the file. + if (istream.eof()) continue; + + // Copy entirely blank lines. + if (!skipWhiteSpace(i, line)) { + ostream << line << std::endl; + continue; + } + + // There were at least some comments in the input file. + existing = true; + + // Copy comments. + if (line[i] == '#') { + ostream << line << std::endl; + continue; + } + + // Category heading. + if (line[i] == '[') { + size_t end = line.find(']', i); + // This should never happen unless the player edited the file while playing. + if (end == std::string::npos) { + ostream << "# unterminated category: " << line << std::endl; + changed = true; + continue; + } + + // Ensure that all options in the current category have been written. + for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { + if (mit->second == false && mit->first.first == currentCategory) { + Log(Debug::Verbose) << "Added new setting: [" << currentCategory << "] " + << mit->first.second << " = " << settings[mit->first]; + ostream << mit->first.second << " = " << settings[mit->first] << std::endl; + mit->second = true; + changed = true; + } + } + + // Update the current category. + currentCategory = line.substr(i+1, end - (i+1)); + boost::algorithm::trim(currentCategory); + + // Write the (new) current category to the file. + ostream << "[" << currentCategory << "]" << std::endl; + // Log(Debug::Verbose) << "Wrote category: " << currentCategory; + + // A setting can apparently follow the category on an input line. That's rather + // inconvenient, since it makes it more likely to have duplicative sections, + // which our algorithm doesn't like. Do the best we can. + i = end+1; + } + + // Truncate trailing whitespace, since we're changing the file anayway. + if (!skipWhiteSpace(i, line)) + continue; + + // If we've found settings before the first category, something's wrong. This + // should never happen unless the player edited the file while playing, since + // the loadSettingsFile() logic rejects it. + if (currentCategory.empty()) { + ostream << "# empty category name: " << line << std::endl; + changed = true; + continue; + } + + // Which setting was at this location in the input file? + size_t settingEnd = line.find('=', i); + // This should never happen unless the player edited the file while playing. + if (settingEnd == std::string::npos) { + ostream << "# unterminated setting name: " << line << std::endl; + changed = true; + continue; + } + std::string setting = line.substr(i, (settingEnd-i)); + boost::algorithm::trim(setting); + + // Get the existing value so we can see if we've changed it. + size_t valueBegin = settingEnd+1; + std::string value = line.substr(valueBegin); + boost::algorithm::trim(value); + + // Construct the setting map key to determine whether the setting has already been + // written to the file. + CategorySetting key = std::make_pair(currentCategory, setting); + CategorySettingStatusMap::iterator finder = written.find(key); + + // Settings not in the written map are definitely invalid. Currently, this can only + // happen if the player edited the file while playing, because loadSettingsFile() + // will accept anything and pass it along in the map, but in the future, we might + // want to handle invalid settings more gracefully here. + if (finder == written.end()) { + ostream << "# invalid setting: " << line << std::endl; + changed = true; + continue; + } + + // Write the current value of the setting to the file. The key must exist in the + // settings map because of how written was initialized and finder != end(). + ostream << setting << " = " << settings[key] << std::endl; + // Mark that setting as written. + finder->second = true; + // Did we really change it? + if (value != settings[key]) { + Log(Debug::Verbose) << "Changed setting: [" << currentCategory << "] " + << setting << " = " << settings[key]; + changed = true; + } + // No need to write the current line, because we just emitted a replacement. + + // Curiously, it appears that comments at the ends of lines with settings are not + // allowed, and the comment becomes part of the value. Was that intended? + } + + // We're done with the input stream file. + istream.close(); + + // Ensure that all options in the current category have been written. We must complete + // the current category at the end of the file before moving on to any new categories. + for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { + if (mit->second == false && mit->first.first == currentCategory) { + Log(Debug::Verbose) << "Added new setting: [" << mit->first.first << "] " + << mit->first.second << " = " << settings[mit->first]; + ostream << mit->first.second << " = " << settings[mit->first] << std::endl; + mit->second = true; + changed = true; + } + } + + // If there was absolutely nothing in the file (or more likely the file didn't + // exist), start the newly created file with a helpful comment. + if (!existing) { + ostream << "# This is the OpenMW user 'settings.cfg' file. This file only contains" << std::endl; + ostream << "# explicitly changed settings. If you would like to revert a setting" << std::endl; + ostream << "# to its default, simply remove it from this file. For available" << std::endl; + ostream << "# settings, see the file 'settings-default.cfg' or the documentation at:" << std::endl; + ostream << "#" << std::endl; + ostream << "# https://openmw.readthedocs.io/en/master/reference/modding/settings/index.html" << std::endl; + } + + // We still have one more thing to do before we're completely done writing the file. + // It's possible that there are entirely new categories, or that the input file had + // disappeared completely, so we need ensure that all settings are written to the file + // regardless of those issues. + for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { + // If the setting hasn't been written yet. + if (mit->second == false) { + // If the catgory has changed, write a new category header. + if (mit->first.first != currentCategory) { + currentCategory = mit->first.first; + Log(Debug::Verbose) << "Created new setting section: " << mit->first.first; + ostream << std::endl; + ostream << "[" << currentCategory << "]" << std::endl; + } + Log(Debug::Verbose) << "Added new setting: [" << mit->first.first << "] " + << mit->first.second << " = " << settings[mit->first]; + // Then write the setting. No need to mark it as written because we're done. + ostream << mit->first.second << " = " << settings[mit->first] << std::endl; + changed = true; + } + } + + // Now install the newly written file in the requested place. + if (changed) { + Log(Debug::Info) << "Updating settings file: " << ipath; + boost::filesystem::ofstream ofstream; + ofstream.open(ipath); + ofstream << ostream.rdbuf(); + ofstream.close(); + } +} + +bool Settings::SettingsFileParser::skipWhiteSpace(size_t& i, std::string& str) +{ + while (i < str.size() && std::isspace(str[i], std::locale::classic())) + { + ++i; + } + return i < str.size(); +} + +void Settings::SettingsFileParser::fail(const std::string& message) +{ + std::stringstream error; + error << "Error on line " << mLine << " in " << mFile << ":\n" << message; + throw std::runtime_error(error.str()); +} diff --git a/components/settings/parser.hpp b/components/settings/parser.hpp new file mode 100644 index 0000000000..bd319e01cc --- /dev/null +++ b/components/settings/parser.hpp @@ -0,0 +1,30 @@ +#ifndef COMPONENTS_SETTINGS_PARSER_H +#define COMPONENTS_SETTINGS_PARSER_H + +#include "categories.hpp" + +#include + +namespace Settings +{ + class SettingsFileParser + { + public: + void loadSettingsFile(const std::string& file, CategorySettingValueMap& settings); + + void saveSettingsFile(const std::string& file, CategorySettingValueMap& settings); + + private: + /// Increment i until it longer points to a whitespace character + /// in the string or has reached the end of the string. + /// @return false if we have reached the end of the string + bool skipWhiteSpace(size_t& i, std::string& str); + + void fail(const std::string& message); + + std::string mFile; + int mLine = 0; + }; +} + +#endif // _COMPONENTS_SETTINGS_PARSER_H diff --git a/components/settings/settings.cpp b/components/settings/settings.cpp index 42f65fc557..8bbde7a641 100644 --- a/components/settings/settings.cpp +++ b/components/settings/settings.cpp @@ -1,12 +1,9 @@ #include "settings.hpp" - -#include +#include "parser.hpp" #include #include -#include -#include #include namespace Settings @@ -16,300 +13,6 @@ CategorySettingValueMap Manager::mDefaultSettings = CategorySettingValueMap(); CategorySettingValueMap Manager::mUserSettings = CategorySettingValueMap(); CategorySettingVector Manager::mChangedSettings = CategorySettingVector(); -typedef std::map< CategorySetting, bool > CategorySettingStatusMap; - -class SettingsFileParser -{ -public: - SettingsFileParser() : mLine(0) {} - - void loadSettingsFile (const std::string& file, CategorySettingValueMap& settings) - { - mFile = file; - boost::filesystem::ifstream stream; - stream.open(boost::filesystem::path(file)); - Log(Debug::Info) << "Loading settings file: " << file; - std::string currentCategory; - mLine = 0; - while (!stream.eof() && !stream.fail()) - { - ++mLine; - std::string line; - std::getline( stream, line ); - - size_t i = 0; - if (!skipWhiteSpace(i, line)) - continue; - - if (line[i] == '#') // skip comment - continue; - - if (line[i] == '[') - { - size_t end = line.find(']', i); - if (end == std::string::npos) - fail("unterminated category"); - - currentCategory = line.substr(i+1, end - (i+1)); - boost::algorithm::trim(currentCategory); - i = end+1; - } - - if (!skipWhiteSpace(i, line)) - continue; - - if (currentCategory.empty()) - fail("empty category name"); - - size_t settingEnd = line.find('=', i); - if (settingEnd == std::string::npos) - fail("unterminated setting name"); - - std::string setting = line.substr(i, (settingEnd-i)); - boost::algorithm::trim(setting); - - size_t valueBegin = settingEnd+1; - std::string value = line.substr(valueBegin); - boost::algorithm::trim(value); - - if (settings.insert(std::make_pair(std::make_pair(currentCategory, setting), value)).second == false) - fail(std::string("duplicate setting: [" + currentCategory + "] " + setting)); - } - } - - void saveSettingsFile (const std::string& file, CategorySettingValueMap& settings) - { - // No options have been written to the file yet. - CategorySettingStatusMap written; - for (CategorySettingValueMap::iterator it = settings.begin(); it != settings.end(); ++it) { - written[it->first] = false; - } - - // Have we substantively changed the settings file? - bool changed = false; - - // Were there any lines at all in the file? - bool existing = false; - - // The category/section we're currently in. - std::string currentCategory; - - // Open the existing settings.cfg file to copy comments. This might not be the same file - // as the output file if we're copying the setting from the default settings.cfg for the - // first time. A minor change in API to pass the source file might be in order here. - boost::filesystem::ifstream istream; - boost::filesystem::path ipath(file); - istream.open(ipath); - - // Create a new string stream to write the current settings to. It's likely that the - // input file and the output file are the same, so this stream serves as a temporary file - // of sorts. The setting files aren't very large so there's no performance issue. - std::stringstream ostream; - - // For every line in the input file... - while (!istream.eof() && !istream.fail()) { - std::string line; - std::getline(istream, line); - - // The current character position in the line. - size_t i = 0; - - // Don't add additional newlines at the end of the file. - if (istream.eof()) continue; - - // Copy entirely blank lines. - if (!skipWhiteSpace(i, line)) { - ostream << line << std::endl; - continue; - } - - // There were at least some comments in the input file. - existing = true; - - // Copy comments. - if (line[i] == '#') { - ostream << line << std::endl; - continue; - } - - // Category heading. - if (line[i] == '[') { - size_t end = line.find(']', i); - // This should never happen unless the player edited the file while playing. - if (end == std::string::npos) { - ostream << "# unterminated category: " << line << std::endl; - changed = true; - continue; - } - - // Ensure that all options in the current category have been written. - for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { - if (mit->second == false && mit->first.first == currentCategory) { - Log(Debug::Verbose) << "Added new setting: [" << currentCategory << "] " - << mit->first.second << " = " << settings[mit->first]; - ostream << mit->first.second << " = " << settings[mit->first] << std::endl; - mit->second = true; - changed = true; - } - } - - // Update the current category. - currentCategory = line.substr(i+1, end - (i+1)); - boost::algorithm::trim(currentCategory); - - // Write the (new) current category to the file. - ostream << "[" << currentCategory << "]" << std::endl; - // Log(Debug::Verbose) << "Wrote category: " << currentCategory; - - // A setting can apparently follow the category on an input line. That's rather - // inconvenient, since it makes it more likely to have duplicative sections, - // which our algorithm doesn't like. Do the best we can. - i = end+1; - } - - // Truncate trailing whitespace, since we're changing the file anayway. - if (!skipWhiteSpace(i, line)) - continue; - - // If we've found settings before the first category, something's wrong. This - // should never happen unless the player edited the file while playing, since - // the loadSettingsFile() logic rejects it. - if (currentCategory.empty()) { - ostream << "# empty category name: " << line << std::endl; - changed = true; - continue; - } - - // Which setting was at this location in the input file? - size_t settingEnd = line.find('=', i); - // This should never happen unless the player edited the file while playing. - if (settingEnd == std::string::npos) { - ostream << "# unterminated setting name: " << line << std::endl; - changed = true; - continue; - } - std::string setting = line.substr(i, (settingEnd-i)); - boost::algorithm::trim(setting); - - // Get the existing value so we can see if we've changed it. - size_t valueBegin = settingEnd+1; - std::string value = line.substr(valueBegin); - boost::algorithm::trim(value); - - // Construct the setting map key to determine whether the setting has already been - // written to the file. - CategorySetting key = std::make_pair(currentCategory, setting); - CategorySettingStatusMap::iterator finder = written.find(key); - - // Settings not in the written map are definitely invalid. Currently, this can only - // happen if the player edited the file while playing, because loadSettingsFile() - // will accept anything and pass it along in the map, but in the future, we might - // want to handle invalid settings more gracefully here. - if (finder == written.end()) { - ostream << "# invalid setting: " << line << std::endl; - changed = true; - continue; - } - - // Write the current value of the setting to the file. The key must exist in the - // settings map because of how written was initialized and finder != end(). - ostream << setting << " = " << settings[key] << std::endl; - // Mark that setting as written. - finder->second = true; - // Did we really change it? - if (value != settings[key]) { - Log(Debug::Verbose) << "Changed setting: [" << currentCategory << "] " - << setting << " = " << settings[key]; - changed = true; - } - // No need to write the current line, because we just emitted a replacement. - - // Curiously, it appears that comments at the ends of lines with settings are not - // allowed, and the comment becomes part of the value. Was that intended? - } - - // We're done with the input stream file. - istream.close(); - - // Ensure that all options in the current category have been written. We must complete - // the current category at the end of the file before moving on to any new categories. - for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { - if (mit->second == false && mit->first.first == currentCategory) { - Log(Debug::Verbose) << "Added new setting: [" << mit->first.first << "] " - << mit->first.second << " = " << settings[mit->first]; - ostream << mit->first.second << " = " << settings[mit->first] << std::endl; - mit->second = true; - changed = true; - } - } - - // If there was absolutely nothing in the file (or more likely the file didn't - // exist), start the newly created file with a helpful comment. - if (!existing) { - ostream << "# This is the OpenMW user 'settings.cfg' file. This file only contains" << std::endl; - ostream << "# explicitly changed settings. If you would like to revert a setting" << std::endl; - ostream << "# to its default, simply remove it from this file. For available" << std::endl; - ostream << "# settings, see the file 'settings-default.cfg' or the documentation at:" << std::endl; - ostream << "#" << std::endl; - ostream << "# https://openmw.readthedocs.io/en/master/reference/modding/settings/index.html" << std::endl; - } - - // We still have one more thing to do before we're completely done writing the file. - // It's possible that there are entirely new categories, or that the input file had - // disappeared completely, so we need ensure that all settings are written to the file - // regardless of those issues. - for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { - // If the setting hasn't been written yet. - if (mit->second == false) { - // If the catgory has changed, write a new category header. - if (mit->first.first != currentCategory) { - currentCategory = mit->first.first; - Log(Debug::Verbose) << "Created new setting section: " << mit->first.first; - ostream << std::endl; - ostream << "[" << currentCategory << "]" << std::endl; - } - Log(Debug::Verbose) << "Added new setting: [" << mit->first.first << "] " - << mit->first.second << " = " << settings[mit->first]; - // Then write the setting. No need to mark it as written because we're done. - ostream << mit->first.second << " = " << settings[mit->first] << std::endl; - changed = true; - } - } - - // Now install the newly written file in the requested place. - if (changed) { - Log(Debug::Info) << "Updating settings file: " << ipath; - boost::filesystem::ofstream ofstream; - ofstream.open(ipath); - ofstream << ostream.rdbuf(); - ofstream.close(); - } - } - -private: - /// Increment i until it longer points to a whitespace character - /// in the string or has reached the end of the string. - /// @return false if we have reached the end of the string - bool skipWhiteSpace(size_t& i, std::string& str) - { - while (i < str.size() && std::isspace(str[i], std::locale::classic())) - { - ++i; - } - return i < str.size(); - } - - void fail(const std::string& message) - { - std::stringstream error; - error << "Error on line " << mLine << " in " << mFile << ":\n" << message; - throw std::runtime_error(error.str()); - } - - std::string mFile; - int mLine; -}; - void Manager::clear() { mDefaultSettings.clear(); diff --git a/components/settings/settings.hpp b/components/settings/settings.hpp index 3d1cabede3..17d237fc35 100644 --- a/components/settings/settings.hpp +++ b/components/settings/settings.hpp @@ -1,16 +1,14 @@ #ifndef COMPONENTS_SETTINGS_H #define COMPONENTS_SETTINGS_H +#include "categories.hpp" + #include #include #include namespace Settings { - typedef std::pair < std::string, std::string > CategorySetting; - typedef std::set< std::pair > CategorySettingVector; - typedef std::map < CategorySetting, std::string > CategorySettingValueMap; - /// /// \brief Settings management (can change during runtime) /// From 862f50346c6b85f1814ce2be627bff22b795508b Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 29 Sep 2019 17:42:01 +0200 Subject: [PATCH 2/3] Add tests for settings parser --- apps/openmw_test_suite/CMakeLists.txt | 2 + apps/openmw_test_suite/settings/parser.cpp | 411 +++++++++++++++++++++ 2 files changed, 413 insertions(+) create mode 100644 apps/openmw_test_suite/settings/parser.cpp diff --git a/apps/openmw_test_suite/CMakeLists.txt b/apps/openmw_test_suite/CMakeLists.txt index 12775035ba..e216ec759c 100644 --- a/apps/openmw_test_suite/CMakeLists.txt +++ b/apps/openmw_test_suite/CMakeLists.txt @@ -25,6 +25,8 @@ if (GTEST_FOUND AND GMOCK_FOUND) detournavigator/recastmeshobject.cpp detournavigator/navmeshtilescache.cpp detournavigator/tilecachedrecastmeshmanager.cpp + + settings/parser.cpp ) source_group(apps\\openmw_test_suite FILES openmw_test_suite.cpp ${UNITTEST_SRC_FILES}) diff --git a/apps/openmw_test_suite/settings/parser.cpp b/apps/openmw_test_suite/settings/parser.cpp new file mode 100644 index 0000000000..d54360fc28 --- /dev/null +++ b/apps/openmw_test_suite/settings/parser.cpp @@ -0,0 +1,411 @@ +#include + +#include + +#include + +namespace +{ + using namespace testing; + using namespace Settings; + + struct SettingsFileParserTest : Test + { + SettingsFileParser mLoader; + SettingsFileParser mSaver; + + template + void withSettingsFile( const std::string& content, F&& f) + { + const auto path = std::string(UnitTest::GetInstance()->current_test_info()->name()) + ".cfg"; + + { + boost::filesystem::ofstream stream; + stream.open(path); + stream << content; + stream.close(); + } + + f(path); + } + }; + + TEST_F(SettingsFileParserTest, load_empty_file) + { + const std::string content; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap()); + }); + } + + TEST_F(SettingsFileParserTest, file_with_single_empty_section) + { + const std::string content = + "[Section]\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap()); + }); + } + + TEST_F(SettingsFileParserTest, file_with_single_section_and_key) + { + const std::string content = + "[Section]\n" + "key = value\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), "value"} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_single_section_and_key_and_line_comments) + { + const std::string content = + "# foo\n" + "[Section]\n" + "# bar\n" + "key = value\n" + "# baz\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), "value"} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_single_section_and_key_file_and_inline_section_comment) + { + const std::string content = + "[Section] # foo\n" + "key = value\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + EXPECT_THROW(mLoader.loadSettingsFile(path, map), std::runtime_error); + }); + } + + TEST_F(SettingsFileParserTest, file_single_section_and_key_and_inline_key_comment) + { + const std::string content = + "[Section]\n" + "key = value # foo\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), "value # foo"} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_single_section_and_key_and_whitespaces) + { + const std::string content = + " [ Section ] \n" + " key = value \n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), "value"} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_quoted_string_value) + { + const std::string content = + "[Section]\n" + R"(key = "value")" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), R"("value")"} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_quoted_string_value_and_eol) + { + const std::string content = + "[Section]\n" + R"(key = "value"\n)" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), R"("value"\n)"} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_empty_value) + { + const std::string content = + "[Section]\n" + "key =\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), ""} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_empty_key) + { + const std::string content = + "[Section]\n" + "=\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", ""), ""} + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_multiple_keys) + { + const std::string content = + "[Section]\n" + "key1 = value1\n" + "key2 = value2\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key1"), "value1"}, + {CategorySetting("Section", "key2"), "value2"}, + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_multiple_sections) + { + const std::string content = + "[Section1]\n" + "key1 = value1\n" + "[Section2]\n" + "key2 = value2\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section1", "key1"), "value1"}, + {CategorySetting("Section2", "key2"), "value2"}, + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_multiple_sections_and_keys) + { + const std::string content = + "[Section1]\n" + "key1 = value1\n" + "key2 = value2\n" + "[Section2]\n" + "key3 = value3\n" + "key4 = value4\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section1", "key1"), "value1"}, + {CategorySetting("Section1", "key2"), "value2"}, + {CategorySetting("Section2", "key3"), "value3"}, + {CategorySetting("Section2", "key4"), "value4"}, + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_repeated_sections) + { + const std::string content = + "[Section]\n" + "key1 = value1\n" + "[Section]\n" + "key2 = value2\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key1"), "value1"}, + {CategorySetting("Section", "key2"), "value2"}, + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_repeated_keys) + { + const std::string content = + "[Section]\n" + "key = value\n" + "key = value\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + EXPECT_THROW(mLoader.loadSettingsFile(path, map), std::runtime_error); + }); + } + + TEST_F(SettingsFileParserTest, file_with_repeated_keys_in_differrent_sections) + { + const std::string content = + "[Section1]\n" + "key = value\n" + "[Section2]\n" + "key = value\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section1", "key"), "value"}, + {CategorySetting("Section2", "key"), "value"}, + })); + }); + } + + TEST_F(SettingsFileParserTest, file_with_unterminated_section) + { + const std::string content = + "[Section" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + EXPECT_THROW(mLoader.loadSettingsFile(path, map), std::runtime_error); + }); + } + + TEST_F(SettingsFileParserTest, file_with_single_empty_section_name) + { + const std::string content = + "[]\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap()); + }); + } + + TEST_F(SettingsFileParserTest, file_with_key_and_without_section) + { + const std::string content = + "key = value\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + EXPECT_THROW(mLoader.loadSettingsFile(path, map), std::runtime_error); + }); + } + + TEST_F(SettingsFileParserTest, file_with_key_in_empty_name_section) + { + const std::string content = + "[]" + "key = value\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + EXPECT_THROW(mLoader.loadSettingsFile(path, map), std::runtime_error); + }); + } + + TEST_F(SettingsFileParserTest, file_with_unterminated_key) + { + const std::string content = + "[Section]\n" + "key\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + EXPECT_THROW(mLoader.loadSettingsFile(path, map), std::runtime_error); + }); + } + + TEST_F(SettingsFileParserTest, file_with_empty_lines) + { + const std::string content = + "\n" + "[Section]\n" + "\n" + "key = value\n" + "\n" + ; + + withSettingsFile(content, [this] (const auto& path) { + CategorySettingValueMap map; + mLoader.loadSettingsFile(path, map); + + EXPECT_EQ(map, CategorySettingValueMap({ + {CategorySetting("Section", "key"), "value"} + })); + }); + } +} From 275f552fcf07eef8b0273576210b03bda53c5242 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 29 Sep 2019 17:41:07 +0200 Subject: [PATCH 3/3] Do not modify settings on save --- components/settings/parser.cpp | 22 +++++++++++----------- components/settings/parser.hpp | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/components/settings/parser.cpp b/components/settings/parser.cpp index 38b1659739..3767bb15d8 100644 --- a/components/settings/parser.cpp +++ b/components/settings/parser.cpp @@ -62,13 +62,13 @@ void Settings::SettingsFileParser::loadSettingsFile(const std::string& file, Cat } } -void Settings::SettingsFileParser::saveSettingsFile(const std::string& file, CategorySettingValueMap& settings) +void Settings::SettingsFileParser::saveSettingsFile(const std::string& file, const CategorySettingValueMap& settings) { using CategorySettingStatusMap = std::map; // No options have been written to the file yet. CategorySettingStatusMap written; - for (CategorySettingValueMap::iterator it = settings.begin(); it != settings.end(); ++it) { + for (auto it = settings.begin(); it != settings.end(); ++it) { written[it->first] = false; } @@ -133,8 +133,8 @@ void Settings::SettingsFileParser::saveSettingsFile(const std::string& file, Cat for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { if (mit->second == false && mit->first.first == currentCategory) { Log(Debug::Verbose) << "Added new setting: [" << currentCategory << "] " - << mit->first.second << " = " << settings[mit->first]; - ostream << mit->first.second << " = " << settings[mit->first] << std::endl; + << mit->first.second << " = " << settings.at(mit->first); + ostream << mit->first.second << " = " << settings.at(mit->first) << std::endl; mit->second = true; changed = true; } @@ -200,13 +200,13 @@ void Settings::SettingsFileParser::saveSettingsFile(const std::string& file, Cat // Write the current value of the setting to the file. The key must exist in the // settings map because of how written was initialized and finder != end(). - ostream << setting << " = " << settings[key] << std::endl; + ostream << setting << " = " << settings.at(key) << std::endl; // Mark that setting as written. finder->second = true; // Did we really change it? - if (value != settings[key]) { + if (value != settings.at(key)) { Log(Debug::Verbose) << "Changed setting: [" << currentCategory << "] " - << setting << " = " << settings[key]; + << setting << " = " << settings.at(key); changed = true; } // No need to write the current line, because we just emitted a replacement. @@ -223,8 +223,8 @@ void Settings::SettingsFileParser::saveSettingsFile(const std::string& file, Cat for (CategorySettingStatusMap::iterator mit = written.begin(); mit != written.end(); ++mit) { if (mit->second == false && mit->first.first == currentCategory) { Log(Debug::Verbose) << "Added new setting: [" << mit->first.first << "] " - << mit->first.second << " = " << settings[mit->first]; - ostream << mit->first.second << " = " << settings[mit->first] << std::endl; + << mit->first.second << " = " << settings.at(mit->first); + ostream << mit->first.second << " = " << settings.at(mit->first) << std::endl; mit->second = true; changed = true; } @@ -256,9 +256,9 @@ void Settings::SettingsFileParser::saveSettingsFile(const std::string& file, Cat ostream << "[" << currentCategory << "]" << std::endl; } Log(Debug::Verbose) << "Added new setting: [" << mit->first.first << "] " - << mit->first.second << " = " << settings[mit->first]; + << mit->first.second << " = " << settings.at(mit->first); // Then write the setting. No need to mark it as written because we're done. - ostream << mit->first.second << " = " << settings[mit->first] << std::endl; + ostream << mit->first.second << " = " << settings.at(mit->first) << std::endl; changed = true; } } diff --git a/components/settings/parser.hpp b/components/settings/parser.hpp index bd319e01cc..449e542235 100644 --- a/components/settings/parser.hpp +++ b/components/settings/parser.hpp @@ -12,7 +12,7 @@ namespace Settings public: void loadSettingsFile(const std::string& file, CategorySettingValueMap& settings); - void saveSettingsFile(const std::string& file, CategorySettingValueMap& settings); + void saveSettingsFile(const std::string& file, const CategorySettingValueMap& settings); private: /// Increment i until it longer points to a whitespace character