From e382f71aea92d05578a41bd6469601549f64561b Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Thu, 7 Oct 2021 00:39:23 +0100 Subject: [PATCH 01/11] Add implementation of config file parser lifted from Boost --- components/CMakeLists.txt | 2 +- components/files/configfileparser.cpp | 289 ++++++++++++++++++++++ components/files/configfileparser.hpp | 17 ++ components/files/configurationmanager.cpp | 3 +- 4 files changed, 309 insertions(+), 2 deletions(-) create mode 100644 components/files/configfileparser.cpp create mode 100644 components/files/configfileparser.hpp diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index 7983de6190..ccac8f8cb7 100644 --- a/components/CMakeLists.txt +++ b/components/CMakeLists.txt @@ -105,7 +105,7 @@ IF(NOT WIN32 AND NOT APPLE) ENDIF() add_component_dir (files linuxpath androidpath windowspath macospath fixedpath multidircollection collections configurationmanager escape - lowlevelfile constrainedfilestream memorystream + lowlevelfile constrainedfilestream memorystream configfileparser ) add_component_dir (compiler diff --git a/components/files/configfileparser.cpp b/components/files/configfileparser.cpp new file mode 100644 index 0000000000..7c43e1f67c --- /dev/null +++ b/components/files/configfileparser.cpp @@ -0,0 +1,289 @@ +#include "configfileparser.hpp" + +#include +#include + +namespace Files +{ + namespace + { + /** Standalone parser for config files in ini-line format. + The parser is a model of single-pass lvalue iterator, and + default constructor creates past-the-end-iterator. The typical usage is: + config_file_iterator i(is, ... set of options ...), e; + for(; i !=e; ++i) { + *i; + } + + Syntax conventions: + + - config file can not contain positional options + - '#' is comment character: it is ignored together with + the rest of the line. + - variable assignments are in the form + name '=' value. + spaces around '=' are trimmed. + - Section names are given in brackets. + + The actual option name is constructed by combining current section + name and specified option name, with dot between. If section_name + already contains dot at the end, new dot is not inserted. For example: + @verbatim + [gui.accessibility] + visual_bell=yes + @endverbatim + will result in option "gui.accessibility.visual_bell" with value + "yes" been returned. + + TODO: maybe, we should just accept a pointer to options_description + class. + */ + class common_config_file_iterator + : public boost::eof_iterator + { + public: + common_config_file_iterator() { found_eof(); } + common_config_file_iterator( + const std::set& allowed_options, + bool allow_unregistered = false); + + virtual ~common_config_file_iterator() {} + + public: // Method required by eof_iterator + + void get(); + +#if BOOST_WORKAROUND(_MSC_VER, <= 1900) + void decrement() {} + void advance(difference_type) {} +#endif + + protected: // Stubs for derived classes + + // Obtains next line from the config file + // Note: really, this design is a bit ugly + // The most clean thing would be to pass 'line_iterator' to + // constructor of this class, but to avoid templating this class + // we'd need polymorphic iterator, which does not exist yet. + virtual bool getline(std::string&) { return false; } + + protected: + /** Adds another allowed option. If the 'name' ends with + '*', then all options with the same prefix are + allowed. For example, if 'name' is 'foo*', then 'foo1' and + 'foo_bar' are allowed. */ + void add_option(const char* name); + + // Returns true if 's' is a registered option name. + bool allowed_option(const std::string& s) const; + + // That's probably too much data for iterator, since + // it will be copied, but let's not bother for now. + std::set allowed_options; + // Invariant: no element is prefix of other element. + std::set allowed_prefixes; + std::string m_prefix; + bool m_allow_unregistered; + }; + + common_config_file_iterator::common_config_file_iterator( + const std::set& allowed_options, + bool allow_unregistered) + : allowed_options(allowed_options), + m_allow_unregistered(allow_unregistered) + { + for (std::set::const_iterator i = allowed_options.begin(); + i != allowed_options.end(); + ++i) + { + add_option(i->c_str()); + } + } + + void common_config_file_iterator::add_option(const char* name) + { + std::string s(name); + assert(!s.empty()); + if (*s.rbegin() == '*') { + s.resize(s.size() - 1); + bool bad_prefixes(false); + // If 's' is a prefix of one of allowed suffix, then + // lower_bound will return that element. + // If some element is prefix of 's', then lower_bound will + // return the next element. + std::set::iterator i = allowed_prefixes.lower_bound(s); + if (i != allowed_prefixes.end()) { + if (i->find(s) == 0) + bad_prefixes = true; + } + if (i != allowed_prefixes.begin()) { + --i; + if (s.find(*i) == 0) + bad_prefixes = true; + } + if (bad_prefixes) + boost::throw_exception(bpo::error("options '" + std::string(name) + "' and '" + + *i + "*' will both match the same " + "arguments from the configuration file")); + allowed_prefixes.insert(s); + } + } + + std::string trim_ws(const std::string& s) + { + std::string::size_type n, n2; + n = s.find_first_not_of(" \t\r\n"); + if (n == std::string::npos) + return std::string(); + else { + n2 = s.find_last_not_of(" \t\r\n"); + return s.substr(n, n2 - n + 1); + } + } + + void common_config_file_iterator::get() + { + std::string s; + std::string::size_type n; + bool found = false; + + while (this->getline(s)) { + + // strip '#' comments and whitespace + if ((n = s.find('#')) != std::string::npos) + s = s.substr(0, n); + s = trim_ws(s); + + if (!s.empty()) { + // Handle section name + if (*s.begin() == '[' && *s.rbegin() == ']') { + m_prefix = s.substr(1, s.size() - 2); + if (*m_prefix.rbegin() != '.') + m_prefix += '.'; + } + else if ((n = s.find('=')) != std::string::npos) { + + std::string name = m_prefix + trim_ws(s.substr(0, n)); + std::string value = trim_ws(s.substr(n + 1)); + + bool registered = allowed_option(name); + if (!registered && !m_allow_unregistered) + boost::throw_exception(bpo::unknown_option(name)); + + found = true; + this->value().string_key = name; + this->value().value.clear(); + this->value().value.push_back(value); + this->value().unregistered = !registered; + this->value().original_tokens.clear(); + this->value().original_tokens.push_back(name); + this->value().original_tokens.push_back(value); + break; + + } else { + boost::throw_exception(bpo::invalid_config_file_syntax(s, bpo::invalid_syntax::unrecognized_line)); + } + } + } + if (!found) + found_eof(); + } + + bool common_config_file_iterator::allowed_option(const std::string& s) const + { + std::set::const_iterator i = allowed_options.find(s); + if (i != allowed_options.end()) + return true; + // If s is "pa" where "p" is allowed prefix then + // lower_bound should find the element after "p". + // This depends on 'allowed_prefixes' invariant. + i = allowed_prefixes.lower_bound(s); + if (i != allowed_prefixes.begin() && s.find(*--i) == 0) + return true; + return false; + } + + + + template + class basic_config_file_iterator : public Files::common_config_file_iterator { + public: + basic_config_file_iterator() + { + found_eof(); + } + + /** Creates a config file parser for the specified stream. + */ + basic_config_file_iterator(std::basic_istream& is, + const std::set& allowed_options, + bool allow_unregistered = false); + + private: // base overrides + + bool getline(std::string&); + + private: // internal data + std::shared_ptr > is; + }; + + template + basic_config_file_iterator:: + basic_config_file_iterator(std::basic_istream& is, + const std::set& allowed_options, + bool allow_unregistered) + : common_config_file_iterator(allowed_options, allow_unregistered) + { + this->is.reset(&is, bpo::detail::null_deleter()); + get(); + } + + template + bool basic_config_file_iterator::getline(std::string& s) + { + std::basic_string in; + if (std::getline(*is, in)) { + s = bpo::to_internal(in); + return true; + } else { + return false; + } + } + } + + template + bpo::basic_parsed_options + parse_config_file(std::basic_istream& is, + const bpo::options_description& desc, + bool allow_unregistered) + { + std::set allowed_options; + + const std::vector >& options = desc.options(); + for (unsigned i = 0; i < options.size(); ++i) + { + const bpo::option_description& d = *options[i]; + + if (d.long_name().empty()) + boost::throw_exception( + bpo::error("abbreviated option names are not permitted in options configuration files")); + + allowed_options.insert(d.long_name()); + } + + // Parser return char strings + bpo::parsed_options result(&desc); + copy(basic_config_file_iterator( + is, allowed_options, allow_unregistered), + basic_config_file_iterator(), + back_inserter(result.options)); + // Convert char strings into desired type. + return bpo::basic_parsed_options(result); + } + + template + bpo::basic_parsed_options + parse_config_file(std::basic_istream& is, + const bpo::options_description& desc, + bool allow_unregistered); +} diff --git a/components/files/configfileparser.hpp b/components/files/configfileparser.hpp new file mode 100644 index 0000000000..60ab27a6d4 --- /dev/null +++ b/components/files/configfileparser.hpp @@ -0,0 +1,17 @@ +#ifndef COMPONENTS_FILES_CONFIGFILEPARSER_HPP +#define COMPONENTS_FILES_CONFIGFILEPARSER_HPP + +#include + +namespace Files +{ + +namespace bpo = boost::program_options; + +template +bpo::basic_parsed_options parse_config_file(std::basic_istream&, const bpo::options_description&, + bool allow_unregistered = false); + +} + +#endif // COMPONENTS_FILES_CONFIGFILEPARSER_HPP \ No newline at end of file diff --git a/components/files/configurationmanager.cpp b/components/files/configurationmanager.cpp index 35679ef293..7fd134cf0f 100644 --- a/components/files/configurationmanager.cpp +++ b/components/files/configurationmanager.cpp @@ -1,6 +1,7 @@ #include "configurationmanager.hpp" #include +#include #include #include @@ -239,7 +240,7 @@ bool ConfigurationManager::loadConfig(const boost::filesystem::path& path, configFileStream.push(configFileStreamUnfiltered); if (configFileStreamUnfiltered.is_open()) { - boost::program_options::store(boost::program_options::parse_config_file( + boost::program_options::store(Files::parse_config_file( configFileStream, description, true), variables); return true; From cf3596fbb4bd05f30c586f5a0a4111c038f313d4 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Thu, 7 Oct 2021 00:44:50 +0100 Subject: [PATCH 02/11] Add copyright preamble --- components/files/configfileparser.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/components/files/configfileparser.cpp b/components/files/configfileparser.cpp index 7c43e1f67c..fe98ea11d2 100644 --- a/components/files/configfileparser.cpp +++ b/components/files/configfileparser.cpp @@ -1,3 +1,11 @@ +// This file's contents is largely lifted from boost::program_options with only minor modification. +// Its original preamble (without updated dates) from those source files is below: + +// Copyright Vladimir Prus 2002-2004. +// Distributed under the Boost Software License, Version 1.0. +// (See accompanying file LICENSE_1_0.txt +// or copy at http://www.boost.org/LICENSE_1_0.txt) + #include "configfileparser.hpp" #include From af0d399103e86964862286f9248589f140874f08 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sat, 9 Oct 2021 01:49:08 +0100 Subject: [PATCH 03/11] Purge all uses of Escape Hash types --- apps/opencs/editor.cpp | 26 ++++----- apps/openmw/main.cpp | 54 +++++++++---------- apps/openmw_test_suite/mwworld/test_store.cpp | 12 ++--- components/files/configfileparser.cpp | 4 +- components/files/configurationmanager.cpp | 23 ++++---- 5 files changed, 58 insertions(+), 61 deletions(-) diff --git a/apps/opencs/editor.cpp b/apps/opencs/editor.cpp index 3f53a523f4..b437a36201 100644 --- a/apps/opencs/editor.cpp +++ b/apps/opencs/editor.cpp @@ -88,16 +88,16 @@ std::pair > CS::Editor::readConfi boost::program_options::options_description desc("Syntax: openmw-cs \nAllowed options"); desc.add_options() - ("data", boost::program_options::value()->default_value(Files::EscapePathContainer(), "data")->multitoken()->composing()) - ("data-local", boost::program_options::value()->default_value(Files::EscapePath(), "")) + ("data", boost::program_options::value()->default_value(Files::PathContainer(), "data")->multitoken()->composing()) + ("data-local", boost::program_options::value()->default_value(Files::PathContainer::value_type(), "")) ("fs-strict", boost::program_options::value()->implicit_value(true)->default_value(false)) - ("encoding", boost::program_options::value()->default_value("win1252")) - ("resources", boost::program_options::value()->default_value(Files::EscapePath(), "resources")) - ("fallback-archive", boost::program_options::value()-> - default_value(Files::EscapeStringVector(), "fallback-archive")->multitoken()) + ("encoding", boost::program_options::value()->default_value("win1252")) + ("resources", boost::program_options::value()->default_value(boost::filesystem::path(), "resources")) + ("fallback-archive", boost::program_options::value>()-> + default_value(std::vector(), "fallback-archive")->multitoken()) ("fallback", boost::program_options::value()->default_value(FallbackMap(), "") ->multitoken()->composing(), "fallback values") - ("script-blacklist", boost::program_options::value()->default_value(Files::EscapeStringVector(), "") + ("script-blacklist", boost::program_options::value>()->default_value(std::vector(), "") ->multitoken(), "exclude specified script from the verifier (if the use of the blacklist is enabled)") ("script-blacklist-use", boost::program_options::value()->implicit_value(true) ->default_value(true), "enable script blacklisting"); @@ -108,24 +108,24 @@ std::pair > CS::Editor::readConfi Fallback::Map::init(variables["fallback"].as().mMap); - mEncodingName = variables["encoding"].as().toStdString(); + mEncodingName = variables["encoding"].as(); mDocumentManager.setEncoding(ToUTF8::calculateEncoding(mEncodingName)); mFileDialog.setEncoding (QString::fromUtf8(mEncodingName.c_str())); - mDocumentManager.setResourceDir (mResources = variables["resources"].as().mPath); + mDocumentManager.setResourceDir (mResources = variables["resources"].as()); if (variables["script-blacklist-use"].as()) mDocumentManager.setBlacklistedScripts ( - variables["script-blacklist"].as().toStdStringVector()); + variables["script-blacklist"].as>()); mFsStrict = variables["fs-strict"].as(); Files::PathContainer dataDirs, dataLocal; if (!variables["data"].empty()) { - dataDirs = Files::PathContainer(Files::EscapePath::toPathContainer(variables["data"].as())); + dataDirs = variables["data"].as(); } - Files::PathContainer::value_type local(variables["data-local"].as().mPath); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) dataLocal.push_back(local); @@ -155,7 +155,7 @@ std::pair > CS::Editor::readConfi mFileDialog.addFiles(path); } - return std::make_pair (dataDirs, variables["fallback-archive"].as().toStdStringVector()); + return std::make_pair (dataDirs, variables["fallback-archive"].as>()); } void CS::Editor::createGame() diff --git a/apps/openmw/main.cpp b/apps/openmw/main.cpp index de0fb0df03..5de177a907 100644 --- a/apps/openmw/main.cpp +++ b/apps/openmw/main.cpp @@ -45,31 +45,31 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat ("help", "print help message") ("version", "print version information and quit") - ("replace", bpo::value()->default_value(Files::EscapeStringVector(), "") + ("replace", bpo::value()->default_value(StringsVector(), "") ->multitoken()->composing(), "settings where the values from the current source should replace those from lower-priority sources instead of being appended") - ("data", bpo::value()->default_value(Files::EscapePathContainer(), "data") + ("data", bpo::value()->default_value(Files::PathContainer(), "data") ->multitoken()->composing(), "set data directories (later directories have higher priority)") - ("data-local", bpo::value()->default_value(Files::EscapePath(), ""), + ("data-local", bpo::value()->default_value(Files::PathContainer::value_type(), ""), "set local data directory (highest priority)") - ("fallback-archive", bpo::value()->default_value(Files::EscapeStringVector(), "fallback-archive") + ("fallback-archive", bpo::value()->default_value(StringsVector(), "fallback-archive") ->multitoken()->composing(), "set fallback BSA archives (later archives have higher priority)") - ("resources", bpo::value()->default_value(Files::EscapePath(), "resources"), + ("resources", bpo::value()->default_value(boost::filesystem::path(), "resources"), "set resources directory") - ("start", bpo::value()->default_value(""), + ("start", bpo::value()->default_value(""), "set initial cell") - ("content", bpo::value()->default_value(Files::EscapeStringVector(), "") + ("content", bpo::value()->default_value(StringsVector(), "") ->multitoken()->composing(), "content file(s): esm/esp, or omwgame/omwaddon") - ("groundcover", bpo::value()->default_value(Files::EscapeStringVector(), "") + ("groundcover", bpo::value()->default_value(StringsVector(), "") ->multitoken()->composing(), "groundcover content file(s): esm/esp, or omwgame/omwaddon") - ("lua-scripts", bpo::value()->default_value(Files::EscapeStringVector(), "") + ("lua-scripts", bpo::value()->default_value(StringsVector(), "") ->multitoken()->composing(), "file(s) with a list of global Lua scripts: omwscripts") ("no-sound", bpo::value()->implicit_value(true) @@ -84,7 +84,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat ("script-console", bpo::value()->implicit_value(true) ->default_value(false), "enable console-only script functionality") - ("script-run", bpo::value()->default_value(""), + ("script-run", bpo::value()->default_value(""), "select a file containing a list of console commands that is executed on startup") ("script-warn", bpo::value()->implicit_value (1) @@ -94,13 +94,13 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat "\t1 - show warning but consider script as correctly compiled anyway\n" "\t2 - treat warnings as errors") - ("script-blacklist", bpo::value()->default_value(Files::EscapeStringVector(), "") + ("script-blacklist", bpo::value()->default_value(StringsVector(), "") ->multitoken()->composing(), "ignore the specified script (if the use of the blacklist is enabled)") ("script-blacklist-use", bpo::value()->implicit_value(true) ->default_value(true), "enable script blacklisting") - ("load-savegame", bpo::value()->default_value(Files::EscapePath(), ""), + ("load-savegame", bpo::value()->default_value(boost::filesystem::path(), ""), "load a save game file on game startup (specify an absolute filename or a filename relative to the current working directory)") ("skip-menu", bpo::value()->implicit_value(true) @@ -112,7 +112,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat ("fs-strict", bpo::value()->implicit_value(true) ->default_value(false), "strict file system handling (no case folding)") - ("encoding", bpo::value()-> + ("encoding", bpo::value()-> default_value("win1252"), "Character encoding used in OpenMW game messages:\n" "\n\twin1250 - Central and Eastern European such as Polish, Czech, Slovak, Hungarian, Slovene, Bosnian, Croatian, Serbian (Latin script), Romanian and Albanian languages\n" @@ -153,7 +153,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat { cfgMgr.readConfiguration(variables, desc, true); - Version::Version v = Version::getOpenmwVersion(variables["resources"].as().mPath.string()); + Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); getRawStdout() << v.describe() << std::endl; return false; } @@ -162,38 +162,38 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat cfgMgr.readConfiguration(variables, desc); cfgMgr.mergeComposingVariables(variables, composingVariables, desc); - Version::Version v = Version::getOpenmwVersion(variables["resources"].as().mPath.string()); + Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); Log(Debug::Info) << v.describe(); engine.setGrabMouse(!variables["no-grab"].as()); // Font encoding settings - std::string encoding(variables["encoding"].as().toStdString()); + std::string encoding(variables["encoding"].as()); Log(Debug::Info) << ToUTF8::encodingUsingMessage(encoding); engine.setEncoding(ToUTF8::calculateEncoding(encoding)); // directory settings engine.enableFSStrict(variables["fs-strict"].as()); - Files::PathContainer dataDirs(Files::EscapePath::toPathContainer(variables["data"].as())); + Files::PathContainer dataDirs(variables["data"].as()); - Files::PathContainer::value_type local(variables["data-local"].as().mPath); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) dataDirs.push_back(local); cfgMgr.processPaths(dataDirs); - engine.setResourceDir(variables["resources"].as().mPath); + engine.setResourceDir(variables["resources"].as()); engine.setDataDirs(dataDirs); // fallback archives - StringsVector archives = variables["fallback-archive"].as().toStdStringVector(); + StringsVector archives = variables["fallback-archive"].as(); for (StringsVector::const_iterator it = archives.begin(); it != archives.end(); ++it) { engine.addArchive(*it); } - StringsVector content = variables["content"].as().toStdStringVector(); + StringsVector content = variables["content"].as(); if (content.empty()) { Log(Debug::Error) << "No content file given (esm/esp, nor omwgame/omwaddon). Aborting..."; @@ -214,18 +214,18 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat engine.addContentFile(file); } - StringsVector groundcover = variables["groundcover"].as().toStdStringVector(); + StringsVector groundcover = variables["groundcover"].as(); for (auto& file : groundcover) { engine.addGroundcoverFile(file); } - StringsVector luaScriptLists = variables["lua-scripts"].as().toStdStringVector(); + StringsVector luaScriptLists = variables["lua-scripts"].as(); for (const auto& file : luaScriptLists) engine.addLuaScriptListFile(file); // startup-settings - engine.setCell(variables["start"].as().toStdString()); + engine.setCell(variables["start"].as()); engine.setSkipMenu (variables["skip-menu"].as(), variables["new-game"].as()); if (!variables["skip-menu"].as() && variables["new-game"].as()) Log(Debug::Warning) << "Warning: new-game used without skip-menu -> ignoring it"; @@ -234,11 +234,11 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat engine.setCompileAll(variables["script-all"].as()); engine.setCompileAllDialogue(variables["script-all-dialogue"].as()); engine.setScriptConsoleMode (variables["script-console"].as()); - engine.setStartupScript (variables["script-run"].as().toStdString()); + engine.setStartupScript (variables["script-run"].as()); engine.setWarningsMode (variables["script-warn"].as()); - engine.setScriptBlacklist (variables["script-blacklist"].as().toStdStringVector()); + engine.setScriptBlacklist (variables["script-blacklist"].as()); engine.setScriptBlacklistUse (variables["script-blacklist-use"].as()); - engine.setSaveGameFile (variables["load-savegame"].as().mPath.string()); + engine.setSaveGameFile (variables["load-savegame"].as().string()); // other settings Fallback::Map::init(variables["fallback"].as().mMap); diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index f3b2bb3dcb..13a9ebee9c 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/apps/openmw_test_suite/mwworld/test_store.cpp @@ -59,10 +59,10 @@ struct ContentFileTest : public ::testing::Test boost::program_options::options_description desc("Allowed options"); desc.add_options() - ("data", boost::program_options::value()->default_value(Files::EscapePathContainer(), "data")->multitoken()->composing()) - ("content", boost::program_options::value()->default_value(Files::EscapeStringVector(), "") + ("data", boost::program_options::value()->default_value(Files::PathContainer(), "data")->multitoken()->composing()) + ("content", boost::program_options::value>()->default_value(std::vector(), "") ->multitoken()->composing(), "content file(s): esm/esp, or omwgame/omwaddon") - ("data-local", boost::program_options::value()->default_value(Files::EscapePath(), "")); + ("data-local", boost::program_options::value()->default_value(Files::PathContainer::value_type(), "")); boost::program_options::notify(variables); @@ -70,10 +70,10 @@ struct ContentFileTest : public ::testing::Test Files::PathContainer dataDirs, dataLocal; if (!variables["data"].empty()) { - dataDirs = Files::EscapePath::toPathContainer(variables["data"].as()); + dataDirs = variables["data"].as(); } - Files::PathContainer::value_type local(variables["data-local"].as().mPath); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) { dataLocal.push_back(local); } @@ -86,7 +86,7 @@ struct ContentFileTest : public ::testing::Test Files::Collections collections (dataDirs, true); - std::vector contentFiles = variables["content"].as().toStdStringVector(); + std::vector contentFiles = variables["content"].as>(); for (auto & contentFile : contentFiles) mContentFiles.push_back(collections.getPath(contentFile)); } diff --git a/components/files/configfileparser.cpp b/components/files/configfileparser.cpp index fe98ea11d2..7fcc033157 100644 --- a/components/files/configfileparser.cpp +++ b/components/files/configfileparser.cpp @@ -158,8 +158,8 @@ namespace Files while (this->getline(s)) { // strip '#' comments and whitespace - if ((n = s.find('#')) != std::string::npos) - s = s.substr(0, n); + if (s.find('#') == s.find_first_not_of(" \t\r\n")) + continue; s = trim_ws(s); if (!s.empty()) { diff --git a/components/files/configurationmanager.cpp b/components/files/configurationmanager.cpp index 7fd134cf0f..8aa6853237 100644 --- a/components/files/configurationmanager.cpp +++ b/components/files/configurationmanager.cpp @@ -108,7 +108,7 @@ void ConfigurationManager::mergeComposingVariables(boost::program_options::varia auto replace = second["replace"]; if (!replace.defaulted() && !replace.empty()) { - std::vector replaceVector = replace.as().toStdStringVector(); + std::vector replaceVector = replace.as>(); replacedVariables.insert(replaceVector.begin(), replaceVector.end()); } } @@ -137,19 +137,19 @@ void ConfigurationManager::mergeComposingVariables(boost::program_options::varia boost::any& firstValue = firstPosition->second.value(); const boost::any& secondValue = second[name].value(); - if (firstValue.type() == typeid(Files::EscapePathContainer)) + if (firstValue.type() == typeid(Files::PathContainer)) { - auto& firstPathContainer = boost::any_cast(firstValue); - const auto& secondPathContainer = boost::any_cast(secondValue); + auto& firstPathContainer = boost::any_cast(firstValue); + const auto& secondPathContainer = boost::any_cast(secondValue); firstPathContainer.insert(firstPathContainer.end(), secondPathContainer.begin(), secondPathContainer.end()); } - else if (firstValue.type() == typeid(Files::EscapeStringVector)) + else if (firstValue.type() == typeid(std::vector)) { - auto& firstVector = boost::any_cast(firstValue); - const auto& secondVector = boost::any_cast(secondValue); + auto& firstVector = boost::any_cast&>(firstValue); + const auto& secondVector = boost::any_cast&>(secondValue); - firstVector.mVector.insert(firstVector.mVector.end(), secondVector.mVector.begin(), secondVector.mVector.end()); + firstVector.insert(firstVector.end(), secondVector.begin(), secondVector.end()); } else if (firstValue.type() == typeid(Fallback::FallbackMap)) { @@ -234,11 +234,8 @@ bool ConfigurationManager::loadConfig(const boost::filesystem::path& path, if (!mSilent) Log(Debug::Info) << "Loading config file: " << cfgFile.string(); - boost::filesystem::ifstream configFileStreamUnfiltered(cfgFile); - boost::iostreams::filtering_istream configFileStream; - configFileStream.push(escape_hash_filter()); - configFileStream.push(configFileStreamUnfiltered); - if (configFileStreamUnfiltered.is_open()) + boost::filesystem::ifstream configFileStream(cfgFile); + if (configFileStream.is_open()) { boost::program_options::store(Files::parse_config_file( configFileStream, description, true), variables); From 7fb5b773dde512986d964138eada0e606f87555b Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 11 Oct 2021 00:01:58 +0100 Subject: [PATCH 04/11] Remove obsolete tests The old behaviour was basically a bug and @ doesn't mean anything special now --- apps/openmw_test_suite/openmw/options.cpp | 27 +---------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/apps/openmw_test_suite/openmw/options.cpp b/apps/openmw_test_suite/openmw/options.cpp index 9de7613d19..0f9ad39053 100644 --- a/apps/openmw_test_suite/openmw/options.cpp +++ b/apps/openmw_test_suite/openmw/options.cpp @@ -22,16 +22,11 @@ namespace { std::vector result = std::move(base); for (int i = 1; i <= std::numeric_limits::max(); ++i) - if (i != '&' && i != '"' && i != ' ' && i != '@' && i != '\n') + if (i != '&' && i != '"' && i != ' ' && i != '\n') result.push_back(std::string(1, i)); return result; } - constexpr std::array supportedAtSignEscapings { - std::pair {'a', '@'}, - std::pair {'h', '#'}, - }; - MATCHER_P(IsPath, v, "") { return arg.string() == v; } template @@ -101,7 +96,6 @@ namespace const std::array arguments {"openmw", "--load-savegame", "my@save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); -// EXPECT_EQ(variables["load-savegame"].as().string(), "my?ave.omwsave"); EXPECT_EQ(variables["load-savegame"].as().string(), "my@save.omwsave"); } @@ -205,25 +199,6 @@ namespace ValuesIn(generateSupportedCharacters({u8"👍", u8"Ъ", u8"Ǽ", "\n"})) ); - struct OpenMWOptionsFromArgumentsEscapings : TestWithParam> {}; - -/* TEST_P(OpenMWOptionsFromArgumentsEscapings, should_support_escaping_with_at_sign_in_load_savegame_path) - { - bpo::options_description description = makeOptionsDescription(); - const std::string path = "save_@" + std::string(1, GetParam().first) + ".omwsave"; - const std::array arguments {"openmw", "--load-savegame", path.c_str()}; - bpo::variables_map variables; - parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), - "save_" + std::string(1, GetParam().second) + ".omwsave"); - } - - INSTANTIATE_TEST_SUITE_P( - SupportedEscapingsWithAtSign, - OpenMWOptionsFromArgumentsEscapings, - ValuesIn(supportedAtSignEscapings) - );*/ - TEST(OpenMWOptionsFromConfig, should_support_single_word_load_savegame_path) { bpo::options_description description = makeOptionsDescription(); From 18869c2c7505d5fe80b38351be8b0edf74f9ed6e Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 11 Oct 2021 00:02:33 +0100 Subject: [PATCH 05/11] Add some extra tests The behaviour here is unchanged, but was previously untested. --- apps/openmw_test_suite/openmw/options.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/apps/openmw_test_suite/openmw/options.cpp b/apps/openmw_test_suite/openmw/options.cpp index 0f9ad39053..b087fd03fb 100644 --- a/apps/openmw_test_suite/openmw/options.cpp +++ b/apps/openmw_test_suite/openmw/options.cpp @@ -272,6 +272,24 @@ namespace EXPECT_EQ(variables["load-savegame"].as().string(), ""); } + TEST(OpenMWOptionsFromConfig, should_ignore_whitespace_prefixed_commented_option) + { + bpo::options_description description = makeOptionsDescription(); + std::istringstream stream(" \t#load-savegame=save.omwsave"); + bpo::variables_map variables; + Files::parseConfig(stream, variables, description); + EXPECT_EQ(variables["load-savegame"].as().string(), ""); + } + + TEST(OpenMWOptionsFromConfig, should_support_whitespace_around_option) + { + bpo::options_description description = makeOptionsDescription(); + std::istringstream stream(" load-savegame = save.omwsave "); + bpo::variables_map variables; + Files::parseConfig(stream, variables, description); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + } + TEST(OpenMWOptionsFromConfig, should_throw_on_multiple_load_savegame) { bpo::options_description description = makeOptionsDescription(); From 9be606a40de19ea7636694beda2f8790f605774b Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 17 Oct 2021 23:40:34 +0100 Subject: [PATCH 06/11] Finish removing old Escape classes --- apps/openmw/main.cpp | 1 - apps/openmw/options.cpp | 2 +- apps/openmw_test_suite/mwworld/test_store.cpp | 1 - apps/openmw_test_suite/openmw/options.cpp | 1 - components/CMakeLists.txt | 2 +- components/fallback/validate.cpp | 9 +- components/fallback/validate.hpp | 2 - components/files/configurationmanager.cpp | 1 - components/files/escape.cpp | 145 ------------- components/files/escape.hpp | 191 ------------------ 10 files changed, 6 insertions(+), 349 deletions(-) delete mode 100644 components/files/escape.cpp delete mode 100644 components/files/escape.hpp diff --git a/apps/openmw/main.cpp b/apps/openmw/main.cpp index 18b1ab267f..7a3a21b149 100644 --- a/apps/openmw/main.cpp +++ b/apps/openmw/main.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include #include diff --git a/apps/openmw/options.cpp b/apps/openmw/options.cpp index 167c275598..dd94590f2a 100644 --- a/apps/openmw/options.cpp +++ b/apps/openmw/options.cpp @@ -1,7 +1,7 @@ #include "options.hpp" #include -#include +#include #include #include diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index 13a9ebee9c..9aa5a6758f 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/apps/openmw_test_suite/mwworld/test_store.cpp @@ -3,7 +3,6 @@ #include #include -#include #include #include #include diff --git a/apps/openmw_test_suite/openmw/options.cpp b/apps/openmw_test_suite/openmw/options.cpp index b087fd03fb..7cf0df1066 100644 --- a/apps/openmw_test_suite/openmw/options.cpp +++ b/apps/openmw_test_suite/openmw/options.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index b9db26dbc1..b61ba11466 100644 --- a/components/CMakeLists.txt +++ b/components/CMakeLists.txt @@ -104,7 +104,7 @@ IF(NOT WIN32 AND NOT APPLE) add_definitions(-DGLOBAL_CONFIG_PATH="${GLOBAL_CONFIG_PATH}") ENDIF() add_component_dir (files - linuxpath androidpath windowspath macospath fixedpath multidircollection collections configurationmanager escape + linuxpath androidpath windowspath macospath fixedpath multidircollection collections configurationmanager lowlevelfile constrainedfilestream memorystream configfileparser ) diff --git a/components/fallback/validate.cpp b/components/fallback/validate.cpp index a482d40faf..6f5d529e05 100644 --- a/components/fallback/validate.cpp +++ b/components/fallback/validate.cpp @@ -11,13 +11,12 @@ void Fallback::validate(boost::any& v, std::vector const& tokens, F for (const auto& token : tokens) { - std::string temp = Files::EscapeHashString::processString(token); - size_t sep = temp.find(','); - if (sep < 1 || sep == temp.length() - 1 || sep == std::string::npos) + size_t sep = token.find(','); + if (sep < 1 || sep == token.length() - 1 || sep == std::string::npos) throw boost::program_options::validation_error(boost::program_options::validation_error::invalid_option_value); - std::string key(temp.substr(0, sep)); - std::string value(temp.substr(sep + 1)); + std::string key(token.substr(0, sep)); + std::string value(token.substr(sep + 1)); map->mMap[key] = value; } diff --git a/components/fallback/validate.hpp b/components/fallback/validate.hpp index 96690f50a5..2b9af88da2 100644 --- a/components/fallback/validate.hpp +++ b/components/fallback/validate.hpp @@ -3,8 +3,6 @@ #include -#include - // Parses and validates a fallback map from boost program_options. // Note: for boost to pick up the validate function, you need to pull in the namespace e.g. // by using namespace Fallback; diff --git a/components/files/configurationmanager.cpp b/components/files/configurationmanager.cpp index dad52a7ee4..23c25fd413 100644 --- a/components/files/configurationmanager.cpp +++ b/components/files/configurationmanager.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include diff --git a/components/files/escape.cpp b/components/files/escape.cpp deleted file mode 100644 index 8b11504d34..0000000000 --- a/components/files/escape.cpp +++ /dev/null @@ -1,145 +0,0 @@ -#include "escape.hpp" - -#include - -namespace Files -{ - const int escape_hash_filter::sEscape = '@'; - const int escape_hash_filter::sEscapeIdentifier = 'a'; - const int escape_hash_filter::sHashIdentifier = 'h'; - - escape_hash_filter::escape_hash_filter() : mSeenNonWhitespace(false), mFinishLine(false) - { - } - - escape_hash_filter::~escape_hash_filter() - { - } - - unescape_hash_filter::unescape_hash_filter() : expectingIdentifier(false) - { - } - - unescape_hash_filter::~unescape_hash_filter() - { - } - - std::string EscapeHashString::processString(const std::string & str) - { - std::string temp = str; - - static const char hash[] = { escape_hash_filter::sEscape, escape_hash_filter::sHashIdentifier }; - Misc::StringUtils::replaceAll(temp, hash, "#", 2, 1); - - static const char escape[] = { escape_hash_filter::sEscape, escape_hash_filter::sEscapeIdentifier }; - Misc::StringUtils::replaceAll(temp, escape, "@", 2, 1); - - return temp; - } - - EscapeHashString::EscapeHashString() : mData() - { - } - - EscapeHashString::EscapeHashString(const std::string & str) : mData(EscapeHashString::processString(str)) - { - } - - EscapeHashString::EscapeHashString(const std::string & str, size_t pos, size_t len) : mData(EscapeHashString::processString(str), pos, len) - { - } - - EscapeHashString::EscapeHashString(const char * s) : mData(EscapeHashString::processString(std::string(s))) - { - } - - EscapeHashString::EscapeHashString(const char * s, size_t n) : mData(EscapeHashString::processString(std::string(s)), 0, n) - { - } - - EscapeHashString::EscapeHashString(size_t n, char c) : mData(n, c) - { - } - - template - EscapeHashString::EscapeHashString(InputIterator first, InputIterator last) : mData(EscapeHashString::processString(std::string(first, last))) - { - } - - std::string EscapeHashString::toStdString() const - { - return std::string(mData); - } - - std::istream & operator>> (std::istream & is, EscapeHashString & eHS) - { - std::string temp; - is >> temp; - eHS = EscapeHashString(temp); - return is; - } - - std::ostream & operator<< (std::ostream & os, const EscapeHashString & eHS) - { - os << eHS.mData; - return os; - } - - EscapeStringVector::EscapeStringVector() : mVector() - { - } - - EscapeStringVector::~EscapeStringVector() - { - } - - std::vector EscapeStringVector::toStdStringVector() const - { - std::vector temp = std::vector(); - for (std::vector::const_iterator it = mVector.begin(); it != mVector.end(); ++it) - { - temp.push_back(it->toStdString()); - } - return temp; - } - - // boost program options validation - - void validate(boost::any &v, const std::vector &tokens, Files::EscapeHashString * eHS, int a) - { - boost::program_options::validators::check_first_occurrence(v); - - if (v.empty()) - v = boost::any(EscapeHashString(boost::program_options::validators::get_single_string(tokens))); - } - - void validate(boost::any &v, const std::vector &tokens, EscapeStringVector *, int) - { - if (v.empty()) - v = boost::any(EscapeStringVector()); - - EscapeStringVector * eSV = boost::any_cast(&v); - - for (std::vector::const_iterator it = tokens.begin(); it != tokens.end(); ++it) - eSV->mVector.emplace_back(*it); - } - - PathContainer EscapePath::toPathContainer(const EscapePathContainer & escapePathContainer) - { - PathContainer temp; - for (EscapePathContainer::const_iterator it = escapePathContainer.begin(); it != escapePathContainer.end(); ++it) - temp.push_back(it->mPath); - return temp; - } - - std::istream & operator>> (std::istream & istream, EscapePath & escapePath) - { - boost::iostreams::filtering_istream filteredStream; - filteredStream.push(unescape_hash_filter()); - filteredStream.push(istream); - - filteredStream >> escapePath.mPath; - - return istream; - } -} diff --git a/components/files/escape.hpp b/components/files/escape.hpp deleted file mode 100644 index d01bd8d980..0000000000 --- a/components/files/escape.hpp +++ /dev/null @@ -1,191 +0,0 @@ -#ifndef COMPONENTS_FILES_ESCAPE_HPP -#define COMPONENTS_FILES_ESCAPE_HPP - -#include - -#include - -#include -#include -#include - -/** - * \namespace Files - */ -namespace Files -{ - /** - * \struct escape_hash_filter - */ - struct escape_hash_filter : public boost::iostreams::input_filter - { - static const int sEscape; - static const int sHashIdentifier; - static const int sEscapeIdentifier; - - escape_hash_filter(); - virtual ~escape_hash_filter(); - - template int get(Source & src); - - private: - std::queue mNext; - - bool mSeenNonWhitespace; - bool mFinishLine; - }; - - template - int escape_hash_filter::get(Source & src) - { - if (mNext.empty()) - { - int character = boost::iostreams::get(src); - if (character == boost::iostreams::WOULD_BLOCK) - { - mNext.push(character); - } - else if (character == EOF) - { - mSeenNonWhitespace = false; - mFinishLine = false; - mNext.push(character); - } - else if (character == '\n') - { - mSeenNonWhitespace = false; - mFinishLine = false; - mNext.push(character); - } - else if (mFinishLine) - { - mNext.push(character); - } - else if (character == '#') - { - if (mSeenNonWhitespace) - { - mNext.push(sEscape); - mNext.push(sHashIdentifier); - } - else - { - //it's fine being interpreted by Boost as a comment, and so is anything afterwards - mNext.push(character); - mFinishLine = true; - } - } - else if (character == sEscape) - { - mNext.push(sEscape); - mNext.push(sEscapeIdentifier); - } - else - { - mNext.push(character); - } - if (!mSeenNonWhitespace && !isspace(character)) - mSeenNonWhitespace = true; - } - int retval = mNext.front(); - mNext.pop(); - return retval; - } - - struct unescape_hash_filter : public boost::iostreams::input_filter - { - unescape_hash_filter(); - virtual ~unescape_hash_filter(); - - template int get(Source & src); - - private: - bool expectingIdentifier; - }; - - template - int unescape_hash_filter::get(Source & src) - { - int character; - if (!expectingIdentifier) - character = boost::iostreams::get(src); - else - { - character = escape_hash_filter::sEscape; - expectingIdentifier = false; - } - if (character == escape_hash_filter::sEscape) - { - int nextChar = boost::iostreams::get(src); - int intended; - if (nextChar == escape_hash_filter::sEscapeIdentifier) - intended = escape_hash_filter::sEscape; - else if (nextChar == escape_hash_filter::sHashIdentifier) - intended = '#'; - else if (nextChar == boost::iostreams::WOULD_BLOCK) - { - expectingIdentifier = true; - intended = nextChar; - } - else - intended = '?'; - return intended; - } - else - return character; - } - - /** - * \class EscapeHashString - */ - class EscapeHashString - { - private: - std::string mData; - public: - static std::string processString(const std::string & str); - - EscapeHashString(); - EscapeHashString(const std::string & str); - EscapeHashString(const std::string & str, size_t pos, size_t len = std::string::npos); - EscapeHashString(const char * s); - EscapeHashString(const char * s, size_t n); - EscapeHashString(size_t n, char c); - template - EscapeHashString(InputIterator first, InputIterator last); - - std::string toStdString() const; - - friend std::ostream & operator<< (std::ostream & os, const EscapeHashString & eHS); - }; - - std::istream & operator>> (std::istream & is, EscapeHashString & eHS); - - struct EscapeStringVector - { - std::vector mVector; - - EscapeStringVector(); - virtual ~EscapeStringVector(); - - std::vector toStdStringVector() const; - }; - - //boost program options validation - - void validate(boost::any &v, const std::vector &tokens, Files::EscapeHashString * eHS, int a); - - void validate(boost::any &v, const std::vector &tokens, EscapeStringVector *, int); - - struct EscapePath - { - boost::filesystem::path mPath; - - static PathContainer toPathContainer(const std::vector & escapePathContainer); - }; - - typedef std::vector EscapePathContainer; - - std::istream & operator>> (std::istream & istream, EscapePath & escapePath); -} /* namespace Files */ -#endif /* COMPONENTS_FILES_ESCAPE_HPP */ From 8fb0b5846ea2e059f225e6659b393584305bcb53 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 14 Nov 2021 00:22:44 +0000 Subject: [PATCH 07/11] Allow paths with trailing data, emmitting a warning --- apps/opencs/editor.cpp | 12 +-- apps/openmw/main.cpp | 12 +-- apps/openmw/options.cpp | 8 +- apps/openmw_test_suite/mwworld/test_store.cpp | 8 +- apps/openmw_test_suite/openmw/options.cpp | 102 ++++++++++-------- components/files/configurationmanager.cpp | 24 ++++- components/files/configurationmanager.hpp | 12 +++ 7 files changed, 113 insertions(+), 65 deletions(-) diff --git a/apps/opencs/editor.cpp b/apps/opencs/editor.cpp index b437a36201..6294e5eb7c 100644 --- a/apps/opencs/editor.cpp +++ b/apps/opencs/editor.cpp @@ -88,11 +88,11 @@ std::pair > CS::Editor::readConfi boost::program_options::options_description desc("Syntax: openmw-cs \nAllowed options"); desc.add_options() - ("data", boost::program_options::value()->default_value(Files::PathContainer(), "data")->multitoken()->composing()) - ("data-local", boost::program_options::value()->default_value(Files::PathContainer::value_type(), "")) + ("data", boost::program_options::value()->default_value(Files::ReluctantPathContainer(), "data")->multitoken()->composing()) + ("data-local", boost::program_options::value()->default_value(Files::ReluctantPathContainer::value_type(), "")) ("fs-strict", boost::program_options::value()->implicit_value(true)->default_value(false)) ("encoding", boost::program_options::value()->default_value("win1252")) - ("resources", boost::program_options::value()->default_value(boost::filesystem::path(), "resources")) + ("resources", boost::program_options::value()->default_value(Files::ReluctantPath(), "resources")) ("fallback-archive", boost::program_options::value>()-> default_value(std::vector(), "fallback-archive")->multitoken()) ("fallback", boost::program_options::value()->default_value(FallbackMap(), "") @@ -112,7 +112,7 @@ std::pair > CS::Editor::readConfi mDocumentManager.setEncoding(ToUTF8::calculateEncoding(mEncodingName)); mFileDialog.setEncoding (QString::fromUtf8(mEncodingName.c_str())); - mDocumentManager.setResourceDir (mResources = variables["resources"].as()); + mDocumentManager.setResourceDir (mResources = variables["resources"].as()); if (variables["script-blacklist-use"].as()) mDocumentManager.setBlacklistedScripts ( @@ -122,10 +122,10 @@ std::pair > CS::Editor::readConfi Files::PathContainer dataDirs, dataLocal; if (!variables["data"].empty()) { - dataDirs = variables["data"].as(); + dataDirs = asPathContainer(variables["data"].as()); } - Files::PathContainer::value_type local(variables["data-local"].as()); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) dataLocal.push_back(local); diff --git a/apps/openmw/main.cpp b/apps/openmw/main.cpp index 7a3a21b149..50a7c3d844 100644 --- a/apps/openmw/main.cpp +++ b/apps/openmw/main.cpp @@ -56,7 +56,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat { cfgMgr.readConfiguration(variables, desc, true); - Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); + Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); getRawStdout() << v.describe() << std::endl; return false; } @@ -65,7 +65,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat cfgMgr.readConfiguration(variables, desc); Files::mergeComposingVariables(variables, composingVariables, desc); - Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); + Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); Log(Debug::Info) << v.describe(); engine.setGrabMouse(!variables["no-grab"].as()); @@ -78,15 +78,15 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat // directory settings engine.enableFSStrict(variables["fs-strict"].as()); - Files::PathContainer dataDirs(variables["data"].as()); + Files::PathContainer dataDirs(asPathContainer(variables["data"].as())); - Files::PathContainer::value_type local(variables["data-local"].as()); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) dataDirs.push_back(local); cfgMgr.processPaths(dataDirs); - engine.setResourceDir(variables["resources"].as()); + engine.setResourceDir(variables["resources"].as()); engine.setDataDirs(dataDirs); // fallback archives @@ -141,7 +141,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat engine.setWarningsMode (variables["script-warn"].as()); engine.setScriptBlacklist (variables["script-blacklist"].as()); engine.setScriptBlacklistUse (variables["script-blacklist-use"].as()); - engine.setSaveGameFile (variables["load-savegame"].as().string()); + engine.setSaveGameFile (variables["load-savegame"].as().string()); // other settings Fallback::Map::init(variables["fallback"].as().mMap); diff --git a/apps/openmw/options.cpp b/apps/openmw/options.cpp index dd94590f2a..a1b2102f08 100644 --- a/apps/openmw/options.cpp +++ b/apps/openmw/options.cpp @@ -25,16 +25,16 @@ namespace OpenMW ("replace", bpo::value()->default_value(StringsVector(), "") ->multitoken()->composing(), "settings where the values from the current source should replace those from lower-priority sources instead of being appended") - ("data", bpo::value()->default_value(Files::PathContainer(), "data") + ("data", bpo::value()->default_value(Files::ReluctantPathContainer(), "data") ->multitoken()->composing(), "set data directories (later directories have higher priority)") - ("data-local", bpo::value()->default_value(Files::PathContainer::value_type(), ""), + ("data-local", bpo::value()->default_value(Files::ReluctantPathContainer::value_type(), ""), "set local data directory (highest priority)") ("fallback-archive", bpo::value()->default_value(StringsVector(), "fallback-archive") ->multitoken()->composing(), "set fallback BSA archives (later archives have higher priority)") - ("resources", bpo::value()->default_value(boost::filesystem::path(), "resources"), + ("resources", bpo::value()->default_value(Files::ReluctantPath(), "resources"), "set resources directory") ("start", bpo::value()->default_value(""), @@ -77,7 +77,7 @@ namespace OpenMW ("script-blacklist-use", bpo::value()->implicit_value(true) ->default_value(true), "enable script blacklisting") - ("load-savegame", bpo::value()->default_value(boost::filesystem::path(), ""), + ("load-savegame", bpo::value()->default_value(Files::ReluctantPath(), ""), "load a save game file on game startup (specify an absolute filename or a filename relative to the current working directory)") ("skip-menu", bpo::value()->implicit_value(true) diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index 9aa5a6758f..f2f9459df0 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/apps/openmw_test_suite/mwworld/test_store.cpp @@ -58,10 +58,10 @@ struct ContentFileTest : public ::testing::Test boost::program_options::options_description desc("Allowed options"); desc.add_options() - ("data", boost::program_options::value()->default_value(Files::PathContainer(), "data")->multitoken()->composing()) + ("data", boost::program_options::value()->default_value(Files::ReluctantPathContainer(), "data")->multitoken()->composing()) ("content", boost::program_options::value>()->default_value(std::vector(), "") ->multitoken()->composing(), "content file(s): esm/esp, or omwgame/omwaddon") - ("data-local", boost::program_options::value()->default_value(Files::PathContainer::value_type(), "")); + ("data-local", boost::program_options::value()->default_value(Files::ReluctantPathContainer::value_type(), "")); boost::program_options::notify(variables); @@ -69,10 +69,10 @@ struct ContentFileTest : public ::testing::Test Files::PathContainer dataDirs, dataLocal; if (!variables["data"].empty()) { - dataDirs = variables["data"].as(); + dataDirs = asPathContainer(variables["data"].as()); } - Files::PathContainer::value_type local(variables["data-local"].as()); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) { dataLocal.push_back(local); } diff --git a/apps/openmw_test_suite/openmw/options.cpp b/apps/openmw_test_suite/openmw/options.cpp index 7cf0df1066..bae324b134 100644 --- a/apps/openmw_test_suite/openmw/options.cpp +++ b/apps/openmw_test_suite/openmw/options.cpp @@ -40,7 +40,7 @@ namespace const std::array arguments {"openmw", "--load-savegame=save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_single_word_load_savegame_path) @@ -49,7 +49,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_multi_component_load_savegame_path) @@ -58,7 +58,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "/home/user/openmw/save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_windows_multi_component_load_savegame_path) @@ -67,18 +67,18 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"(C:\OpenMW\save.omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); } - /*TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_spaces) + TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_spaces) { bpo::options_description description = makeOptionsDescription(); const std::array arguments {"openmw", "--load-savegame", "my save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my"); -// EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); - }*/ + EXPECT_EQ(variables["load-savegame"].as().string(), "my"); +// EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); + } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_octothorpe) { @@ -86,7 +86,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "my#save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my#save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "my#save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_at_sign) @@ -95,7 +95,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "my@save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my@save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "my@save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_quote) @@ -104,18 +104,18 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"(my"save.omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save.omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save.omwsave)"); } -/* TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path) + TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path) { bpo::options_description description = makeOptionsDescription(); const std::array arguments {"openmw", "--load-savegame", R"("save".omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save)"); -// EXPECT_EQ(variables["load-savegame"].as().string(), R"("save".omwsave)"); - }*/ + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save)"); +// EXPECT_EQ(variables["load-savegame"].as().string(), R"("save".omwsave)"); + } TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path_with_escaped_quote_by_ampersand) { @@ -123,7 +123,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"("save&".omwsave")"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); } TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path_with_escaped_ampersand) @@ -132,7 +132,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"("save.omwsave&&")"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_ampersand) @@ -141,7 +141,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "save&.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_multiple_quotes) @@ -150,7 +150,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"(my"save".omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save".omwsave)"); } TEST(OpenMWOptionsFromArguments, should_compose_data) @@ -159,7 +159,7 @@ namespace const std::array arguments {"openmw", "--data", "1", "--data", "2"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); + EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); } TEST(OpenMWOptionsFromArguments, should_compose_data_from_single_flag) @@ -168,7 +168,7 @@ namespace const std::array arguments {"openmw", "--data", "1", "2"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); + EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); } TEST(OpenMWOptionsFromArguments, should_throw_on_multiple_load_savegame) @@ -189,7 +189,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", pathArgument.c_str()}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), path); + EXPECT_EQ(variables["load-savegame"].as().string(), path); } INSTANTIATE_TEST_SUITE_P( @@ -204,7 +204,7 @@ namespace std::istringstream stream("load-savegame=save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_strip_quotes_from_load_savegame_path) @@ -213,18 +213,18 @@ namespace std::istringstream stream(R"(load-savegame="save.omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } -/* TEST(OpenMWOptionsFromConfig, should_strip_outer_quotes_from_load_savegame_path) + TEST(OpenMWOptionsFromConfig, should_strip_outer_quotes_from_load_savegame_path) { bpo::options_description description = makeOptionsDescription(); std::istringstream stream(R"(load-savegame=""save".omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), ""); -// EXPECT_EQ(variables["load-savegame"].as().string(), R"(""save".omwsave")"); - }*/ + EXPECT_EQ(variables["load-savegame"].as().string(), ""); +// EXPECT_EQ(variables["load-savegame"].as().string(), R"(""save".omwsave")"); + } TEST(OpenMWOptionsFromConfig, should_strip_quotes_from_load_savegame_path_with_space) { @@ -232,7 +232,7 @@ namespace std::istringstream stream(R"(load-savegame="my save.omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_octothorpe) @@ -241,7 +241,7 @@ namespace std::istringstream stream("load-savegame=save#.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save#.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save#.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_at_sign) @@ -250,7 +250,7 @@ namespace std::istringstream stream("load-savegame=save@.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save@.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save@.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_quote) @@ -259,7 +259,25 @@ namespace std::istringstream stream(R"(load-savegame=save".omwsave)"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); + } + + TEST(OpenMWOptionsFromConfig, should_support_confusing_savegame_path_with_lots_going_on) + { + bpo::options_description description = makeOptionsDescription(); + std::istringstream stream(R"(load-savegame="one &"two"three".omwsave")"); + bpo::variables_map variables; + Files::parseConfig(stream, variables, description); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(one "two)"); + } + + TEST(OpenMWOptionsFromConfig, should_support_confusing_savegame_path_with_even_more_going_on) + { + bpo::options_description description = makeOptionsDescription(); + std::istringstream stream(R"(load-savegame="one &"two"three ".omwsave")"); + bpo::variables_map variables; + Files::parseConfig(stream, variables, description); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(one "two)"); } TEST(OpenMWOptionsFromConfig, should_ignore_commented_option) @@ -268,7 +286,7 @@ namespace std::istringstream stream("#load-savegame=save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), ""); + EXPECT_EQ(variables["load-savegame"].as().string(), ""); } TEST(OpenMWOptionsFromConfig, should_ignore_whitespace_prefixed_commented_option) @@ -277,7 +295,7 @@ namespace std::istringstream stream(" \t#load-savegame=save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), ""); + EXPECT_EQ(variables["load-savegame"].as().string(), ""); } TEST(OpenMWOptionsFromConfig, should_support_whitespace_around_option) @@ -286,7 +304,7 @@ namespace std::istringstream stream(" load-savegame = save.omwsave "); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_throw_on_multiple_load_savegame) @@ -303,7 +321,7 @@ namespace std::istringstream stream("load-savegame=/home/user/openmw/save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_windows_multi_component_load_savegame_path) @@ -312,7 +330,7 @@ namespace std::istringstream stream(R"(load-savegame=C:\OpenMW\save.omwsave)"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); } TEST(OpenMWOptionsFromConfig, should_compose_data) @@ -321,7 +339,7 @@ namespace std::istringstream stream("data=1\ndata=2"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); + EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_escaped_quote_by_ampersand) @@ -330,7 +348,7 @@ namespace std::istringstream stream(R"(load-savegame="save&".omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_escaped_ampersand) @@ -339,7 +357,7 @@ namespace std::istringstream stream(R"(load-savegame="save.omwsave&&")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); } TEST(OpenMWOptionsFromConfig, should_support_load_savegame_path_with_ampersand) @@ -348,7 +366,7 @@ namespace std::istringstream stream("load-savegame=save&.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); } struct OpenMWOptionsFromConfigStrings : TestWithParam {}; @@ -360,7 +378,7 @@ namespace std::istringstream stream("load-savegame=\"" + path + "\""); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), path); + EXPECT_EQ(variables["load-savegame"].as().string(), path); } INSTANTIATE_TEST_SUITE_P( diff --git a/components/files/configurationmanager.cpp b/components/files/configurationmanager.cpp index 23c25fd413..b04b211967 100644 --- a/components/files/configurationmanager.cpp +++ b/components/files/configurationmanager.cpp @@ -138,10 +138,10 @@ void mergeComposingVariables(boost::program_options::variables_map& first, boost boost::any& firstValue = firstPosition->second.value(); const boost::any& secondValue = second[name].value(); - if (firstValue.type() == typeid(Files::PathContainer)) + if (firstValue.type() == typeid(Files::ReluctantPathContainer)) { - auto& firstPathContainer = boost::any_cast(firstValue); - const auto& secondPathContainer = boost::any_cast(secondValue); + auto& firstPathContainer = boost::any_cast(firstValue); + const auto& secondPathContainer = boost::any_cast(secondValue); firstPathContainer.insert(firstPathContainer.end(), secondPathContainer.begin(), secondPathContainer.end()); } @@ -317,4 +317,22 @@ void parseConfig(std::istream& stream, boost::program_options::variables_map& va ); } +std::istream& operator>> (std::istream& istream, ReluctantPath& reluctantPath) +{ + // Read from stream using boost::filesystem::path rules, then discard anything remaining. + // This prevents boost::program_options getting upset that we've not consumed the whole stream. + istream >> static_cast(reluctantPath); + if (istream && !istream.eof() && istream.peek() != EOF) + { + std::string remainder(std::istreambuf_iterator(istream), {}); + Log(Debug::Warning) << "Trailing data in path setting. Used '" << reluctantPath.string() << "' but '" << remainder << "' remained"; + } + return istream; +} + +PathContainer asPathContainer(const ReluctantPathContainer& reluctantPathContainer) +{ + return PathContainer(reluctantPathContainer.begin(), reluctantPathContainer.end()); +} + } /* namespace Cfg */ diff --git a/components/files/configurationmanager.hpp b/components/files/configurationmanager.hpp index 99e65a7658..8f3e5f938e 100644 --- a/components/files/configurationmanager.hpp +++ b/components/files/configurationmanager.hpp @@ -77,6 +77,18 @@ void parseArgs(int argc, const char* const argv[], boost::program_options::varia void parseConfig(std::istream& stream, boost::program_options::variables_map& variables, boost::program_options::options_description& description); +class ReluctantPath : public boost::filesystem::path +{ +public: + operator boost::filesystem::path() { return *this; } +}; + +std::istream& operator>> (std::istream& istream, ReluctantPath& reluctantPath); + +typedef std::vector ReluctantPathContainer; + +PathContainer asPathContainer(const ReluctantPathContainer& reluctantPathContainer); + } /* namespace Cfg */ #endif /* COMPONENTS_FILES_CONFIGURATIONMANAGER_HPP */ From 84d6de3eba16c79ae814d8359c2d0b299f6649ba Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 21 Nov 2021 19:51:02 +0000 Subject: [PATCH 08/11] Parse paths with boost rules when it's quoted, but use the string verbatim when it's not --- apps/opencs/editor.cpp | 12 +-- apps/openmw/main.cpp | 12 +-- apps/openmw/options.cpp | 8 +- apps/openmw_test_suite/mwworld/test_store.cpp | 8 +- apps/openmw_test_suite/openmw/options.cpp | 75 +++++++++---------- components/files/configurationmanager.cpp | 31 +++++--- components/files/configurationmanager.hpp | 8 +- 7 files changed, 81 insertions(+), 73 deletions(-) diff --git a/apps/opencs/editor.cpp b/apps/opencs/editor.cpp index 6294e5eb7c..ac0a7a5ef7 100644 --- a/apps/opencs/editor.cpp +++ b/apps/opencs/editor.cpp @@ -88,11 +88,11 @@ std::pair > CS::Editor::readConfi boost::program_options::options_description desc("Syntax: openmw-cs \nAllowed options"); desc.add_options() - ("data", boost::program_options::value()->default_value(Files::ReluctantPathContainer(), "data")->multitoken()->composing()) - ("data-local", boost::program_options::value()->default_value(Files::ReluctantPathContainer::value_type(), "")) + ("data", boost::program_options::value()->default_value(Files::MaybeQuotedPathContainer(), "data")->multitoken()->composing()) + ("data-local", boost::program_options::value()->default_value(Files::MaybeQuotedPathContainer::value_type(), "")) ("fs-strict", boost::program_options::value()->implicit_value(true)->default_value(false)) ("encoding", boost::program_options::value()->default_value("win1252")) - ("resources", boost::program_options::value()->default_value(Files::ReluctantPath(), "resources")) + ("resources", boost::program_options::value()->default_value(Files::MaybeQuotedPath(), "resources")) ("fallback-archive", boost::program_options::value>()-> default_value(std::vector(), "fallback-archive")->multitoken()) ("fallback", boost::program_options::value()->default_value(FallbackMap(), "") @@ -112,7 +112,7 @@ std::pair > CS::Editor::readConfi mDocumentManager.setEncoding(ToUTF8::calculateEncoding(mEncodingName)); mFileDialog.setEncoding (QString::fromUtf8(mEncodingName.c_str())); - mDocumentManager.setResourceDir (mResources = variables["resources"].as()); + mDocumentManager.setResourceDir (mResources = variables["resources"].as()); if (variables["script-blacklist-use"].as()) mDocumentManager.setBlacklistedScripts ( @@ -122,10 +122,10 @@ std::pair > CS::Editor::readConfi Files::PathContainer dataDirs, dataLocal; if (!variables["data"].empty()) { - dataDirs = asPathContainer(variables["data"].as()); + dataDirs = asPathContainer(variables["data"].as()); } - Files::PathContainer::value_type local(variables["data-local"].as()); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) dataLocal.push_back(local); diff --git a/apps/openmw/main.cpp b/apps/openmw/main.cpp index 50a7c3d844..d502ecbc05 100644 --- a/apps/openmw/main.cpp +++ b/apps/openmw/main.cpp @@ -56,7 +56,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat { cfgMgr.readConfiguration(variables, desc, true); - Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); + Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); getRawStdout() << v.describe() << std::endl; return false; } @@ -65,7 +65,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat cfgMgr.readConfiguration(variables, desc); Files::mergeComposingVariables(variables, composingVariables, desc); - Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); + Version::Version v = Version::getOpenmwVersion(variables["resources"].as().string()); Log(Debug::Info) << v.describe(); engine.setGrabMouse(!variables["no-grab"].as()); @@ -78,15 +78,15 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat // directory settings engine.enableFSStrict(variables["fs-strict"].as()); - Files::PathContainer dataDirs(asPathContainer(variables["data"].as())); + Files::PathContainer dataDirs(asPathContainer(variables["data"].as())); - Files::PathContainer::value_type local(variables["data-local"].as()); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) dataDirs.push_back(local); cfgMgr.processPaths(dataDirs); - engine.setResourceDir(variables["resources"].as()); + engine.setResourceDir(variables["resources"].as()); engine.setDataDirs(dataDirs); // fallback archives @@ -141,7 +141,7 @@ bool parseOptions (int argc, char** argv, OMW::Engine& engine, Files::Configurat engine.setWarningsMode (variables["script-warn"].as()); engine.setScriptBlacklist (variables["script-blacklist"].as()); engine.setScriptBlacklistUse (variables["script-blacklist-use"].as()); - engine.setSaveGameFile (variables["load-savegame"].as().string()); + engine.setSaveGameFile (variables["load-savegame"].as().string()); // other settings Fallback::Map::init(variables["fallback"].as().mMap); diff --git a/apps/openmw/options.cpp b/apps/openmw/options.cpp index a1b2102f08..f7d20b4dd7 100644 --- a/apps/openmw/options.cpp +++ b/apps/openmw/options.cpp @@ -25,16 +25,16 @@ namespace OpenMW ("replace", bpo::value()->default_value(StringsVector(), "") ->multitoken()->composing(), "settings where the values from the current source should replace those from lower-priority sources instead of being appended") - ("data", bpo::value()->default_value(Files::ReluctantPathContainer(), "data") + ("data", bpo::value()->default_value(Files::MaybeQuotedPathContainer(), "data") ->multitoken()->composing(), "set data directories (later directories have higher priority)") - ("data-local", bpo::value()->default_value(Files::ReluctantPathContainer::value_type(), ""), + ("data-local", bpo::value()->default_value(Files::MaybeQuotedPathContainer::value_type(), ""), "set local data directory (highest priority)") ("fallback-archive", bpo::value()->default_value(StringsVector(), "fallback-archive") ->multitoken()->composing(), "set fallback BSA archives (later archives have higher priority)") - ("resources", bpo::value()->default_value(Files::ReluctantPath(), "resources"), + ("resources", bpo::value()->default_value(Files::MaybeQuotedPath(), "resources"), "set resources directory") ("start", bpo::value()->default_value(""), @@ -77,7 +77,7 @@ namespace OpenMW ("script-blacklist-use", bpo::value()->implicit_value(true) ->default_value(true), "enable script blacklisting") - ("load-savegame", bpo::value()->default_value(Files::ReluctantPath(), ""), + ("load-savegame", bpo::value()->default_value(Files::MaybeQuotedPath(), ""), "load a save game file on game startup (specify an absolute filename or a filename relative to the current working directory)") ("skip-menu", bpo::value()->implicit_value(true) diff --git a/apps/openmw_test_suite/mwworld/test_store.cpp b/apps/openmw_test_suite/mwworld/test_store.cpp index f2f9459df0..71b1fb0ff0 100644 --- a/apps/openmw_test_suite/mwworld/test_store.cpp +++ b/apps/openmw_test_suite/mwworld/test_store.cpp @@ -58,10 +58,10 @@ struct ContentFileTest : public ::testing::Test boost::program_options::options_description desc("Allowed options"); desc.add_options() - ("data", boost::program_options::value()->default_value(Files::ReluctantPathContainer(), "data")->multitoken()->composing()) + ("data", boost::program_options::value()->default_value(Files::MaybeQuotedPathContainer(), "data")->multitoken()->composing()) ("content", boost::program_options::value>()->default_value(std::vector(), "") ->multitoken()->composing(), "content file(s): esm/esp, or omwgame/omwaddon") - ("data-local", boost::program_options::value()->default_value(Files::ReluctantPathContainer::value_type(), "")); + ("data-local", boost::program_options::value()->default_value(Files::MaybeQuotedPathContainer::value_type(), "")); boost::program_options::notify(variables); @@ -69,10 +69,10 @@ struct ContentFileTest : public ::testing::Test Files::PathContainer dataDirs, dataLocal; if (!variables["data"].empty()) { - dataDirs = asPathContainer(variables["data"].as()); + dataDirs = asPathContainer(variables["data"].as()); } - Files::PathContainer::value_type local(variables["data-local"].as()); + Files::PathContainer::value_type local(variables["data-local"].as()); if (!local.empty()) { dataLocal.push_back(local); } diff --git a/apps/openmw_test_suite/openmw/options.cpp b/apps/openmw_test_suite/openmw/options.cpp index bae324b134..9b8535ea36 100644 --- a/apps/openmw_test_suite/openmw/options.cpp +++ b/apps/openmw_test_suite/openmw/options.cpp @@ -40,7 +40,7 @@ namespace const std::array arguments {"openmw", "--load-savegame=save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_single_word_load_savegame_path) @@ -49,7 +49,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_multi_component_load_savegame_path) @@ -58,7 +58,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "/home/user/openmw/save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_windows_multi_component_load_savegame_path) @@ -67,7 +67,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"(C:\OpenMW\save.omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_spaces) @@ -76,8 +76,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "my save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my"); -// EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_octothorpe) @@ -86,7 +85,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "my#save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my#save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "my#save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_at_sign) @@ -95,7 +94,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "my@save.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my@save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "my@save.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_quote) @@ -104,7 +103,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"(my"save.omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save.omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save.omwsave)"); } TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path) @@ -113,8 +112,8 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"("save".omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save)"); -// EXPECT_EQ(variables["load-savegame"].as().string(), R"("save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save)"); +// EXPECT_EQ(variables["load-savegame"].as().string(), R"("save".omwsave)"); } TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path_with_escaped_quote_by_ampersand) @@ -123,7 +122,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"("save&".omwsave")"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); } TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path_with_escaped_ampersand) @@ -132,7 +131,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"("save.omwsave&&")"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_ampersand) @@ -141,7 +140,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", "save&.omwsave"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); } TEST(OpenMWOptionsFromArguments, should_support_load_savegame_path_with_multiple_quotes) @@ -150,7 +149,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", R"(my"save".omwsave)"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(my"save".omwsave)"); } TEST(OpenMWOptionsFromArguments, should_compose_data) @@ -159,7 +158,7 @@ namespace const std::array arguments {"openmw", "--data", "1", "--data", "2"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); + EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); } TEST(OpenMWOptionsFromArguments, should_compose_data_from_single_flag) @@ -168,7 +167,7 @@ namespace const std::array arguments {"openmw", "--data", "1", "2"}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); + EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); } TEST(OpenMWOptionsFromArguments, should_throw_on_multiple_load_savegame) @@ -189,7 +188,7 @@ namespace const std::array arguments {"openmw", "--load-savegame", pathArgument.c_str()}; bpo::variables_map variables; parseArgs(arguments, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), path); + EXPECT_EQ(variables["load-savegame"].as().string(), path); } INSTANTIATE_TEST_SUITE_P( @@ -204,7 +203,7 @@ namespace std::istringstream stream("load-savegame=save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_strip_quotes_from_load_savegame_path) @@ -213,7 +212,7 @@ namespace std::istringstream stream(R"(load-savegame="save.omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_strip_outer_quotes_from_load_savegame_path) @@ -222,8 +221,8 @@ namespace std::istringstream stream(R"(load-savegame=""save".omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), ""); -// EXPECT_EQ(variables["load-savegame"].as().string(), R"(""save".omwsave")"); + EXPECT_EQ(variables["load-savegame"].as().string(), ""); +// EXPECT_EQ(variables["load-savegame"].as().string(), R"(""save".omwsave")"); } TEST(OpenMWOptionsFromConfig, should_strip_quotes_from_load_savegame_path_with_space) @@ -232,7 +231,7 @@ namespace std::istringstream stream(R"(load-savegame="my save.omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "my save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_octothorpe) @@ -241,7 +240,7 @@ namespace std::istringstream stream("load-savegame=save#.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save#.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save#.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_at_sign) @@ -250,7 +249,7 @@ namespace std::istringstream stream("load-savegame=save@.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save@.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save@.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_quote) @@ -259,7 +258,7 @@ namespace std::istringstream stream(R"(load-savegame=save".omwsave)"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); } TEST(OpenMWOptionsFromConfig, should_support_confusing_savegame_path_with_lots_going_on) @@ -268,7 +267,7 @@ namespace std::istringstream stream(R"(load-savegame="one &"two"three".omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(one "two)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(one "two)"); } TEST(OpenMWOptionsFromConfig, should_support_confusing_savegame_path_with_even_more_going_on) @@ -277,7 +276,7 @@ namespace std::istringstream stream(R"(load-savegame="one &"two"three ".omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(one "two)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(one "two)"); } TEST(OpenMWOptionsFromConfig, should_ignore_commented_option) @@ -286,7 +285,7 @@ namespace std::istringstream stream("#load-savegame=save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), ""); + EXPECT_EQ(variables["load-savegame"].as().string(), ""); } TEST(OpenMWOptionsFromConfig, should_ignore_whitespace_prefixed_commented_option) @@ -295,7 +294,7 @@ namespace std::istringstream stream(" \t#load-savegame=save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), ""); + EXPECT_EQ(variables["load-savegame"].as().string(), ""); } TEST(OpenMWOptionsFromConfig, should_support_whitespace_around_option) @@ -304,7 +303,7 @@ namespace std::istringstream stream(" load-savegame = save.omwsave "); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_throw_on_multiple_load_savegame) @@ -321,7 +320,7 @@ namespace std::istringstream stream("load-savegame=/home/user/openmw/save.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "/home/user/openmw/save.omwsave"); } TEST(OpenMWOptionsFromConfig, should_support_windows_multi_component_load_savegame_path) @@ -330,7 +329,7 @@ namespace std::istringstream stream(R"(load-savegame=C:\OpenMW\save.omwsave)"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(C:\OpenMW\save.omwsave)"); } TEST(OpenMWOptionsFromConfig, should_compose_data) @@ -339,7 +338,7 @@ namespace std::istringstream stream("data=1\ndata=2"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); + EXPECT_THAT(variables["data"].as(), ElementsAre(IsPath("1"), IsPath("2"))); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_escaped_quote_by_ampersand) @@ -348,7 +347,7 @@ namespace std::istringstream stream(R"(load-savegame="save&".omwsave")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); + EXPECT_EQ(variables["load-savegame"].as().string(), R"(save".omwsave)"); } TEST(OpenMWOptionsFromConfig, should_support_quoted_load_savegame_path_with_escaped_ampersand) @@ -357,7 +356,7 @@ namespace std::istringstream stream(R"(load-savegame="save.omwsave&&")"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save.omwsave&"); } TEST(OpenMWOptionsFromConfig, should_support_load_savegame_path_with_ampersand) @@ -366,7 +365,7 @@ namespace std::istringstream stream("load-savegame=save&.omwsave"); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); + EXPECT_EQ(variables["load-savegame"].as().string(), "save&.omwsave"); } struct OpenMWOptionsFromConfigStrings : TestWithParam {}; @@ -378,7 +377,7 @@ namespace std::istringstream stream("load-savegame=\"" + path + "\""); bpo::variables_map variables; Files::parseConfig(stream, variables, description); - EXPECT_EQ(variables["load-savegame"].as().string(), path); + EXPECT_EQ(variables["load-savegame"].as().string(), path); } INSTANTIATE_TEST_SUITE_P( diff --git a/components/files/configurationmanager.cpp b/components/files/configurationmanager.cpp index b04b211967..bb22e4750f 100644 --- a/components/files/configurationmanager.cpp +++ b/components/files/configurationmanager.cpp @@ -138,10 +138,10 @@ void mergeComposingVariables(boost::program_options::variables_map& first, boost boost::any& firstValue = firstPosition->second.value(); const boost::any& secondValue = second[name].value(); - if (firstValue.type() == typeid(Files::ReluctantPathContainer)) + if (firstValue.type() == typeid(Files::MaybeQuotedPathContainer)) { - auto& firstPathContainer = boost::any_cast(firstValue); - const auto& secondPathContainer = boost::any_cast(secondValue); + auto& firstPathContainer = boost::any_cast(firstValue); + const auto& secondPathContainer = boost::any_cast(secondValue); firstPathContainer.insert(firstPathContainer.end(), secondPathContainer.begin(), secondPathContainer.end()); } @@ -317,22 +317,31 @@ void parseConfig(std::istream& stream, boost::program_options::variables_map& va ); } -std::istream& operator>> (std::istream& istream, ReluctantPath& reluctantPath) +std::istream& operator>> (std::istream& istream, MaybeQuotedPath& MaybeQuotedPath) { - // Read from stream using boost::filesystem::path rules, then discard anything remaining. + // If the stream starts with a double quote, read from stream using boost::filesystem::path rules, then discard anything remaining. // This prevents boost::program_options getting upset that we've not consumed the whole stream. - istream >> static_cast(reluctantPath); - if (istream && !istream.eof() && istream.peek() != EOF) + // If it doesn't start with a double quote, read the whole thing verbatim + if (istream.peek() == '"') { - std::string remainder(std::istreambuf_iterator(istream), {}); - Log(Debug::Warning) << "Trailing data in path setting. Used '" << reluctantPath.string() << "' but '" << remainder << "' remained"; + istream >> static_cast(MaybeQuotedPath); + if (istream && !istream.eof() && istream.peek() != EOF) + { + std::string remainder(std::istreambuf_iterator(istream), {}); + Log(Debug::Warning) << "Trailing data in path setting. Used '" << MaybeQuotedPath.string() << "' but '" << remainder << "' remained"; + } + } + else + { + std::string intermediate(std::istreambuf_iterator(istream), {}); + static_cast(MaybeQuotedPath) = intermediate; } return istream; } -PathContainer asPathContainer(const ReluctantPathContainer& reluctantPathContainer) +PathContainer asPathContainer(const MaybeQuotedPathContainer& MaybeQuotedPathContainer) { - return PathContainer(reluctantPathContainer.begin(), reluctantPathContainer.end()); + return PathContainer(MaybeQuotedPathContainer.begin(), MaybeQuotedPathContainer.end()); } } /* namespace Cfg */ diff --git a/components/files/configurationmanager.hpp b/components/files/configurationmanager.hpp index 8f3e5f938e..0c4d2dc835 100644 --- a/components/files/configurationmanager.hpp +++ b/components/files/configurationmanager.hpp @@ -77,17 +77,17 @@ void parseArgs(int argc, const char* const argv[], boost::program_options::varia void parseConfig(std::istream& stream, boost::program_options::variables_map& variables, boost::program_options::options_description& description); -class ReluctantPath : public boost::filesystem::path +class MaybeQuotedPath : public boost::filesystem::path { public: operator boost::filesystem::path() { return *this; } }; -std::istream& operator>> (std::istream& istream, ReluctantPath& reluctantPath); +std::istream& operator>> (std::istream& istream, MaybeQuotedPath& MaybeQuotedPath); -typedef std::vector ReluctantPathContainer; +typedef std::vector MaybeQuotedPathContainer; -PathContainer asPathContainer(const ReluctantPathContainer& reluctantPathContainer); +PathContainer asPathContainer(const MaybeQuotedPathContainer& MaybeQuotedPathContainer); } /* namespace Cfg */ From bc4b54157b4b5e558e8fc25072b708a1b48ba0ce Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 28 Nov 2021 20:30:16 +0000 Subject: [PATCH 09/11] Remove commented-out test conditions --- apps/openmw_test_suite/openmw/options.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/apps/openmw_test_suite/openmw/options.cpp b/apps/openmw_test_suite/openmw/options.cpp index 9b8535ea36..04c209e8a4 100644 --- a/apps/openmw_test_suite/openmw/options.cpp +++ b/apps/openmw_test_suite/openmw/options.cpp @@ -113,7 +113,6 @@ namespace bpo::variables_map variables; parseArgs(arguments, variables, description); EXPECT_EQ(variables["load-savegame"].as().string(), R"(save)"); -// EXPECT_EQ(variables["load-savegame"].as().string(), R"("save".omwsave)"); } TEST(OpenMWOptionsFromArguments, should_support_quoted_load_savegame_path_with_escaped_quote_by_ampersand) @@ -222,7 +221,6 @@ namespace bpo::variables_map variables; Files::parseConfig(stream, variables, description); EXPECT_EQ(variables["load-savegame"].as().string(), ""); -// EXPECT_EQ(variables["load-savegame"].as().string(), R"(""save".omwsave")"); } TEST(OpenMWOptionsFromConfig, should_strip_quotes_from_load_savegame_path_with_space) From 5e9d460032d523a90628c61717ef6fcb187e80fc Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Sun, 28 Nov 2021 20:33:17 +0000 Subject: [PATCH 10/11] Remove redundant conversion operator --- components/files/configurationmanager.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/files/configurationmanager.hpp b/components/files/configurationmanager.hpp index 0c4d2dc835..4b641c12fd 100644 --- a/components/files/configurationmanager.hpp +++ b/components/files/configurationmanager.hpp @@ -79,8 +79,6 @@ void parseConfig(std::istream& stream, boost::program_options::variables_map& va class MaybeQuotedPath : public boost::filesystem::path { -public: - operator boost::filesystem::path() { return *this; } }; std::istream& operator>> (std::istream& istream, MaybeQuotedPath& MaybeQuotedPath); From b991263a92beb2c4bb78d7ce98ca698a49b4d7f6 Mon Sep 17 00:00:00 2001 From: AnyOldName3 Date: Mon, 29 Nov 2021 20:16:49 +0000 Subject: [PATCH 11/11] Work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89062 --- components/files/configurationmanager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/files/configurationmanager.cpp b/components/files/configurationmanager.cpp index bb22e4750f..c2cd44960f 100644 --- a/components/files/configurationmanager.cpp +++ b/components/files/configurationmanager.cpp @@ -327,13 +327,13 @@ std::istream& operator>> (std::istream& istream, MaybeQuotedPath& MaybeQuotedPat istream >> static_cast(MaybeQuotedPath); if (istream && !istream.eof() && istream.peek() != EOF) { - std::string remainder(std::istreambuf_iterator(istream), {}); + std::string remainder{std::istreambuf_iterator(istream), {}}; Log(Debug::Warning) << "Trailing data in path setting. Used '" << MaybeQuotedPath.string() << "' but '" << remainder << "' remained"; } } else { - std::string intermediate(std::istreambuf_iterator(istream), {}); + std::string intermediate{std::istreambuf_iterator(istream), {}}; static_cast(MaybeQuotedPath) = intermediate; } return istream;