From a22e3acf2841632bcca5fccdbb7718543deb5079 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Tue, 21 Mar 2023 00:07:22 +0100 Subject: [PATCH] LocatorFilterEntry: Discourage the use of internalData Drop internalData from c'tor. The internalData is going to be removed, soon. Drop also the icon arg from c'tor since LocatorFilterEntry instances are usually created in non-main thread, while operating on QIcon instances isn't really safe in non-main thread. The use of QIcon inside this struct is a subject to change in the future, in a way like it was done in other parts of code that generated icons from non-main thread. Change-Id: Ic6aa719a64e5fbd65883c54149796057c632780e Reviewed-by: David Schulz Reviewed-by: Reviewed-by: Qt CI Bot --- src/plugins/bookmarks/bookmarkfilter.cpp | 4 ++-- src/plugins/coreplugin/actionsfilter.cpp | 14 ++++++++++---- .../coreplugin/locator/commandlocator.cpp | 3 ++- .../coreplugin/locator/externaltoolsfilter.cpp | 5 +++-- src/plugins/coreplugin/locator/ilocatorfilter.h | 9 ++------- .../coreplugin/locator/javascriptfilter.cpp | 16 ++++++++++++---- .../coreplugin/locator/locatorfiltersfilter.cpp | 7 +++---- .../cppeditor/cppcurrentdocumentfilter.cpp | 3 ++- src/plugins/cppeditor/cpplocatorfilter.cpp | 9 ++++++--- src/plugins/help/helpindexfilter.cpp | 3 ++- src/plugins/macros/macrolocatorfilter.cpp | 3 ++- src/plugins/texteditor/linenumberfilter.cpp | 4 +++- 12 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/plugins/bookmarks/bookmarkfilter.cpp b/src/plugins/bookmarks/bookmarkfilter.cpp index d659885a548..2fad930d37e 100644 --- a/src/plugins/bookmarks/bookmarkfilter.cpp +++ b/src/plugins/bookmarks/bookmarkfilter.cpp @@ -59,8 +59,8 @@ void BookmarkFilter::prepareSearch(const QString &entry) const Bookmark *bookmark = m_manager->bookmarkForIndex(idx); const QString filename = bookmark->filePath().fileName(); LocatorFilterEntry filterEntry(this, - QString("%1:%2").arg(filename).arg(bookmark->lineNumber()), - QVariant::fromValue(idx)); + QString("%1:%2").arg(filename).arg(bookmark->lineNumber())); + filterEntry.internalData = QVariant::fromValue(idx); if (!bookmark->note().isEmpty()) filterEntry.extraInfo = bookmark->note(); else if (!bookmark->lineText().isEmpty()) diff --git a/src/plugins/coreplugin/actionsfilter.cpp b/src/plugins/coreplugin/actionsfilter.cpp index 00c1be83e52..9aaf5a1db74 100644 --- a/src/plugins/coreplugin/actionsfilter.cpp +++ b/src/plugins/coreplugin/actionsfilter.cpp @@ -215,7 +215,9 @@ void ActionsFilter::collectEntriesForAction(QAction *action, } } else if (!text.isEmpty()) { const ActionFilterEntryData data{action, {}}; - LocatorFilterEntry filterEntry(this, text, QVariant::fromValue(data), action->icon()); + LocatorFilterEntry filterEntry(this, text); + filterEntry.internalData = QVariant::fromValue(data); + filterEntry.displayIcon = action->icon(); filterEntry.extraInfo = path.join(" > "); updateEntry(action, filterEntry); } @@ -241,8 +243,10 @@ void ActionsFilter::collectEntriesForCommands() const QString identifier = command->id().toString(); const QStringList path = identifier.split(QLatin1Char('.')); - const ActionFilterEntryData data{action, command->id()}; - LocatorFilterEntry filterEntry(this, text, QVariant::fromValue(data), action->icon()); + const ActionFilterEntryData data{}; + LocatorFilterEntry filterEntry(this, text); + filterEntry.internalData = QVariant::fromValue(ActionFilterEntryData{action, command->id()}); + filterEntry.displayIcon = action->icon(); filterEntry.displayExtra = command->keySequence().toString(QKeySequence::NativeText); if (path.size() >= 2) filterEntry.extraInfo = path.mid(0, path.size() - 1).join(" > "); @@ -260,7 +264,9 @@ void ActionsFilter::collectEntriesForLastTriggered() if (!data.action || !m_enabledActions.contains(data.action)) continue; const QString text = actionText(data.action); - LocatorFilterEntry filterEntry(this, text, QVariant::fromValue(data), data.action->icon()); + LocatorFilterEntry filterEntry(this, text); + filterEntry.internalData = QVariant::fromValue(data); + filterEntry.displayIcon = data.action->icon(); updateEntry(data.action, filterEntry); } } diff --git a/src/plugins/coreplugin/locator/commandlocator.cpp b/src/plugins/coreplugin/locator/commandlocator.cpp index de2989bc17c..387cae9fef6 100644 --- a/src/plugins/coreplugin/locator/commandlocator.cpp +++ b/src/plugins/coreplugin/locator/commandlocator.cpp @@ -77,7 +77,8 @@ QList CommandLocator::matchesFor(QFutureInterface= 0) { - LocatorFilterEntry filterEntry(this, text, QVariant(pair.first)); + LocatorFilterEntry filterEntry(this, text); + filterEntry.internalData = QVariant(pair.first); filterEntry.highlightInfo = {index, int(entry.length())}; if (index == 0) diff --git a/src/plugins/coreplugin/locator/externaltoolsfilter.cpp b/src/plugins/coreplugin/locator/externaltoolsfilter.cpp index 96ce58e20b3..7828cc13724 100644 --- a/src/plugins/coreplugin/locator/externaltoolsfilter.cpp +++ b/src/plugins/coreplugin/locator/externaltoolsfilter.cpp @@ -66,7 +66,8 @@ void ExternalToolsFilter::prepareSearch(const QString &entry) } if (index >= 0) { - LocatorFilterEntry filterEntry(this, tool->displayName(), QVariant::fromValue(tool)); + LocatorFilterEntry filterEntry(this, tool->displayName()); + filterEntry.internalData = QVariant::fromValue(tool); filterEntry.extraInfo = tool->description(); filterEntry.highlightInfo = LocatorFilterEntry::HighlightInfo(index, entry.length(), hDataType); @@ -78,7 +79,7 @@ void ExternalToolsFilter::prepareSearch(const QString &entry) goodEntries.append(filterEntry); } } - LocatorFilterEntry configEntry(this, "Configure External Tool...", {}); + LocatorFilterEntry configEntry(this, "Configure External Tool..."); configEntry.extraInfo = "Opens External Tool settings"; m_results = {}; m_results << bestEntries << betterEntries << goodEntries << configEntry; diff --git a/src/plugins/coreplugin/locator/ilocatorfilter.h b/src/plugins/coreplugin/locator/ilocatorfilter.h index a8e30029d6f..ca7a0dff763 100644 --- a/src/plugins/coreplugin/locator/ilocatorfilter.h +++ b/src/plugins/coreplugin/locator/ilocatorfilter.h @@ -72,14 +72,9 @@ public: LocatorFilterEntry() = default; - LocatorFilterEntry(ILocatorFilter *fromFilter, - const QString &name, - const QVariant &data = {}, - std::optional icon = std::nullopt) + LocatorFilterEntry(ILocatorFilter *fromFilter, const QString &name) : filter(fromFilter) , displayName(name) - , internalData(data) - , displayIcon(icon) {} /* backpointer to creating filter */ @@ -93,7 +88,7 @@ public: /* additional tooltip */ QString toolTip; /* can be used by the filter to save more information about the entry */ - QVariant internalData; + QVariant internalData; // DON'T USE IN NEW CODE, IT'S GOING TO BE REMOVED, SOON... /* icon to display along with the entry */ std::optional displayIcon; /* file path, if the entry is related to a file, is used e.g. for resolving a file icon */ diff --git a/src/plugins/coreplugin/locator/javascriptfilter.cpp b/src/plugins/coreplugin/locator/javascriptfilter.cpp index b97b054051f..37803abbaaa 100644 --- a/src/plugins/coreplugin/locator/javascriptfilter.cpp +++ b/src/plugins/coreplugin/locator/javascriptfilter.cpp @@ -52,17 +52,25 @@ QList JavaScriptFilter::matchesFor( QList entries; if (entry.trimmed().isEmpty()) { - entries.append({this, Tr::tr("Reset Engine"), QVariant::fromValue(EngineAction::Reset)}); + LocatorFilterEntry entry{this, Tr::tr("Reset Engine")}; + entry.internalData = QVariant::fromValue(EngineAction::Reset); + entries.append(entry); } else { const QString result = m_engine->evaluate(entry).toString(); if (m_aborted) { const QString message = entry + " = " + Tr::tr("Engine aborted after timeout."); - entries.append({this, message, QVariant::fromValue(EngineAction::Abort)}); + LocatorFilterEntry entry(this, message); + entry.internalData = QVariant::fromValue(EngineAction::Abort); + entries.append(entry); } else { const QString expression = entry + " = " + result; entries.append({this, expression}); - entries.append({this, Tr::tr("Copy to clipboard: %1").arg(result), result}); - entries.append({this, Tr::tr("Copy to clipboard: %1").arg(expression), expression}); + LocatorFilterEntry resultEntry(this, Tr::tr("Copy to clipboard: %1").arg(result)); + resultEntry.internalData = result; + entries.append(resultEntry); + LocatorFilterEntry expressionEntry(this, Tr::tr("Copy to clipboard: %1").arg(expression)); + resultEntry.internalData = expression; + entries.append(expressionEntry); } } diff --git a/src/plugins/coreplugin/locator/locatorfiltersfilter.cpp b/src/plugins/coreplugin/locator/locatorfiltersfilter.cpp index 800c9171dbc..b8cd9ef3007 100644 --- a/src/plugins/coreplugin/locator/locatorfiltersfilter.cpp +++ b/src/plugins/coreplugin/locator/locatorfiltersfilter.cpp @@ -60,10 +60,9 @@ QList LocatorFiltersFilter::matchesFor(QFutureInterface CppCurrentDocumentFilter::matchesFor( } } - LocatorFilterEntry filterEntry(this, name, {}, info->icon()); + LocatorFilterEntry filterEntry(this, name); + filterEntry.displayIcon = info->icon(); filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()}; filterEntry.extraInfo = extraInfo; if (match.hasMatch()) { diff --git a/src/plugins/cppeditor/cpplocatorfilter.cpp b/src/plugins/cppeditor/cpplocatorfilter.cpp index c74fc73d505..541acb05154 100644 --- a/src/plugins/cppeditor/cpplocatorfilter.cpp +++ b/src/plugins/cppeditor/cpplocatorfilter.cpp @@ -25,7 +25,8 @@ CppLocatorFilter::CppLocatorFilter() LocatorFilterEntry CppLocatorFilter::filterEntryFromIndexItem(IndexItem::Ptr info) { - LocatorFilterEntry filterEntry(this, info->scopedSymbolName(), {}, info->icon()); + LocatorFilterEntry filterEntry(this, info->scopedSymbolName()); + filterEntry.displayIcon = info->icon(); filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()}; if (info->type() == IndexItem::Class || info->type() == IndexItem::Enum) filterEntry.extraInfo = info->shortNativeFilePath(); @@ -120,7 +121,8 @@ CppClassesFilter::CppClassesFilter() LocatorFilterEntry CppClassesFilter::filterEntryFromIndexItem(IndexItem::Ptr info) { - LocatorFilterEntry filterEntry(this, info->symbolName(), {}, info->icon()); + LocatorFilterEntry filterEntry(this, info->symbolName()); + filterEntry.displayIcon = info->icon(); filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()}; filterEntry.extraInfo = info->symbolScope().isEmpty() ? info->shortNativeFilePath() @@ -148,7 +150,8 @@ LocatorFilterEntry CppFunctionsFilter::filterEntryFromIndexItem(IndexItem::Ptr i extraInfo.append(" (" + info->filePath().fileName() + ')'); } - LocatorFilterEntry filterEntry(this, name + info->symbolType(), {}, info->icon()); + LocatorFilterEntry filterEntry(this, name + info->symbolType()); + filterEntry.displayIcon = info->icon(); filterEntry.linkForEditor = {info->filePath(), info->line(), info->column()}; filterEntry.extraInfo = extraInfo; diff --git a/src/plugins/help/helpindexfilter.cpp b/src/plugins/help/helpindexfilter.cpp index 63578ef944d..9c2c7ada44c 100644 --- a/src/plugins/help/helpindexfilter.cpp +++ b/src/plugins/help/helpindexfilter.cpp @@ -78,7 +78,8 @@ QList HelpIndexFilter::matchesFor(QFutureInterface entries; for (const QString &keyword : std::as_const(m_lastIndicesCache)) { const int index = keyword.indexOf(entry, 0, cs); - LocatorFilterEntry filterEntry(this, keyword, {}, m_icon); + LocatorFilterEntry filterEntry(this, keyword); + filterEntry.displayIcon = m_icon; filterEntry.highlightInfo = {index, int(entry.length())}; entries.append(filterEntry); } diff --git a/src/plugins/macros/macrolocatorfilter.cpp b/src/plugins/macros/macrolocatorfilter.cpp index 43a9a489459..5749b187eee 100644 --- a/src/plugins/macros/macrolocatorfilter.cpp +++ b/src/plugins/macros/macrolocatorfilter.cpp @@ -50,7 +50,8 @@ QList MacroLocatorFilter::matchesFor(QFutureInterface< } if (index >= 0) { - Core::LocatorFilterEntry filterEntry(this, displayName, {}, m_icon); + Core::LocatorFilterEntry filterEntry(this, displayName); + filterEntry.displayIcon = m_icon; filterEntry.extraInfo = description; filterEntry.highlightInfo = Core::LocatorFilterEntry::HighlightInfo(index, entry.length(), hDataType); diff --git a/src/plugins/texteditor/linenumberfilter.cpp b/src/plugins/texteditor/linenumberfilter.cpp index 78a65c84d2f..83f0adaf925 100644 --- a/src/plugins/texteditor/linenumberfilter.cpp +++ b/src/plugins/texteditor/linenumberfilter.cpp @@ -64,7 +64,9 @@ QList LineNumberFilter::matchesFor(QFutureInterface