diff --git a/src/models/disassemblymodel.cpp b/src/models/disassemblymodel.cpp index 92368324..5daec8d4 100644 --- a/src/models/disassemblymodel.cpp +++ b/src/models/disassemblymodel.cpp @@ -244,7 +244,8 @@ void DisassemblyModel::find(const QString& search, Direction direction, int curr auto endReached = [this] { emit searchEndReached(); }; - const int resultIndex = ::search(m_data.disassemblyLines, current, direction, searchFunc, endReached); + const int resultIndex = ::search(m_data.disassemblyLines.cbegin(), m_data.disassemblyLines.cend(), current, + direction, searchFunc, endReached); if (resultIndex >= 0) { emit resultFound(createIndex(resultIndex, DisassemblyColumn)); diff --git a/src/models/search.h b/src/models/search.h index 16a0655e..45d4e3be 100644 --- a/src/models/search.h +++ b/src/models/search.h @@ -9,7 +9,6 @@ #include #include -#include enum class Direction { @@ -17,19 +16,17 @@ enum class Direction Backward }; -/** a search function that wrap around at the end +/** a search function that wraps around at the end * this function evaluates searchFunc starting from current and returns the offset from begin. In case end is reached, * it calls endReached and starts again at begin * * return: offset from begin * */ -template -int search_impl(iter begin, iter end, iter current, SearchFunc searchFunc, EndReached endReached) +template +int search_helper(const it begin, const it end, const it current, SearchFunc searchFunc, EndReached endReached) { - if (begin == end) - return -1; - - const auto start = (current == end) ? begin : (current + 1); + // if current points to the last line, current will now point to end -> wrap around + const auto start = (current == end) ? begin : current; auto found = std::find_if(start, end, searchFunc); if (found != end) { @@ -40,28 +37,28 @@ int search_impl(iter begin, iter end, iter current, SearchFunc searchFunc, EndRe found = std::find_if(begin, start, searchFunc); - if (found != end) { + if (found != start) { return std::distance(begin, found); } return -1; } -// a wrapper around search_impl that returns the result index from begin -template -int search(const Array& array, int current, Direction direction, SearchFunc searchFunc, EndReached endReached) +template +int search(const it begin, const it end, int current, Direction direction, SearchFunc searchFunc, EndReached endReached) { - current = std::clamp(0, static_cast(array.size()), current); - int resultIndex = 0; + if (begin == end) + return -1; + + const auto size = std::distance(begin, end); + current = std::clamp(current, 0, static_cast(size) - 1); + const auto currentIt = begin + current; if (direction == Direction::Forward) { - resultIndex = ::search_impl(array.begin(), array.end(), array.begin() + current, searchFunc, endReached); - } else { - resultIndex = ::search_impl(array.rbegin(), array.rend(), - std::make_reverse_iterator(array.begin() + current) - 1, searchFunc, endReached); - if (resultIndex != -1) { - resultIndex = array.size() - resultIndex - 1; - } + return search_helper(begin, end, std::next(currentIt), searchFunc, endReached); } - return resultIndex; + + int resultIndex = search_helper(std::make_reverse_iterator(end), std::make_reverse_iterator(begin), + std::make_reverse_iterator(currentIt), searchFunc, endReached); + return resultIndex != -1 ? (size - resultIndex - 1) : -1; } diff --git a/src/models/sourcecodemodel.cpp b/src/models/sourcecodemodel.cpp index cfc6d4fe..26b91012 100644 --- a/src/models/sourcecodemodel.cpp +++ b/src/models/sourcecodemodel.cpp @@ -254,8 +254,8 @@ void SourceCodeModel::find(const QString& search, Direction direction, int curre auto searchFunc = [&search](const QString& line) { return line.indexOf(search, 0, Qt::CaseInsensitive) != -1; }; auto endReached = [this] { emit searchEndReached(); }; - - const int resultIndex = ::search(m_lines.mid(m_startLine, m_numLines), current, direction, searchFunc, endReached); + const int resultIndex = ::search(m_lines.cbegin() + m_startLine, m_lines.cbegin() + m_startLine + m_numLines, + current, direction, searchFunc, endReached); if (resultIndex >= 0) { emit resultFound(createIndex(resultIndex + 1, SourceCodeColumn)); diff --git a/src/resultsdisassemblypage.cpp b/src/resultsdisassemblypage.cpp index 9067ef57..813afe8a 100644 --- a/src/resultsdisassemblypage.cpp +++ b/src/resultsdisassemblypage.cpp @@ -402,7 +402,6 @@ ResultsDisassemblyPage::ResultsDisassemblyPage(CostContextMenu* costContextMenu, auto searchPrev = [model, edit, additionalRows, searchResultIndex] { const auto offset = searchResultIndex->isValid() ? searchResultIndex->row() - additionalRows : 0; - model->find(edit->text(), Direction::Backward, offset); }; diff --git a/tests/modeltests/tst_models.cpp b/tests/modeltests/tst_models.cpp index 5e4cee09..c1940075 100644 --- a/tests/modeltests/tst_models.cpp +++ b/tests/modeltests/tst_models.cpp @@ -11,10 +11,13 @@ #include #include #include +#include +#include #include #include #include "../testutils.h" +#include "search.h" #include #include @@ -473,6 +476,88 @@ private slots: 28); } + void testSourceCodeModelSearch() + { + QTemporaryFile file; + if (!file.open()) { + QSKIP("Failed to create test file"); + } + + for (int i = 0; i < 10; i++) { + file.write(QStringLiteral("Line %1\n").arg(i).toUtf8()); + } + + file.flush(); + + DisassemblyOutput output; + output.mainSourceFileName = file.fileName(); + output.realSourceFileName = file.fileName(); + + DisassemblyOutput::DisassemblyLine line1; + line1.fileLine = Data::FileLine {file.fileName(), 4}; + + DisassemblyOutput::DisassemblyLine line2; + line2.fileLine = Data::FileLine {file.fileName(), 8}; + + output.disassemblyLines = {line1, line2}; + + SourceCodeModel model(nullptr); + model.setDisassembly(output, {}); + + QCOMPARE(model.rowCount(), 6); // 5 lines + function name + QCOMPARE(model.data(model.index(1, SourceCodeModel::SourceCodeColumn), Qt::DisplayRole).value(), + QStringLiteral("Line 3")); + + QCOMPARE(model.data(model.index(5, SourceCodeModel::SourceCodeColumn), Qt::DisplayRole).value(), + QStringLiteral("Line 7")); + + // check if search works in general + QSignalSpy searchSpy(&model, &SourceCodeModel::resultFound); + for (int i = 0; i < 5; i++) { + model.find(QStringLiteral("Line 5"), Direction::Forward, i); + auto result = searchSpy.takeFirst(); + QCOMPARE(result.at(0).value(), model.index(3, SourceCodeModel::SourceCodeColumn)); + } + + // Check wrap around + for (int i = 1; i < 4; i++) { + QSignalSpy endReached(&model, &SourceCodeModel::searchEndReached); + model.find(QStringLiteral("Line 3"), Direction::Forward, i); + QCOMPARE(endReached.size(), 1); + } + + // check if no result found works + searchSpy.clear(); + for (int i = 0; i < 5; i++) { + model.find(QStringLiteral("Line 8"), Direction::Forward, i); + auto result = searchSpy.takeFirst(); + qDebug() << i; + QCOMPARE(result.at(0).value().isValid(), false); + } + + // test backward search + for (int i = 4; i > 0; i--) { + model.find(QStringLiteral("Line 7"), Direction::Backward, i); + auto result = searchSpy.takeFirst(); + QCOMPARE(result.at(0).value(), model.index(5, SourceCodeModel::SourceCodeColumn)); + } + + // Check wrap around + for (int i = 4; i > 0; i--) { + QSignalSpy endReached(&model, &SourceCodeModel::searchEndReached); + model.find(QStringLiteral("Line 7"), Direction::Backward, i); + QCOMPARE(endReached.size(), 1); + } + + // check if no result found works + searchSpy.clear(); + for (int i = 0; i < 5; i++) { + model.find(QStringLiteral("Line 8"), Direction::Backward, i); + auto result = searchSpy.takeFirst(); + QCOMPARE(result.at(0).value().isValid(), false); + } + } + void testEventModel() { Data::EventResults events; diff --git a/tests/modeltests/tst_search.cpp b/tests/modeltests/tst_search.cpp index 4193d788..8bf95978 100644 --- a/tests/modeltests/tst_search.cpp +++ b/tests/modeltests/tst_search.cpp @@ -23,11 +23,11 @@ private slots: { const std::array testArray = {}; - QCOMPARE(search_impl( - testArray.begin(), testArray.end(), testArray.begin(), [](int) { return false; }, [] {}), + QCOMPARE(search( + testArray.cbegin(), testArray.cend(), 0, Direction::Forward, [](int) { return false; }, [] {}), -1); - QCOMPARE(search_impl( - testArray.rbegin(), testArray.rend(), testArray.rbegin(), [](int) { return false; }, [] {}), + QCOMPARE(search( + testArray.cbegin(), testArray.cend(), 0, Direction::Backward, [](int) { return false; }, [] {}), -1); } void testSearch() @@ -36,14 +36,14 @@ private slots: int maxOffset = testArray.size() - 1; for (int offset = 0; offset < maxOffset; offset++) { - QCOMPARE(search_impl( - testArray.begin(), testArray.end(), testArray.begin() + offset, + QCOMPARE(search( + testArray.cbegin(), testArray.cend(), offset, Direction::Forward, [](int num) { return num == 2; }, [] {}), 1); - QCOMPARE(search_impl( - testArray.rbegin(), testArray.rend(), testArray.rbegin() + offset, + QCOMPARE(search( + testArray.cbegin(), testArray.cend(), offset, Direction::Backward, [](int num) { return num == 2; }, [] {}), - 3); + 1); } } @@ -52,8 +52,8 @@ private slots: const std::array testArray = {1, 2, 3, 4, 5}; { bool endReached = false; - QCOMPARE(search_impl( - testArray.begin(), testArray.end(), testArray.begin() + 1, [](int i) { return i == 1; }, + QCOMPARE(search( + testArray.cbegin(), testArray.cend(), 1, Direction::Forward, [](int i) { return i == 1; }, [&endReached] { endReached = true; }), 0); QCOMPARE(endReached, true); @@ -61,26 +61,11 @@ private slots: { bool endReached = false; - QCOMPARE(search_impl( - testArray.rbegin(), testArray.rend(), std::make_reverse_iterator(testArray.begin() + 4 - 1), - [](int i) { return i == 4; }, [&endReached] { endReached = true; }), - 1); - QCOMPARE(endReached, true); - } - } - - void testWrapper() - { - const std::array testArray = {1, 2, 3, 4, 5}; - - int maxOffset = testArray.size() - 1; - for (int i = 0; i < maxOffset; i++) { - QCOMPARE(search( - testArray, i, Direction::Forward, [](int i) { return i == 4; }, [] {}), - 3); QCOMPARE(search( - testArray, i, Direction::Backward, [](int i) { return i == 4; }, [] {}), + testArray.cbegin(), testArray.cend(), 1, Direction::Backward, [](int i) { return i == 4; }, + [&endReached] { endReached = true; }), 3); + QCOMPARE(endReached, true); } } @@ -90,7 +75,7 @@ private slots: for (int i = 0; i < 2; i++) { QCOMPARE(search( - testArray, i, Direction::Forward, [](int) { return true; }, [] {}), + testArray.cbegin(), testArray.cend(), 0, Direction::Forward, [](int) { return true; }, [] {}), -1); } } @@ -100,9 +85,21 @@ private slots: const std::array testArray = {0}; QCOMPARE(search( - testArray, 1, Direction::Forward, [](int i) { return i == 0; }, [] {}), + testArray.cbegin(), testArray.cend(), 1, Direction::Forward, [](int i) { return i == 0; }, [] {}), 0); } + + void testSearchOnIterators() + { + const std::array testArray = {0, 1, 2, 3, 0}; + + for (int i = 1; i < 3; i++) { + QCOMPARE(search( + testArray.cbegin() + 1, testArray.cbegin() + 3, i, Direction::Forward, + [](int i) { return i == 0; }, [] {}), + -1); + } + } }; QTEST_GUILESS_MAIN(TestSearch)