From 1a98dda5c417e892cadbedb656829cf2ba4d1d0a Mon Sep 17 00:00:00 2001 From: David Schulz Date: Thu, 11 May 2023 09:34:53 +0200 Subject: [PATCH] Utils: fix Text::Range length and remove mid Those functions are based on the assumption that the passed text starts at the begin position, which was good enough for search results, but if used in other parts of the codebase it might give unwanted results. Calculate the length of the range now as expected and subtract the beginning lines. In order to still got the correct results for the text result texts modify the result range to always start at the first line before calculating the length of the range. Also add tests for the modified functionality Change-Id: I7ccd75b642dda6dd4f738877cbe3543d46c03652 Reviewed-by: Qt CI Bot Reviewed-by: Jarek Kobus --- src/libs/utils/textutils.cpp | 26 ++++++--- src/libs/utils/textutils.h | 2 +- .../coreplugin/find/searchresulttreemodel.cpp | 8 ++- src/plugins/texteditor/basefilefind.cpp | 10 +++- tests/auto/utils/CMakeLists.txt | 1 + tests/auto/utils/text/CMakeLists.txt | 4 ++ tests/auto/utils/text/text.qbs | 7 +++ tests/auto/utils/text/tst_text.cpp | 58 +++++++++++++++++++ 8 files changed, 101 insertions(+), 15 deletions(-) create mode 100644 tests/auto/utils/text/CMakeLists.txt create mode 100644 tests/auto/utils/text/text.qbs create mode 100644 tests/auto/utils/text/tst_text.cpp diff --git a/src/libs/utils/textutils.cpp b/src/libs/utils/textutils.cpp index 34dbb5e1fa5..8cf7a41ed60 100644 --- a/src/libs/utils/textutils.cpp +++ b/src/libs/utils/textutils.cpp @@ -58,22 +58,30 @@ Position Position::fromPositionInDocument(const QTextDocument *document, int pos int Range::length(const QString &text) const { + if (end.line < begin.line) + return -1; + if (begin.line == end.line) return end.column - begin.column; - const int lineCount = end.line - begin.line; - int index = text.indexOf(QChar::LineFeed); + int index = 0; int currentLine = 1; - while (index > 0 && currentLine < lineCount) { - ++index; + while (currentLine < begin.line) { index = text.indexOf(QChar::LineFeed, index); + if (index < 0) + return -1; + ++index; ++currentLine; } - - if (index < 0) - return 0; - - return index - begin.column + end.column; + const int beginIndex = index + begin.column; + while (currentLine < end.line) { + index = text.indexOf(QChar::LineFeed, index); + if (index < 0) + return -1; + ++index; + ++currentLine; + } + return index + end.column - beginIndex; } bool Range::operator==(const Range &other) const diff --git a/src/libs/utils/textutils.h b/src/libs/utils/textutils.h index 4ba3b0e1a82..6ee82274dc8 100644 --- a/src/libs/utils/textutils.h +++ b/src/libs/utils/textutils.h @@ -37,7 +37,6 @@ public: class QTCREATOR_UTILS_EXPORT Range { public: - QString mid(const QString &text) const { return text.mid(begin.column, length(text)); } int length(const QString &text) const; Position begin; @@ -97,3 +96,4 @@ QTCREATOR_UTILS_EXPORT QString utf16LineTextInUtf8Buffer(const QByteArray &utf8B } // Utils Q_DECLARE_METATYPE(Utils::Text::Position) +Q_DECLARE_METATYPE(Utils::Text::Range) diff --git a/src/plugins/coreplugin/find/searchresulttreemodel.cpp b/src/plugins/coreplugin/find/searchresulttreemodel.cpp index 769c63a6bca..b6d620b80b5 100644 --- a/src/plugins/coreplugin/find/searchresulttreemodel.cpp +++ b/src/plugins/coreplugin/find/searchresulttreemodel.cpp @@ -322,9 +322,13 @@ QVariant SearchResultTreeModel::data(const SearchResultTreeItem *row, int role) case ItemDataRoles::ResultBeginColumnNumberRole: result = row->item.mainRange().begin.column; break; - case ItemDataRoles::SearchTermLengthRole: - result = row->item.mainRange().length(row->item.lineText()); + case ItemDataRoles::SearchTermLengthRole:{ + Text::Range range = row->item.mainRange(); + range.end.line -= range.begin.line - 1; + range.begin.line = 1; + result = range.length(row->item.lineText()); break; + } case ItemDataRoles::ContainingFunctionNameRole: result = row->item.containingFunctionName().value_or(QString{}); break; diff --git a/src/plugins/texteditor/basefilefind.cpp b/src/plugins/texteditor/basefilefind.cpp index 197a2fa6968..974f1472912 100644 --- a/src/plugins/texteditor/basefilefind.cpp +++ b/src/plugins/texteditor/basefilefind.cpp @@ -486,9 +486,13 @@ FilePaths BaseFileFind::replaceAll(const QString &text, const SearchResultItems if (item.userData().canConvert() && !item.userData().toStringList().isEmpty()) { replacement = Utils::expandRegExpReplacement(text, item.userData().toStringList()); } else if (preserveCase) { - const QString originalText = (item.mainRange().length(item.lineText()) == 0) - ? item.lineText() - : item.mainRange().mid(item.lineText()); + Text::Range range = item.mainRange(); + range.end.line -= range.begin.line - 1; + range.begin.line = 1; + QString originalText = item.lineText(); + const int rangeLength = range.length(item.lineText()); + if (rangeLength > 0) + originalText = originalText.mid(range.begin.column, rangeLength); replacement = Utils::matchCaseReplacement(originalText, text); } else { replacement = text; diff --git a/tests/auto/utils/CMakeLists.txt b/tests/auto/utils/CMakeLists.txt index 36f20fdac52..1bb4bb641ae 100644 --- a/tests/auto/utils/CMakeLists.txt +++ b/tests/auto/utils/CMakeLists.txt @@ -17,4 +17,5 @@ add_subdirectory(stringutils) add_subdirectory(tasktree) add_subdirectory(templateengine) add_subdirectory(treemodel) +add_subdirectory(text) add_subdirectory(unixdevicefileaccess) diff --git a/tests/auto/utils/text/CMakeLists.txt b/tests/auto/utils/text/CMakeLists.txt new file mode 100644 index 00000000000..78fd3838f42 --- /dev/null +++ b/tests/auto/utils/text/CMakeLists.txt @@ -0,0 +1,4 @@ +add_qtc_test(tst_utils_text + DEPENDS Utils + SOURCES tst_text.cpp +) diff --git a/tests/auto/utils/text/text.qbs b/tests/auto/utils/text/text.qbs new file mode 100644 index 00000000000..828dec8d117 --- /dev/null +++ b/tests/auto/utils/text/text.qbs @@ -0,0 +1,7 @@ +import qbs + +QtcAutotest { + name: "Utils::Text autotest" + Depends { name: "Utils" } + files: "tst_text.cpp" +} diff --git a/tests/auto/utils/text/tst_text.cpp b/tests/auto/utils/text/tst_text.cpp new file mode 100644 index 00000000000..a2e9dc5037b --- /dev/null +++ b/tests/auto/utils/text/tst_text.cpp @@ -0,0 +1,58 @@ +// Copyright (C) 2016 The Qt Company Ltd. +// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0 + +#include + +#include + +using namespace Utils::Text; + +class tst_Text : public QObject +{ + Q_OBJECT + +private slots: + void testRangeLength_data(); + void testRangeLength(); +}; + +void tst_Text::testRangeLength_data() +{ + QTest::addColumn("text"); + QTest::addColumn("range"); + QTest::addColumn("length"); + + QTest::newRow("empty range") << QString() << Range{{1, 0}, {1, 0}} << 0; + + QTest::newRow("range on same line") << QString() << Range{{1, 0}, {1, 1}} << 1; + + QTest::newRow("range spanning lines") << QString("foo\nbar") << Range{{1, 0}, {2, 0}} << 4; + + QTest::newRow("range spanning lines and column offset") + << QString("foo\nbar") << Range{{1, 1}, {2, 1}} << 4; + + QTest::newRow("range spanning lines and begin column offset") + << QString("foo\nbar") << Range{{1, 1}, {2, 0}} << 3; + + QTest::newRow("range spanning lines and end column offset") + << QString("foo\nbar") << Range{{1, 0}, {2, 1}} << 5; + + QTest::newRow("hyper range") << QString("foo\nbar\nfoobar") << Range{{2, 1}, {3, 1}} << 4; + + QTest::newRow("flipped range") << QString() << Range{{2, 0}, {1, 0}} << -1; + + QTest::newRow("out of range") << QString() << Range{{1, 0}, {2, 0}} << -1; +} + +void tst_Text::testRangeLength() +{ + QFETCH(QString, text); + QFETCH(Range, range); + QFETCH(int, length); + + QCOMPARE(range.length(text), length); +} + +QTEST_GUILESS_MAIN(tst_Text) + +#include "tst_text.moc"