From 537470553147b402ebdcc0f004ca03ffdd1baecf Mon Sep 17 00:00:00 2001 From: Mathias Hasselmann Date: Sat, 13 Jan 2024 00:10:25 +0100 Subject: [PATCH] Core: Change filterNewContent() to use a predicate At this point this might appear slightly over-designed, but when adding context matching and highlighting of matches the body of the lastBlock loop will become much more complex. Something you surly don't want to copy and paste. On the other hand the following changes are way too complex to mix them with this refactoring. That's what this change prepares for, and that's why it is separate. Task-number: QTCREATORBUG-30167 Change-Id: Idb0ed2471006ebaa199acb164043f8fea8fc0d42 Reviewed-by: hjk Reviewed-by: Eike Ziller --- src/plugins/coreplugin/outputwindow.cpp | 40 +++++++++++++++++-------- src/plugins/coreplugin/outputwindow.h | 3 ++ 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/plugins/coreplugin/outputwindow.cpp b/src/plugins/coreplugin/outputwindow.cpp index 6117917b0e1..c9b3ec86f32 100644 --- a/src/plugins/coreplugin/outputwindow.cpp +++ b/src/plugins/coreplugin/outputwindow.cpp @@ -379,30 +379,44 @@ void OutputWindow::setOutputFileNameHint(const QString &fileName) d->outputFileNameHint = fileName; } -void OutputWindow::filterNewContent() +OutputWindow::TextMatchingFunction OutputWindow::makeMatchingFunction() const { - QTextBlock lastBlock = document()->findBlockByNumber(d->lastFilteredBlockNumber); - if (!lastBlock.isValid()) - lastBlock = document()->begin(); - - const bool invert = d->filterMode.testFlag(FilterModeFlag::Inverted); - if (d->filterMode.testFlag(OutputWindow::FilterModeFlag::RegExp)) { + if (d->filterText.isEmpty()) { + return [](const QString &) { return true; }; + } else if (d->filterMode.testFlag(OutputWindow::FilterModeFlag::RegExp)) { QRegularExpression regExp(d->filterText); if (!d->filterMode.testFlag(OutputWindow::FilterModeFlag::CaseSensitive)) regExp.setPatternOptions(QRegularExpression::CaseInsensitiveOption); + if (!regExp.isValid()) + return [](const QString &) { return false; }; - for (; lastBlock != document()->end(); lastBlock = lastBlock.next()) - lastBlock.setVisible(d->filterText.isEmpty() - || regExp.match(lastBlock.text()).hasMatch() != invert); + return [regExp](const QString &text) { return regExp.match(text).hasMatch(); }; } else { const auto cs = d->filterMode.testFlag(OutputWindow::FilterModeFlag::CaseSensitive) ? Qt::CaseSensitive : Qt::CaseInsensitive; - for (; lastBlock != document()->end(); lastBlock = lastBlock.next()) - lastBlock.setVisible(d->filterText.isEmpty() - || lastBlock.text().contains(d->filterText, cs) != invert); + return [cs, filterText = d->filterText](const QString &text) { + return text.contains(filterText, cs); + }; } + return {}; +} + +void OutputWindow::filterNewContent() +{ + const auto findNextMatch = makeMatchingFunction(); + QTC_ASSERT(findNextMatch, return); + const bool invert = d->filterMode.testFlag(FilterModeFlag::Inverted) + && !d->filterText.isEmpty(); + + QTextBlock lastBlock = document()->findBlockByNumber(d->lastFilteredBlockNumber); + if (!lastBlock.isValid()) + lastBlock = document()->begin(); + + for (; lastBlock != document()->end(); lastBlock = lastBlock.next()) + lastBlock.setVisible(findNextMatch(lastBlock.text()) != invert); + d->lastFilteredBlockNumber = document()->lastBlock().blockNumber(); // FIXME: Why on earth is this necessary? We should probably do something else instead... diff --git a/src/plugins/coreplugin/outputwindow.h b/src/plugins/coreplugin/outputwindow.h index cd44e5678e9..19ab9a3c568 100644 --- a/src/plugins/coreplugin/outputwindow.h +++ b/src/plugins/coreplugin/outputwindow.h @@ -97,6 +97,9 @@ private: void handleOutputChunk(const QString &output, Utils::OutputFormat format); void updateAutoScroll(); + using TextMatchingFunction = std::function; + TextMatchingFunction makeMatchingFunction() const; + Internal::OutputWindowPrivate *d = nullptr; };