Skip to content

Commit 24b969f

Browse files
CapostrophicCapostrophic
committed
Merge branch 'search' into 'master'
Reimplement weightedSearch to prevent crashing with large menus Closes #8601 See merge request OpenMW/openmw!5001
2 parents 1223ca0 + b621d09 commit 24b969f

File tree

4 files changed

+102
-46
lines changed

4 files changed

+102
-46
lines changed

‎apps/openmw/mwgui/settingswindow.cpp‎

Lines changed: 15 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
#include <array>
44
#include <iomanip>
5-
#include <regex>
65

76
#include <unicode/locid.h>
87

@@ -43,6 +42,7 @@
4342
#include "../mwlua/luamanagerimp.hpp"
4443

4544
#include "confirmationdialog.hpp"
45+
#include "weightedsearch.hpp"
4646

4747
namespace
4848
{
@@ -942,42 +942,6 @@ namespace MWGui
942942
mControlsBox->setVisibleVScroll(true);
943943
}
944944

945-
namespace
946-
{
947-
std::string escapeRegex(const std::string& str)
948-
{
949-
static const std::regex specialChars(R"r([\^\.\[\$\(\)\|\*\+\?\{])r", std::regex_constants::extended);
950-
return std::regex_replace(str, specialChars, R"(\$&)");
951-
}
952-
953-
std::regex wordSearch(const std::string& query)
954-
{
955-
static const std::regex wordsRegex(R"([^[:space:]]+)", std::regex_constants::extended);
956-
auto wordsBegin = std::sregex_iterator(query.begin(), query.end(), wordsRegex);
957-
auto wordsEnd = std::sregex_iterator();
958-
std::string searchRegex("(");
959-
for (auto it = wordsBegin; it != wordsEnd; ++it)
960-
{
961-
if (it != wordsBegin)
962-
searchRegex += '|';
963-
searchRegex += escapeRegex(query.substr(it->position(), it->length()));
964-
}
965-
searchRegex += ')';
966-
// query had only whitespace characters
967-
if (searchRegex == "()")
968-
searchRegex = "^(.*)$";
969-
return std::regex(searchRegex, std::regex_constants::extended | std::regex_constants::icase);
970-
}
971-
972-
double weightedSearch(const std::regex& regex, const std::string& text)
973-
{
974-
std::smatch matches;
975-
std::regex_search(text, matches, regex);
976-
// need a signed value, so cast to double (not an integer type to guarantee no overflow)
977-
return static_cast<double>(matches.size());
978-
}
979-
}
980-
981945
void SettingsWindow::renderScriptSettings()
982946
{
983947
mScriptAdapter->detach();
@@ -989,24 +953,29 @@ namespace MWGui
989953
{
990954
size_t mIndex;
991955
std::string mName;
992-
double mNameWeight;
993-
double mHintWeight;
994-
995-
constexpr auto tie() const { return std::tie(mNameWeight, mHintWeight, mName); }
956+
size_t mNameWeight;
957+
size_t mHintWeight;
996958

997-
constexpr bool operator<(const WeightedPage& rhs) const { return tie() < rhs.tie(); }
959+
constexpr bool operator<(const WeightedPage& rhs) const
960+
{
961+
if (mNameWeight != rhs.mNameWeight)
962+
return mNameWeight > rhs.mNameWeight;
963+
if (mHintWeight != rhs.mHintWeight)
964+
return mHintWeight > rhs.mHintWeight;
965+
return mName < rhs.mName;
966+
}
998967
};
999968

1000-
std::regex searchRegex = wordSearch(mScriptFilter->getCaption());
969+
const std::vector<std::string> patternArray = generatePatternArray(mScriptFilter->getCaption());
1001970
std::vector<WeightedPage> weightedPages;
1002971
weightedPages.reserve(LuaUi::scriptSettingsPageCount());
1003972
for (size_t i = 0; i < LuaUi::scriptSettingsPageCount(); ++i)
1004973
{
1005974
LuaUi::ScriptSettingsPage page = LuaUi::scriptSettingsPageAt(i);
1006-
double nameWeight = weightedSearch(searchRegex, page.mName);
1007-
double hintWeight = weightedSearch(searchRegex, page.mSearchHints);
975+
size_t nameWeight = weightedSearch(page.mName, patternArray);
976+
size_t hintWeight = weightedSearch(page.mSearchHints, patternArray);
1008977
if ((nameWeight + hintWeight) > 0)
1009-
weightedPages.push_back({ i, page.mName, -nameWeight, -hintWeight });
978+
weightedPages.push_back({ i, page.mName, nameWeight, hintWeight });
1010979
}
1011980
std::sort(weightedPages.begin(), weightedPages.end());
1012981
for (const WeightedPage& weightedPage : weightedPages)

‎apps/openmw/mwgui/weightedsearch.hpp‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
#ifndef MWGUI_WEIGHTEDSEARCH_H
2+
#define MWGUI_WEIGHTEDSEARCH_H
3+
4+
#include <cstddef>
5+
#include <iterator>
6+
#include <sstream>
7+
#include <string>
8+
#include <vector>
9+
10+
#include <components/misc/strings/lower.hpp>
11+
12+
namespace MWGui
13+
{
14+
inline std::vector<std::string> generatePatternArray(const std::string& inputString)
15+
{
16+
if (inputString.empty() || inputString.find_first_not_of(" ") == std::string::npos)
17+
return std::vector<std::string>();
18+
std::string inputStringLowerCase = Misc::StringUtils::lowerCase(inputString);
19+
std::istringstream stringStream(inputStringLowerCase);
20+
return { std::istream_iterator<std::string>(stringStream), std::istream_iterator<std::string>() };
21+
}
22+
inline std::size_t weightedSearch(const std::string& corpus, const std::vector<std::string>& patternArray)
23+
{
24+
if (patternArray.empty())
25+
return 1;
26+
27+
std::string corpusLowerCase = Misc::StringUtils::lowerCase(corpus);
28+
29+
std::size_t numberOfMatches = 0;
30+
for (const std::string& word : patternArray)
31+
numberOfMatches += corpusLowerCase.find(word) != std::string::npos ? 1 : 0;
32+
33+
return numberOfMatches;
34+
}
35+
}
36+
37+
#endif

‎apps/openmw_tests/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ file(GLOB UNITTEST_SRC_FILES
1515
mwdialogue/testkeywordsearch.cpp
1616

1717
mwgui/tooltips.cpp
18+
mwgui/weightedsearch.cpp
1819

1920
mwscript/testscripts.cpp
2021
)

‎apps/openmw_tests/mwgui/weightedsearch.cpp‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#include <gmock/gmock.h>
2+
#include <gtest/gtest.h>
3+
4+
#include <algorithm>
5+
#include <string>
6+
#include <vector>
7+
8+
#include "apps/openmw/mwgui/weightedsearch.hpp"
9+
10+
namespace MWGui
11+
{
12+
namespace
13+
{
14+
TEST(MWGuiWeightedSearchTests, weightedSearchShouldNotCrashWithLargeCorpus)
15+
{
16+
EXPECT_NO_THROW(weightedSearch(std::string(100000, 'x'), std::vector<std::string>{ "x" }));
17+
}
18+
TEST(MWGuiWeightedSearchTests, weightedSearchShouldReturn1WithEmptyPatternArray)
19+
{
20+
EXPECT_EQ(weightedSearch(std::string(100, 'x'), std::vector<std::string>{}), 1);
21+
}
22+
TEST(MWGuiWeightedSearchTests, weightedSearchShouldReturnTheSumOfAllPatternsWithAtLeastOneMatch)
23+
{
24+
EXPECT_EQ(weightedSearch(std::string("xyyzzz"), std::vector<std::string>{ "x", "y", "z" }), 3);
25+
}
26+
TEST(MWGuiWeightedSearchTests, weightedSearchShouldBeCaseInsensitive)
27+
{
28+
EXPECT_EQ(weightedSearch(std::string("XYZ"), std::vector<std::string>{ "x", "y", "z" }), 3);
29+
}
30+
TEST(MWGuiWeightedSearchTests, generatePatternArrayShouldReturnEmptyArrayIfInputIsEmptyOrOnlySpaces)
31+
{
32+
EXPECT_THAT(generatePatternArray(std::string("")), testing::IsEmpty());
33+
EXPECT_THAT(generatePatternArray(std::string(10, ' ')), testing::IsEmpty());
34+
}
35+
TEST(MWGuiWeightedSearchTests, generatePatternArrayBasicSplittingTest)
36+
{
37+
std::vector<std::string> expected = { "x", "y", "z" };
38+
39+
std::vector<std::string> output1 = generatePatternArray(std::string("x y z"));
40+
std::sort(output1.begin(), output1.end());
41+
42+
std::vector<std::string> output2 = generatePatternArray(std::string(" x y z "));
43+
std::sort(output2.begin(), output2.end());
44+
45+
EXPECT_EQ(output1, expected);
46+
EXPECT_EQ(output2, expected);
47+
}
48+
}
49+
}

0 commit comments

Comments
 (0)