Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix some locator filters show results when no string is entered
and filter prefix is not used
  • Loading branch information
nyalldawson committed Apr 6, 2018
1 parent ccb72eb commit ebab649
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 10 deletions.
2 changes: 2 additions & 0 deletions python/core/locator/qgslocatorcontext.sip.in
Expand Up @@ -31,6 +31,8 @@ Constructor for QgsLocatorContext.

QgsCoordinateReferenceSystem targetExtentCrs;

bool usingPrefix;

};


Expand Down
3 changes: 2 additions & 1 deletion python/plugins/processing/gui/AlgorithmLocatorFilter.py
Expand Up @@ -65,7 +65,8 @@ def fetchResults(self, string, context, feedback):
if a.flags() & QgsProcessingAlgorithm.FlagHideFromToolbox:
continue

if QgsLocatorFilter.stringMatches(a.displayName(), string) or [t for t in a.tags() if QgsLocatorFilter.stringMatches(t, string)]:
if QgsLocatorFilter.stringMatches(a.displayName(), string) or [t for t in a.tags() if QgsLocatorFilter.stringMatches(t, string)] or \
(context.usingPrefix and not string):
result = QgsLocatorResult()
result.filter = self
result.displayString = a.displayName()
Expand Down
12 changes: 6 additions & 6 deletions src/app/locator/qgsinbuiltlocatorfilters.cpp
Expand Up @@ -37,13 +37,13 @@ QgsLayerTreeLocatorFilter *QgsLayerTreeLocatorFilter::clone() const
return new QgsLayerTreeLocatorFilter();
}

void QgsLayerTreeLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback * )
void QgsLayerTreeLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback * )
{
QgsLayerTree *tree = QgsProject::instance()->layerTreeRoot();
const QList<QgsLayerTreeLayer *> layers = tree->findLayers();
for ( QgsLayerTreeLayer *layer : layers )
{
if ( layer->layer() && stringMatches( layer->layer()->name(), string ) )
if ( layer->layer() && ( stringMatches( layer->layer()->name(), string ) || ( context.usingPrefix && string.isEmpty() ) ) )
{
QgsLocatorResult result;
result.displayString = layer->layer()->name();
Expand Down Expand Up @@ -75,12 +75,12 @@ QgsLayoutLocatorFilter *QgsLayoutLocatorFilter::clone() const
return new QgsLayoutLocatorFilter();
}

void QgsLayoutLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback * )
void QgsLayoutLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback * )
{
const QList< QgsMasterLayoutInterface * > layouts = QgsProject::instance()->layoutManager()->layouts();
for ( QgsMasterLayoutInterface *layout : layouts )
{
if ( layout && stringMatches( layout->name(), string ) )
if ( layout && ( stringMatches( layout->name(), string ) || ( context.usingPrefix && string.isEmpty() ) ) )
{
QgsLocatorResult result;
result.displayString = layout->name();
Expand Down Expand Up @@ -332,7 +332,7 @@ QgsBookmarkLocatorFilter *QgsBookmarkLocatorFilter::clone() const
return new QgsBookmarkLocatorFilter();
}

void QgsBookmarkLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &, QgsFeedback *feedback )
void QgsBookmarkLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback )
{
QMap<QString, QModelIndex> bookmarkMap = QgisApp::instance()->getBookmarkIndexMap();

Expand All @@ -346,7 +346,7 @@ void QgsBookmarkLocatorFilter::fetchResults( const QString &string, const QgsLoc

QString name = i.key();

if ( stringMatches( name, string ) )
if ( stringMatches( name, string ) || ( context.usingPrefix && string.isEmpty() ) )
{
QModelIndex index = i.value();
QgsLocatorResult result;
Expand Down
4 changes: 3 additions & 1 deletion src/core/locator/qgslocator.cpp
Expand Up @@ -82,8 +82,9 @@ void QgsLocator::registerFilter( QgsLocatorFilter *filter )
filter->setUseWithoutPrefix( byDefault );
}

void QgsLocator::fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback )
void QgsLocator::fetchResults( const QString &string, const QgsLocatorContext &c, QgsFeedback *feedback )
{
QgsLocatorContext context( c );
// ideally this should not be required, as well behaved callers
// will NOT fire up a new fetchResults call while an existing one is
// operating/waiting to be canceled...
Expand Down Expand Up @@ -112,6 +113,7 @@ void QgsLocator::fetchResults( const QString &string, const QgsLocatorContext &c
activeFilters << mPrefixedFilters.value( prefix );
searchString = searchString.mid( prefix.length() + 1 );
}
context.usingPrefix = !activeFilters.empty();
}
if ( activeFilters.isEmpty() )
{
Expand Down
5 changes: 5 additions & 0 deletions src/core/locator/qgslocatorcontext.h
Expand Up @@ -51,6 +51,11 @@ class CORE_EXPORT QgsLocatorContext
*/
QgsCoordinateReferenceSystem targetExtentCrs;

/**
* Will be true if search is being conducted using a filter prefix.
*/
bool usingPrefix = false;

};

#endif // QGSLOCATORCONTEXT_H
Expand Down
2 changes: 1 addition & 1 deletion src/core/locator/qgslocatorfilter.cpp
Expand Up @@ -34,7 +34,7 @@ QgsLocatorFilter::Flags QgsLocatorFilter::flags() const

bool QgsLocatorFilter::stringMatches( const QString &candidate, const QString &search )
{
return candidate.contains( search, Qt::CaseInsensitive );
return !search.isEmpty() && candidate.contains( search, Qt::CaseInsensitive );
}

bool QgsLocatorFilter::enabled() const
Expand Down
77 changes: 76 additions & 1 deletion tests/src/app/testqgsapplocatorfilters.cpp
Expand Up @@ -16,6 +16,8 @@
#include "qgisapp.h"
#include "qgslocatorfilter.h"
#include "qgslocator.h"
#include "qgsprintlayout.h"
#include "qgslayoutmanager.h"
#include "locator/qgsinbuiltlocatorfilters.h"
#include <QSignalSpy>
#include <QClipboard>
Expand All @@ -32,6 +34,8 @@ class TestQgsAppLocatorFilters : public QObject
void initTestCase();// will be called before the first testfunction is executed.
void cleanupTestCase();// will be called after the last testfunction was executed.
void testCalculator();
void testLayers();
void testLayouts();

private:
QgisApp *mQgisApp = nullptr;
Expand Down Expand Up @@ -70,6 +74,77 @@ void TestQgsAppLocatorFilters::testCalculator()
// invalid expression
results = gatherResults( &filter, QStringLiteral( "1+" ), QgsLocatorContext() );
QVERIFY( results.empty() );
}

void TestQgsAppLocatorFilters::testLayers()
{
QgsVectorLayer *l1 = new QgsVectorLayer( QStringLiteral( "Point" ), QStringLiteral( "aaaaa" ), QStringLiteral( "memory" ) );
QgsVectorLayer *l2 = new QgsVectorLayer( QStringLiteral( "Point" ), QStringLiteral( "abc" ), QStringLiteral( "memory" ) );
QgsVectorLayer *l3 = new QgsVectorLayer( QStringLiteral( "Point" ), QStringLiteral( "ccccc" ), QStringLiteral( "memory" ) );
QgsProject::instance()->addMapLayers( QList< QgsMapLayer *>() << l1 << l2 << l3 );

QgsLayerTreeLocatorFilter filter;

QList< QgsLocatorResult > results = gatherResults( &filter, QStringLiteral( "xxxxx" ), QgsLocatorContext() );
QCOMPARE( results.count(), 0 );

results = gatherResults( &filter, QStringLiteral( "aa" ), QgsLocatorContext() );
QCOMPARE( results.count(), 1 );
QCOMPARE( results.at( 0 ).userData.toString(), l1->id() );

results = gatherResults( &filter, QStringLiteral( "A" ), QgsLocatorContext() );
QCOMPARE( results.count(), 2 );
QCOMPARE( results.at( 0 ).userData.toString(), l1->id() );
QCOMPARE( results.at( 1 ).userData.toString(), l2->id() );

results = gatherResults( &filter, QString(), QgsLocatorContext() );
QCOMPARE( results.count(), 0 );

QgsLocatorContext context;
context.usingPrefix = true;
results = gatherResults( &filter, QString(), context );
QCOMPARE( results.count(), 3 );
QCOMPARE( results.at( 0 ).userData.toString(), l1->id() );
QCOMPARE( results.at( 1 ).userData.toString(), l2->id() );
QCOMPARE( results.at( 2 ).userData.toString(), l3->id() );
}

void TestQgsAppLocatorFilters::testLayouts()
{
QgsPrintLayout *pl1 = new QgsPrintLayout( QgsProject::instance() );
pl1->setName( QStringLiteral( "aaaaaa" ) );
QgsProject::instance()->layoutManager()->addLayout( pl1 );
QgsPrintLayout *pl2 = new QgsPrintLayout( QgsProject::instance() );
pl2->setName( QStringLiteral( "abc" ) );
QgsProject::instance()->layoutManager()->addLayout( pl2 );
QgsPrintLayout *pl3 = new QgsPrintLayout( QgsProject::instance() );
pl3->setName( QStringLiteral( "ccccc" ) );
QgsProject::instance()->layoutManager()->addLayout( pl3 );

QgsLayoutLocatorFilter filter;

QList< QgsLocatorResult > results = gatherResults( &filter, QStringLiteral( "xxxxx" ), QgsLocatorContext() );
QCOMPARE( results.count(), 0 );

results = gatherResults( &filter, QStringLiteral( "aa" ), QgsLocatorContext() );
QCOMPARE( results.count(), 1 );
QCOMPARE( results.at( 0 ).userData.toString(), pl1->name() );

results = gatherResults( &filter, QStringLiteral( "A" ), QgsLocatorContext() );
QCOMPARE( results.count(), 2 );
QCOMPARE( results.at( 0 ).userData.toString(), pl1->name() );
QCOMPARE( results.at( 1 ).userData.toString(), pl2->name() );

results = gatherResults( &filter, QString(), QgsLocatorContext() );
QCOMPARE( results.count(), 0 );

QgsLocatorContext context;
context.usingPrefix = true;
results = gatherResults( &filter, QString(), context );
QCOMPARE( results.count(), 3 );
QCOMPARE( results.at( 0 ).userData.toString(), pl1->name() );
QCOMPARE( results.at( 1 ).userData.toString(), pl2->name() );
QCOMPARE( results.at( 2 ).userData.toString(), pl3->name() );

}

Expand All @@ -82,7 +157,7 @@ QList<QgsLocatorResult> TestQgsAppLocatorFilters::gatherResults( QgsLocatorFilte
QList< QgsLocatorResult > results;
for ( int i = 0; i < spy.count(); ++ i )
{
QVariant v = spy.at( i ).at( i );
QVariant v = spy.at( i ).at( 0 );
QgsLocatorResult result = v.value<QgsLocatorResult>();
results.append( result );
}
Expand Down
6 changes: 6 additions & 0 deletions tests/src/python/test_qgslocator.py
Expand Up @@ -260,6 +260,12 @@ def testAutoModel(self):
self.assertEqual(m.data(m.index(2, 0)), 'a1')
self.assertEqual(m.data(m.index(3, 0)), 'a2')

def testStringMatches(self):
self.assertFalse(QgsLocatorFilter.stringMatches('xxx', 'yyyy'))
self.assertTrue(QgsLocatorFilter.stringMatches('axxxy', 'xxx'))
self.assertTrue(QgsLocatorFilter.stringMatches('aXXXXy', 'xxx'))
self.assertFalse(QgsLocatorFilter.stringMatches('aXXXXy', ''))


if __name__ == '__main__':
unittest.main()

0 comments on commit ebab649

Please sign in to comment.