LocatorFilterEntry: Add linkForEditor

Make it of std::optional<Utils::Link> type.

Use it for:
1. Removing duplicates
2. Showing link inside editor

Before, QVariant internalData was used in above cases.

Rationale:
1. Removing duplicates between results from different
   ILocatorFilter makes only sense if accept() overload
   for these filter types are the same (i.e. the implementation
   is repeated). Otherwise, we are loosing some functionality
   when some result is removed. Taking into account that
   original intention was to eliminate duplicates for filters
   where we do BaseFileFilter::openEditorAt() inside accept(),
   we store linkForEditor in 3 cases (instead of storing
   internalData):
 - BaseFileFilter
 - FileSystemFilter (only existing file case)
 - OpenDocumentsFilter

   We don't save a linkForEditor where we stored QString
   inside internalData in the following cases:
 - JavaScriptFilter
 - FileSystemFilter (non existing file case)

   Before, in above cases, we could have filtered out
   some results unintentionally. This patch should fix it.

   Now, we remove duplicates only for enties that have value
   for linkForEditor.

2. Use directly linkForEditor, if exists, for opening editor.

This patch aims to decrease the number of different
responsibilities of internalData by 1.

Make 3rd arg of LocatorFilterEntry c'tor default.

Change-Id: I144c58661d7456bb5991f8077dc103272dfab194
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: David Schulz <david.schulz@qt.io>
This commit is contained in:
Jarek Kobus
2023-02-09 18:07:41 +01:00
parent 4b9054c463
commit 5d19894f5b
13 changed files with 57 additions and 69 deletions

View File

@@ -117,8 +117,7 @@ QList<LocatorFilterEntry> BaseFileFilter::matchesFor(QFutureInterface<LocatorFil
QList<LocatorFilterEntry> entries[int(MatchLevel::Count)];
// If search string contains spaces, treat them as wildcard '*' and search in full path
const QString entry = QDir::fromNativeSeparators(origEntry).replace(' ', '*');
QString postfix;
Link link = Link::fromString(entry, true, &postfix);
const Link link = Link::fromString(entry, true);
const QRegularExpression regexp = createRegExp(link.targetFilePath.toString());
if (!regexp.isValid()) {
@@ -156,10 +155,10 @@ QList<LocatorFilterEntry> BaseFileFilter::matchesFor(QFutureInterface<LocatorFil
QRegularExpressionMatch match = regexp.match(matchText);
if (match.hasMatch()) {
LocatorFilterEntry filterEntry(this, path.fileName(), QString(path.toString() + postfix));
LocatorFilterEntry filterEntry(this, path.fileName());
filterEntry.filePath = path;
filterEntry.extraInfo = path.shortNativePath();
filterEntry.linkForEditor = Link(path, link.targetLine, link.targetColumn);
const MatchLevel matchLevel = matchLevelFor(match, matchText);
if (hasPathSeparator) {
match = regexp.match(filterEntry.extraInfo);
@@ -187,7 +186,7 @@ QList<LocatorFilterEntry> BaseFileFilter::matchesFor(QFutureInterface<LocatorFil
for (auto &entry : entries) {
if (entry.size() < 1000)
Utils::sort(entry, Core::LocatorFilterEntry::compareLexigraphically);
Utils::sort(entry, LocatorFilterEntry::compareLexigraphically);
}
return std::accumulate(std::begin(entries), std::end(entries), QList<LocatorFilterEntry>());
@@ -205,21 +204,13 @@ void BaseFileFilter::accept(const LocatorFilterEntry &selection,
openEditorAt(selection);
}
void BaseFileFilter::openEditorAt(const LocatorFilterEntry& selection)
void BaseFileFilter::openEditorAt(const LocatorFilterEntry &entry)
{
const FilePath locatorText = FilePath::fromVariant(selection.internalData);
const int postfixLength = locatorText.fileName().length() - selection.filePath.fileName().length();
if (postfixLength > 0) {
const QString postfix = selection.internalData.toString().right(postfixLength);
int postfixPos = -1;
const LineColumn lineColumn = LineColumn::extractFromFileName(postfix, postfixPos);
if (postfixPos >= 0) {
const Link link(selection.filePath, lineColumn.line, lineColumn.column);
EditorManager::openEditorAt(link, {}, Core::EditorManager::AllowExternalEditor);
return;
}
if (entry.linkForEditor) {
EditorManager::openEditorAt(*entry.linkForEditor, {}, EditorManager::AllowExternalEditor);
return;
}
EditorManager::openEditor(selection.filePath, {}, Core::EditorManager::AllowExternalEditor);
EditorManager::openEditor(entry.filePath, {}, EditorManager::AllowExternalEditor);
}
/*!

View File

@@ -48,7 +48,7 @@ public:
const QString &entry) override;
void accept(const LocatorFilterEntry &selection,
QString *newText, int *selectionStart, int *selectionLength) const override;
static void openEditorAt(const LocatorFilterEntry &selection);
static void openEditorAt(const LocatorFilterEntry &entry);
protected:
void setFileIterator(Iterator *iterator);

View File

@@ -44,7 +44,7 @@ QList<LocatorFilterEntry> ExecuteFilter::matchesFor(QFutureInterface<LocatorFilt
{
QList<LocatorFilterEntry> value;
if (!entry.isEmpty()) // avoid empty entry
value.append(LocatorFilterEntry(this, entry, QVariant()));
value.append(LocatorFilterEntry(this, entry));
QList<LocatorFilterEntry> others;
const Qt::CaseSensitivity entryCaseSensitivity = caseSensitivity(entry);
for (const QString &cmd : std::as_const(m_commandHistory)) {
@@ -52,7 +52,7 @@ QList<LocatorFilterEntry> ExecuteFilter::matchesFor(QFutureInterface<LocatorFilt
break;
if (cmd == entry) // avoid repeated entry
continue;
LocatorFilterEntry filterEntry(this, cmd, QVariant());
LocatorFilterEntry filterEntry(this, cmd);
const int index = cmd.indexOf(entry, 0, entryCaseSensitivity);
if (index >= 0) {
filterEntry.highlightInfo = {index, int(entry.length())};

View File

@@ -109,7 +109,7 @@ QList<LocatorFilterEntry> FileSystemFilter::matchesFor(QFutureInterface<LocatorF
if (match.hasMatch()) {
const MatchLevel level = matchLevelFor(match, dir);
const QString fullPath = dirInfo.filePath(dir);
LocatorFilterEntry filterEntry(this, dir, QVariant());
LocatorFilterEntry filterEntry(this, dir);
filterEntry.filePath = FilePath::fromString(fullPath);
filterEntry.highlightInfo = highlightInfo(match);
@@ -117,8 +117,7 @@ QList<LocatorFilterEntry> FileSystemFilter::matchesFor(QFutureInterface<LocatorF
}
}
// file names can match with +linenumber or :linenumber
QString postfix;
Link link = Link::fromString(entryFileName, true, &postfix);
const Link link = Link::fromString(entryFileName, true);
regExp = createRegExp(link.targetFilePath.toString(), caseSensitivity_);
if (!regExp.isValid())
return {};
@@ -130,10 +129,11 @@ QList<LocatorFilterEntry> FileSystemFilter::matchesFor(QFutureInterface<LocatorF
if (match.hasMatch()) {
const MatchLevel level = matchLevelFor(match, file);
const QString fullPath = dirInfo.filePath(file);
LocatorFilterEntry filterEntry(this, file, QString(fullPath + postfix));
LocatorFilterEntry filterEntry(this, file);
filterEntry.filePath = FilePath::fromString(fullPath);
filterEntry.highlightInfo = highlightInfo(match);
filterEntry.linkForEditor = Link(filterEntry.filePath, link.targetLine,
link.targetColumn);
entries[int(level)].append(filterEntry);
}
}
@@ -142,9 +142,7 @@ QList<LocatorFilterEntry> FileSystemFilter::matchesFor(QFutureInterface<LocatorF
const QString fullFilePath = dirInfo.filePath(entryFileName);
const bool containsWildcard = expandedEntry.contains('?') || expandedEntry.contains('*');
if (!containsWildcard && !QFileInfo::exists(fullFilePath) && dirInfo.exists()) {
LocatorFilterEntry createAndOpen(this,
Tr::tr("Create and Open \"%1\"").arg(expandedEntry),
fullFilePath);
LocatorFilterEntry createAndOpen(this, Tr::tr("Create and Open \"%1\"").arg(expandedEntry));
createAndOpen.filePath = FilePath::fromString(fullFilePath);
createAndOpen.extraInfo = FilePath::fromString(dirInfo.absolutePath()).shortNativePath();
entries[int(MatchLevel::Normal)].append(createAndOpen);
@@ -170,13 +168,12 @@ void FileSystemFilter::accept(const LocatorFilterEntry &selection,
} else {
// Don't block locator filter execution with dialog
QMetaObject::invokeMethod(EditorManager::instance(), [selection] {
const FilePath targetFile = FilePath::fromVariant(selection.internalData);
if (!selection.filePath.exists()) {
if (CheckableMessageBox::shouldAskAgain(ICore::settings(), kAlwaysCreate)) {
CheckableMessageBox messageBox(ICore::dialogParent());
messageBox.setWindowTitle(Tr::tr("Create File"));
messageBox.setIcon(QMessageBox::Question);
messageBox.setText(Tr::tr("Create \"%1\"?").arg(targetFile.shortNativePath()));
messageBox.setText(Tr::tr("Create \"%1\"?").arg(selection.filePath.shortNativePath()));
messageBox.setCheckBoxVisible(true);
messageBox.setCheckBoxText(Tr::tr("Always create"));
messageBox.setChecked(false);
@@ -190,10 +187,10 @@ void FileSystemFilter::accept(const LocatorFilterEntry &selection,
if (messageBox.isChecked())
CheckableMessageBox::doNotAskAgain(ICore::settings(), kAlwaysCreate);
}
QFile file(targetFile.toString());
QFile file(selection.filePath.toString());
file.open(QFile::WriteOnly);
file.close();
VcsManager::promptToAdd(targetFile.absolutePath(), {targetFile});
VcsManager::promptToAdd(selection.filePath.absolutePath(), {selection.filePath});
}
BaseFileFilter::openEditorAt(selection);
}, Qt::QueuedConnection);

View File

@@ -7,6 +7,7 @@
#include <utils/filepath.h>
#include <utils/id.h>
#include <utils/link.h>
#include <QFutureInterface>
#include <QIcon>
@@ -67,7 +68,7 @@ struct LocatorFilterEntry
LocatorFilterEntry(ILocatorFilter *fromFilter,
const QString &name,
const QVariant &data,
const QVariant &data = {},
std::optional<QIcon> icon = std::nullopt)
: filter(fromFilter)
, displayName(name)
@@ -93,7 +94,8 @@ struct LocatorFilterEntry
Utils::FilePath filePath;
/* highlighting support */
HighlightInfo highlightInfo;
// Should be used only when accept() calls BaseFileFilter::openEditorAt()
std::optional<Utils::Link> linkForEditor;
static bool compareLexigraphically(const Core::LocatorFilterEntry &lhs,
const Core::LocatorFilterEntry &rhs)
{

View File

@@ -60,7 +60,7 @@ QList<LocatorFilterEntry> JavaScriptFilter::matchesFor(
entries.append({this, message, QVariant::fromValue(EngineAction::Abort)});
} else {
const QString expression = entry + " = " + result;
entries.append({this, expression, QVariant()});
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});
}

View File

@@ -3,34 +3,33 @@
#include "locatorsearchutils.h"
#include <QSet>
#include <QString>
#include <QVariant>
#include <utils/link.h>
#include <unordered_set>
void Core::Internal::runSearch(QFutureInterface<Core::LocatorFilterEntry> &future,
const QList<ILocatorFilter *> &filters, const QString &searchText)
{
QSet<QString> alreadyAdded;
std::unordered_set<Utils::FilePath> addedCache;
const bool checkDuplicates = (filters.size() > 1);
const auto duplicatesRemoved = [&](const QList<LocatorFilterEntry> &entries) {
if (!checkDuplicates)
return entries;
QList<LocatorFilterEntry> results;
results.reserve(entries.size());
for (const LocatorFilterEntry &entry : entries) {
const auto &link = entry.linkForEditor;
if (!link || addedCache.emplace(link->targetFilePath).second)
results.append(entry);
}
return results;
};
for (ILocatorFilter *filter : filters) {
if (future.isCanceled())
break;
const QList<LocatorFilterEntry> filterResults = filter->matchesFor(future, searchText);
QVector<LocatorFilterEntry> uniqueFilterResults;
uniqueFilterResults.reserve(filterResults.size());
for (const LocatorFilterEntry &entry : filterResults) {
if (checkDuplicates) {
const QString stringData = entry.internalData.toString();
if (!stringData.isEmpty()) {
if (alreadyAdded.contains(stringData))
continue;
alreadyAdded.insert(stringData);
}
}
uniqueFilterResults.append(entry);
}
if (!uniqueFilterResults.isEmpty())
future.reportResults(uniqueFilterResults);
const auto results = duplicatesRemoved(filter->matchesFor(future, searchText));
if (!results.isEmpty())
future.reportResults(results);
}
}

View File

@@ -9,8 +9,8 @@ namespace Core {
namespace Internal {
void CORE_EXPORT runSearch(QFutureInterface<LocatorFilterEntry> &future,
const QList<ILocatorFilter *> &filters,
const QString &searchText);
const QList<ILocatorFilter *> &filters,
const QString &searchText);
} // namespace Internal
} // namespace Core

View File

@@ -39,8 +39,7 @@ QList<LocatorFilterEntry> OpenDocumentsFilter::matchesFor(QFutureInterface<Locat
{
QList<LocatorFilterEntry> goodEntries;
QList<LocatorFilterEntry> betterEntries;
QString postfix;
Link link = Link::fromString(entry, true, &postfix);
const Link link = Link::fromString(entry, true);
const QRegularExpression regexp = createRegExp(link.targetFilePath.toString());
if (!regexp.isValid())
@@ -56,10 +55,12 @@ QList<LocatorFilterEntry> OpenDocumentsFilter::matchesFor(QFutureInterface<Locat
QString displayName = editorEntry.displayName;
const QRegularExpressionMatch match = regexp.match(displayName);
if (match.hasMatch()) {
LocatorFilterEntry filterEntry(this, displayName, QString(fileName + postfix));
LocatorFilterEntry filterEntry(this, displayName);
filterEntry.filePath = FilePath::fromString(fileName);
filterEntry.extraInfo = filterEntry.filePath.shortNativePath();
filterEntry.highlightInfo = highlightInfo(match);
filterEntry.linkForEditor = Link(filterEntry.filePath, link.targetLine,
link.targetColumn);
if (match.capturedStart() == 0)
betterEntries.append(filterEntry);
else

View File

@@ -176,7 +176,7 @@ QList<Core::LocatorFilterEntry> UrlLocatorFilter::matchesFor(
if (future.isCanceled())
break;
const QString name = url.arg(entry);
Core::LocatorFilterEntry filterEntry(this, name, QVariant());
Core::LocatorFilterEntry filterEntry(this, name);
filterEntry.highlightInfo = {int(name.lastIndexOf(entry)), int(entry.length())};
entries.append(filterEntry);
}

View File

@@ -86,7 +86,7 @@ QList<LocatorFilterEntry> HelpIndexFilter::matchesFor(QFutureInterface<LocatorFi
QList<LocatorFilterEntry> entries;
for (const QString &keyword : std::as_const(m_lastIndicesCache)) {
const int index = keyword.indexOf(entry, 0, cs);
LocatorFilterEntry filterEntry(this, keyword, QVariant(), m_icon);
LocatorFilterEntry filterEntry(this, keyword, {}, m_icon);
filterEntry.highlightInfo = {index, int(entry.length())};
entries.append(filterEntry);
}

View File

@@ -50,7 +50,7 @@ QList<Core::LocatorFilterEntry> MacroLocatorFilter::matchesFor(QFutureInterface<
}
if (index >= 0) {
Core::LocatorFilterEntry filterEntry(this, displayName, QVariant(), m_icon);
Core::LocatorFilterEntry filterEntry(this, displayName, {}, m_icon);
filterEntry.extraInfo = description;
filterEntry.highlightInfo = Core::LocatorFilterEntry::HighlightInfo(index, entry.length(), hDataType);

View File

@@ -4362,10 +4362,8 @@ void RunConfigurationLocatorFilter::prepareSearch(const QString &entry)
if (!target)
return;
for (auto rc : target->runConfigurations()) {
if (rc->displayName().contains(entry, Qt::CaseInsensitive)) {
Core::LocatorFilterEntry filterEntry(this, rc->displayName(), {});
m_result.append(filterEntry);
}
if (rc->displayName().contains(entry, Qt::CaseInsensitive))
m_result.append(LocatorFilterEntry(this, rc->displayName()));
}
}