Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nominatim Geocoder Locator filter - add countryCodes configuration option #60719

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Nominatim Geocoder Locator filter - add countryCodes configuration option #60719

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e5da8eb
Add country codes config to Nominatim Geocoder Locator filter
uprel Feb 21, 2025
af02358
Remove debug line
uprel Feb 21, 2025
9d6b3d6
Merge branch 'qgis:master' into nom_countrycodes
uprel Feb 21, 2025
138546e
Merge branch 'qgis:master' into nom_countrycodes
uprel Feb 21, 2025
ac17f47
Update src/app/locator/qgsnominatimlocatorfilter.cpp
uprel Feb 25, 2025
61082a8
Update src/app/locator/qgsnominatimlocatorfilter.cpp
uprel Feb 25, 2025
460dcc9
Merge branch 'qgis:master' into nom_countrycodes
uprel Feb 25, 2025
518bd8d
revert qgsabstractgeocoderlocatorfilter.cpp from master
uprel Feb 25, 2025
c03a524
- Fix formLayout to gridLayout
uprel Feb 25, 2025
5ed878c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/app/locator/qgsnominatimlocatorfilter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

#include "qgsnominatimlocatorfilter.h"
#include "moc_qgsnominatimlocatorfilter.cpp"
#include "qgsgeocoder.h"
#include "qgslocatorfilter.h"
#include "qgssettings.h"
#include "qgsmessagebaritem.h"
#include "qgsmessagebar.h"
Expand Down Expand Up @@ -50,3 +52,30 @@ void QgsNominatimLocatorFilter::triggerResult( const QgsLocatorResult &result )
}
QgsGeocoderLocatorFilter::triggerResult( result );
}

void QgsNominatimLocatorFilter::openConfigWidget( QWidget *parent )
{
auto dlg = std::make_unique<QDialog>( parent );
dlg->setWindowTitle( "Nominatim Geocoder Country Codes" );

QFormLayout *formLayout = new QFormLayout;
QLineEdit *countryCodesEdit = new QLineEdit( dlg.get() );

// Load existing settings
QgsSettings settings;
countryCodesEdit->setText( settings.value( "locator_filters/nominatim_geocoder/country_codes", "", QgsSettings::App ).toString() );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use new settings tree API here and in other places?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know, I used the code from other Locator filters as example. Like this

QGIS/src/app/locator/qgsalllayersfeatureslocatorfilter.cpp

Line 219 in f38d190

QgsSettings settings;

I can check this, but not sure what will be better. Is approach I used deprecated?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not deprecated, but IMHO for new code we should use new API. Also with this API new setting will be nicely exposed in the advanced settings editor.


formLayout->addRow( tr( "Two letter Country Codes (comma-separated):" ), countryCodesEdit );
QDialogButtonBox *buttonbBox = new QDialogButtonBox( QDialogButtonBox::Ok | QDialogButtonBox::Cancel, dlg.get() );
formLayout->addRow( buttonbBox );
dlg->setLayout( formLayout );

// Save settings when dialog accepted
connect( buttonbBox, &QDialogButtonBox::accepted, dlg.get(), [&]() {
settings.setValue( "locator_filters/nominatim_geocoder/country_codes", countryCodesEdit->text().trimmed(), QgsSettings::App );
dlg->accept();
} );

connect( buttonbBox, &QDialogButtonBox::rejected, dlg.get(), &QDialog::reject );
dlg->exec();
}
2 changes: 2 additions & 0 deletions src/app/locator/qgsnominatimlocatorfilter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ class APP_EXPORT QgsNominatimLocatorFilter : public QgsGeocoderLocatorFilter
QgsNominatimLocatorFilter( QgsGeocoderInterface *geocoder, QgsMapCanvas *canvas );

void triggerResult( const QgsLocatorResult &result ) override;
bool hasConfigWidget() const override { return true; }
void openConfigWidget( QWidget *parent ) override;
};

#endif // QGSNOMINATIMLOCATORFILTERS_H
18 changes: 18 additions & 0 deletions src/core/geocoding/qgsabstractgeocoderlocatorfilter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
#include "moc_qgsabstractgeocoderlocatorfilter.cpp"
#include "qgsgeocoder.h"
#include "qgsgeocodercontext.h"
#include "qgsnominatimgeocoder.h"
#include "qgssettings.h"

QgsAbstractGeocoderLocatorFilter::QgsAbstractGeocoderLocatorFilter( const QString &name, const QString &displayName, const QString &prefix, QgsGeocoderInterface *geocoder, const QgsRectangle &boundingBox )
: mName( name )
Expand Down Expand Up @@ -45,10 +47,26 @@ QString QgsAbstractGeocoderLocatorFilter::prefix() const

void QgsAbstractGeocoderLocatorFilter::fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback )
{
if ( !mGeocoder )
{
return;
}

QgsGeocoderContext geocodeContext( context.transformContext );
geocodeContext.setAreaOfInterest( QgsGeometry::fromRect( context.targetExtent ) );
geocodeContext.setAreaOfInterestCrs( context.targetExtentCrs );

// Check if we have QgsNominatimLocatorFilter instance
QgsNominatimGeocoder *nominatimGeocoder = dynamic_cast<QgsNominatimGeocoder *>( mGeocoder );
if ( nominatimGeocoder )
{
// Read countryCodes from settings
QgsSettings settings;
QString countryCodes = settings.value( "locator_filters/nominatim_geocoder/country_codes", "", QgsSettings::App ).toString().trimmed();

nominatimGeocoder->setCountryCodes( countryCodes );
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of the dynamic_cast here, it would be cleaner to override fetchResults in QgsNominatimLocatorFilter and set the country code there, and then call this base class QgsAbstractGeocoderLocatorFilter::fetchResults method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but override doesn't work. I commited this override for reference, it doesn't get called. I always get fetchResults from QgsAbstractGeocoderLocatorFilter. Overriding hierarchy in this case is somehow weird. Do you see easy fix that I missed?


const QList< QgsGeocoderResult > results = mGeocoder->geocodeString( string, geocodeContext, feedback );
for ( const QgsGeocoderResult &result : results )
{
Expand Down
Loading