From af287062103e9d62173597a782c7dfa08ebf4d32 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Tue, 21 Jan 2025 20:14:15 +0000 Subject: [PATCH] Introduce ComparableFilename abstraction It's a typedef that's std::wstring on Windows and icu::UnicodeString on Linux, to simplify calling CompareFilenames() with cached pre-converted strings. --- src/api/helpers/text.cpp | 19 +++++---- src/api/helpers/text.h | 20 ++++++--- src/api/sorting/plugin_graph.cpp | 41 +++++++++---------- src/api/sorting/plugin_graph.h | 18 ++++---- src/api/sorting/plugin_sort.cpp | 13 ++---- src/api/sorting/plugin_sorting_data.cpp | 17 ++------ src/api/sorting/plugin_sorting_data.h | 7 +--- .../api/internals/sorting/plugin_sort_test.h | 11 ++--- .../sorting/plugin_sorting_data_test.h | 10 ++--- 9 files changed, 68 insertions(+), 88 deletions(-) diff --git a/src/api/helpers/text.cpp b/src/api/helpers/text.cpp index c2a26f6e..3a157212 100644 --- a/src/api/helpers/text.cpp +++ b/src/api/helpers/text.cpp @@ -180,18 +180,21 @@ std::string FromWinWide(const std::wstring& wstr) { } #endif -int CompareFilenames(const std::string& lhs, const std::string& rhs) { +ComparableFilename ToComparableFilename(const std::string filename) { #ifdef _WIN32 - return CompareFilenames(ToWinWide(lhs), ToWinWide(rhs)); + return ToWinWide(filename); #else - auto unicodeLhs = icu::UnicodeString::fromUTF8(lhs); - auto unicodeRhs = icu::UnicodeString::fromUTF8(rhs); - return unicodeLhs.caseCompare(unicodeRhs, U_FOLD_CASE_DEFAULT); + return icu::UnicodeString::fromUTF8(filename); #endif } +int CompareFilenames(const std::string& lhs, const std::string& rhs) { + return CompareFilenames(ToComparableFilename(lhs), ToComparableFilename(rhs)); +} + +int CompareFilenames(const ComparableFilename& lhs, + const ComparableFilename& rhs) { #ifdef _WIN32 -int CompareFilenames(const std::wstring& lhs, const std::wstring& rhs) { // Use CompareStringOrdinal as that will perform case conversion // using the operating system uppercase table information, which (I think) // will give results that match the filesystem, and is not locale-dependent. @@ -208,8 +211,10 @@ int CompareFilenames(const std::wstring& lhs, const std::wstring& rhs) { throw std::invalid_argument( "One of the filenames to compare was invalid."); } -} +#else + return lhs.caseCompare(rhs, U_FOLD_CASE_DEFAULT); #endif +} std::string NormalizeFilename(const std::string& filename) { #ifdef _WIN32 diff --git a/src/api/helpers/text.h b/src/api/helpers/text.h index 72d9e9e1..f3b0b50e 100644 --- a/src/api/helpers/text.h +++ b/src/api/helpers/text.h @@ -31,26 +31,36 @@ #include "loot/metadata/tag.h" +#ifndef _WIN32 +#define UNISTR_FROM_STRING_EXPLICIT explicit +#include +#endif + namespace loot { inline constexpr const char* GHOST_FILE_EXTENSION = ".ghost"; inline constexpr std::size_t GHOST_FILE_EXTENSION_LENGTH = std::char_traits::length(GHOST_FILE_EXTENSION); +#ifdef _WIN32 +typedef std::wstring ComparableFilename; +#else +typedef icu::UnicodeString ComparableFilename; +#endif + std::vector ExtractBashTags(const std::string& description); std::optional ExtractVersion(const std::string& text); +ComparableFilename ToComparableFilename(const std::string filename); + // Compare strings as if they're filenames, respecting filesystem case // insensitivity on Windows. Returns -1 if lhs < rhs, 0 if lhs == rhs, and 1 if // lhs > rhs. The comparison may give different results on Linux, but is still // locale-invariant. int CompareFilenames(const std::string& lhs, const std::string& rhs); -#ifdef _WIN32 -std::wstring ToWinWide(const std::string& str); - -int CompareFilenames(const std::wstring& lhs, const std::wstring& rhs); -#endif +int CompareFilenames(const ComparableFilename& lhs, + const ComparableFilename& rhs); // Normalize the given filename in a way that is locale-invariant. On Windows, // this uppercases the filename according to the same case mapping rules as used diff --git a/src/api/sorting/plugin_graph.cpp b/src/api/sorting/plugin_graph.cpp index 50fc265b..f6283a86 100644 --- a/src/api/sorting/plugin_graph.cpp +++ b/src/api/sorting/plugin_graph.cpp @@ -659,33 +659,35 @@ void PathsCache::CachePath(const vertex_t& fromVertex, } } -#if _WIN32 -const std::wstring& WideStringsCache::Get(const std::string& narrowString) { - auto vertexNameIt = wideStringsCache_.find(narrowString); - if (vertexNameIt == wideStringsCache_.end()) { +const ComparableFilename& ComparableFilenamesCache::Get( + const std::string& narrowString) { + auto vertexNameIt = comparableFilenamesCache_.find(narrowString); + if (vertexNameIt == comparableFilenamesCache_.end()) { throw std::invalid_argument("Given string was not already cached"); } return vertexNameIt->second; } -const std::wstring& WideStringsCache::GetOrInsert( +const ComparableFilename& ComparableFilenamesCache::GetOrInsert( const std::string& narrowString) { - auto vertexNameIt = wideStringsCache_.find(narrowString); - if (vertexNameIt == wideStringsCache_.end()) { + auto vertexNameIt = comparableFilenamesCache_.find(narrowString); + if (vertexNameIt == comparableFilenamesCache_.end()) { vertexNameIt = - wideStringsCache_.emplace(narrowString, ToWinWide(narrowString)).first; + comparableFilenamesCache_ + .emplace(narrowString, ToComparableFilename(narrowString)) + .first; } return vertexNameIt->second; } -void WideStringsCache::Insert(const std::string& narrowString) { - if (!wideStringsCache_.contains(narrowString)) { - wideStringsCache_.emplace(narrowString, ToWinWide(narrowString)); +void ComparableFilenamesCache::Insert(const std::string& narrowString) { + if (!comparableFilenamesCache_.contains(narrowString)) { + comparableFilenamesCache_.emplace(narrowString, + ToComparableFilename(narrowString)); } } -#endif size_t PluginGraph::CountVertices() const { return boost::num_vertices(graph_); @@ -696,19 +698,14 @@ std::pair PluginGraph::GetVertices() const { } std::optional PluginGraph::GetVertexByName( - const std::string& name) const { + const std::string& name) { for (const auto& vertex : boost::make_iterator_range(GetVertices())) { -#if _WIN32 const auto& vertexName = GetPlugin(vertex).GetName(); - wideStringCache_.Insert(vertexName); - auto& wideName = wideStringCache_.GetOrInsert(name); - auto& wideVertexName = wideStringCache_.Get(vertexName); + comparableFilenamesCache_.Insert(vertexName); + const auto& comparableName = comparableFilenamesCache_.GetOrInsert(name); + const auto& comparableVertexName = comparableFilenamesCache_.Get(vertexName); - int comparison = CompareFilenames(wideVertexName, wideName); -#else - int comparison = CompareFilenames(GetPlugin(vertex).GetName(), name); -#endif - if (comparison == 0) { + if (CompareFilenames(comparableVertexName, comparableName) == 0) { return vertex; } } diff --git a/src/api/sorting/plugin_graph.h b/src/api/sorting/plugin_graph.h index 052caa7f..5a0e1206 100644 --- a/src/api/sorting/plugin_graph.h +++ b/src/api/sorting/plugin_graph.h @@ -31,6 +31,7 @@ #include #include +#include "api/helpers/text.h" #include "api/sorting/group_sort.h" #include "api/sorting/plugin_sorting_data.h" #include "loot/enum/edge_type.h" @@ -58,23 +59,22 @@ private: pathsCache_; }; -#if _WIN32 -class WideStringsCache { +class ComparableFilenamesCache { public: void Insert(const std::string& narrowString); - const std::wstring& Get(const std::string& narrowString); - const std::wstring& GetOrInsert(const std::string& narrowString); + const ComparableFilename& Get(const std::string& narrowString); + const ComparableFilename& GetOrInsert(const std::string& narrowString); private: - boost::unordered_flat_map wideStringsCache_; + boost::unordered_flat_map + comparableFilenamesCache_; }; -#endif class PluginGraph { public: size_t CountVertices() const; std::pair GetVertices() const; - std::optional GetVertexByName(const std::string& name) const; + std::optional GetVertexByName(const std::string& name); const PluginSortingData& GetPlugin(const vertex_t& vertex) const; @@ -112,9 +112,7 @@ public: private: RawPluginGraph graph_; PathsCache pathsCache_; -#if _WIN32 - mutable WideStringsCache wideStringCache_; -#endif + ComparableFilenamesCache comparableFilenamesCache_; }; } diff --git a/src/api/sorting/plugin_sort.cpp b/src/api/sorting/plugin_sort.cpp index bc804bcc..09bb0fe2 100644 --- a/src/api/sorting/plugin_sort.cpp +++ b/src/api/sorting/plugin_sort.cpp @@ -39,12 +39,10 @@ std::vector GetPluginsSortingData( std::vector pluginsSortingData; pluginsSortingData.reserve(loadedPluginInterfaces.size()); -#ifdef _WIN32 - std::vector wideLoadOrder; + std::vector comparableLoadOrder; for (const auto& pluginName : loadOrder) { - wideLoadOrder.push_back(ToWinWide(pluginName)); + comparableLoadOrder.push_back(ToComparableFilename(pluginName)); } -#endif for (const auto& pluginInterface : loadedPluginInterfaces) { if (!pluginInterface) { @@ -64,13 +62,8 @@ std::vector GetPluginsSortingData( const auto userMetadata = db.GetPluginUserMetadata(plugin->GetName(), true) .value_or(PluginMetadata(plugin->GetName())); -#ifdef _WIN32 const auto pluginSortingData = PluginSortingData( - plugin, masterlistMetadata, userMetadata, wideLoadOrder); -#else - const auto pluginSortingData = - PluginSortingData(plugin, masterlistMetadata, userMetadata, loadOrder); -#endif + plugin, masterlistMetadata, userMetadata, comparableLoadOrder); pluginsSortingData.push_back(pluginSortingData); } diff --git a/src/api/sorting/plugin_sorting_data.cpp b/src/api/sorting/plugin_sorting_data.cpp index 56a1e501..253c8403 100644 --- a/src/api/sorting/plugin_sorting_data.cpp +++ b/src/api/sorting/plugin_sorting_data.cpp @@ -53,11 +53,7 @@ PluginSortingData::PluginSortingData( const PluginSortingInterface* plugin, const PluginMetadata& masterlistMetadata, const PluginMetadata& userMetadata, -#ifdef _WIN32 - const std::vector& loadOrder) : -#else - const std::vector& loadOrder) : -#endif + const std::vector& loadOrder) : plugin_(plugin), name_(plugin == nullptr ? std::string() : plugin->GetName()), isMaster_(plugin != nullptr && plugin->IsMaster()), @@ -72,17 +68,10 @@ PluginSortingData::PluginSortingData( return; } -#ifdef _WIN32 - auto wideName = ToWinWide(GetName()); -#endif + const auto comparableName = ToComparableFilename(GetName()); for (size_t i = 0; i < loadOrder.size(); i++) { -#ifdef _WIN32 - int comparison = CompareFilenames(wideName, loadOrder.at(i)); -#else - int comparison = CompareFilenames(GetName(), loadOrder.at(i)); -#endif - if (comparison == 0) { + if (CompareFilenames(comparableName, loadOrder.at(i)) == 0) { loadOrderIndex_ = i; break; } diff --git a/src/api/sorting/plugin_sorting_data.h b/src/api/sorting/plugin_sorting_data.h index 715cbcf9..721d41cb 100644 --- a/src/api/sorting/plugin_sorting_data.h +++ b/src/api/sorting/plugin_sorting_data.h @@ -28,6 +28,7 @@ #include #include "api/plugin.h" +#include "api/helpers/text.h" #include "loot/metadata/plugin_metadata.h" namespace loot { @@ -43,11 +44,7 @@ public: explicit PluginSortingData(const PluginSortingInterface* plugin, const PluginMetadata& masterlistMetadata, const PluginMetadata& userMetadata, -#ifdef _WIN32 - const std::vector& loadOrder); -#else - const std::vector& loadOrder); -#endif + const std::vector& loadOrder); const std::string& GetName() const; bool IsMaster() const; diff --git a/src/tests/api/internals/sorting/plugin_sort_test.h b/src/tests/api/internals/sorting/plugin_sort_test.h index 955263f4..c493049f 100644 --- a/src/tests/api/internals/sorting/plugin_sort_test.h +++ b/src/tests/api/internals/sorting/plugin_sort_test.h @@ -83,18 +83,13 @@ protected: const std::vector& loadOrder = {}) { const auto plugin = GetPlugin(name); -#ifdef _WIN32 - std::vector wideLoadOrder; + std::vector comparableLoadOrder; for (const auto& pluginName : loadOrder) { - wideLoadOrder.push_back(ToWinWide(pluginName)); + comparableLoadOrder.push_back(ToComparableFilename(pluginName)); } return PluginSortingData( - plugin, PluginMetadata(), PluginMetadata(), wideLoadOrder); -#else - return PluginSortingData( - plugin, PluginMetadata(), PluginMetadata(), loadOrder); -#endif + plugin, PluginMetadata(), PluginMetadata(), comparableLoadOrder); } plugingraph::TestPlugin* GetPlugin(const std::string& name) { diff --git a/src/tests/api/internals/sorting/plugin_sorting_data_test.h b/src/tests/api/internals/sorting/plugin_sorting_data_test.h index c86c46e8..ccf28c82 100644 --- a/src/tests/api/internals/sorting/plugin_sorting_data_test.h +++ b/src/tests/api/internals/sorting/plugin_sorting_data_test.h @@ -63,18 +63,14 @@ protected: return loadedPluginInterfaces; } -#ifdef _WIN32 - std::vector getNativeLoadOrder() { - std::vector wideLoadOrder; + std::vector getNativeLoadOrder() { + std::vector wideLoadOrder; for (const auto &pluginName : getLoadOrder()) { - wideLoadOrder.push_back(ToWinWide(pluginName)); + wideLoadOrder.push_back(ToComparableFilename(pluginName)); } return wideLoadOrder; } -#else - std::vector getNativeLoadOrder() { return getLoadOrder(); } -#endif Game game_; const std::string blankEslEsp;