From 47f7612004e9f9077296809a5d8f35e50c7b5be0 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 2 Aug 2021 10:39:06 +1000 Subject: [PATCH] Add flag to babel command generation methods to control whether paths should be quoted --- python/core/auto_additions/qgis.py | 5 ++ .../auto_generated/gps/qgsbabelformat.sip.in | 11 +++- .../gps/qgsbabelgpsdevice.sip.in | 8 +-- python/core/auto_generated/qgis.sip.in | 9 +++ src/core/gps/qgsbabelformat.cpp | 14 +++-- src/core/gps/qgsbabelformat.h | 11 +++- src/core/gps/qgsbabelgpsdevice.cpp | 16 ++--- src/core/gps/qgsbabelgpsdevice.h | 6 +- src/core/qgis.h | 14 +++++ tests/src/python/test_qgsbabelgpsformat.py | 60 +++++++++++++------ 10 files changed, 110 insertions(+), 44 deletions(-) diff --git a/python/core/auto_additions/qgis.py b/python/core/auto_additions/qgis.py index ef631ca33ef..881c94dc0c4 100644 --- a/python/core/auto_additions/qgis.py +++ b/python/core/auto_additions/qgis.py @@ -508,6 +508,11 @@ Qgis.BabelFormatCapability.__doc__ = 'Babel GPS format capabilities.\n\n.. versi # -- Qgis.BabelFormatCapability.baseClass = Qgis # monkey patching scoped based enum +Qgis.BabelCommandFlag.QuoteFilePaths.__doc__ = "File paths should be enclosed in quotations and escaped" +Qgis.BabelCommandFlag.__doc__ = 'Babel command flags, which control how commands and arguments\nare generated for executing GPSBabel processes.\n\n.. versionadded:: 3.22\n\n' + '* ``QuoteFilePaths``: ' + Qgis.BabelCommandFlag.QuoteFilePaths.__doc__ +# -- +Qgis.BabelCommandFlag.baseClass = Qgis +# monkey patching scoped based enum Qgis.GpsFeatureType.Waypoint.__doc__ = "Waypoint" Qgis.GpsFeatureType.Route.__doc__ = "Route" Qgis.GpsFeatureType.Track.__doc__ = "Track" diff --git a/python/core/auto_generated/gps/qgsbabelformat.sip.in b/python/core/auto_generated/gps/qgsbabelformat.sip.in index 35810970146..32d0f0bbbc3 100644 --- a/python/core/auto_generated/gps/qgsbabelformat.sip.in +++ b/python/core/auto_generated/gps/qgsbabelformat.sip.in @@ -37,7 +37,8 @@ Returns the format's capabilities. virtual QStringList importCommand( const QString &babel, Qgis::GpsFeatureType featureType, const QString &input, - const QString &output ) const; + const QString &output, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const; %Docstring Generates a command for importing data into a GPS format using babel. @@ -45,6 +46,7 @@ Generates a command for importing data into a GPS format using babel. :param featureType: type of GPS feature to import :param input: input data path :param output: output path +:param flags: optional flags to control how babel command is generated Returns an empty list if the format does not support imports (see :py:func:`~QgsAbstractBabelFormat.capabilities`). %End @@ -52,7 +54,8 @@ Returns an empty list if the format does not support imports (see :py:func:`~Qgs virtual QStringList exportCommand( const QString &babel, Qgis::GpsFeatureType featureType, const QString &input, - const QString &output ) const; + const QString &output, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const; %Docstring Generates a command for exporting GPS data into a different format using babel. @@ -60,6 +63,7 @@ Generates a command for exporting GPS data into a different format using babel. :param featureType: type of GPS feature to export :param input: input data path :param output: output path +:param flags: optional flags to control how babel command is generated Returns an empty list if the format does not support exports (see :py:func:`~QgsAbstractBabelFormat.capabilities`). %End @@ -122,7 +126,8 @@ Returns the list of known extensions for the format, e.g. "csv", "txt". virtual QStringList importCommand( const QString &babel, Qgis::GpsFeatureType featureType, const QString &input, - const QString &output ) const; + const QString &output, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const; }; /************************************************************************ diff --git a/python/core/auto_generated/gps/qgsbabelgpsdevice.sip.in b/python/core/auto_generated/gps/qgsbabelgpsdevice.sip.in index fd63365c590..b12bff39ce0 100644 --- a/python/core/auto_generated/gps/qgsbabelgpsdevice.sip.in +++ b/python/core/auto_generated/gps/qgsbabelgpsdevice.sip.in @@ -45,10 +45,10 @@ Constructor for QgsBabelGpsDeviceFormat. :param trackUploadCommand: command for uploading tracks to device %End - virtual QStringList importCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out ) const; - - virtual QStringList exportCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out ) const; - + virtual QStringList importCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const; + virtual QStringList exportCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const; }; diff --git a/python/core/auto_generated/qgis.sip.in b/python/core/auto_generated/qgis.sip.in index b517894fe92..bd97396c119 100644 --- a/python/core/auto_generated/qgis.sip.in +++ b/python/core/auto_generated/qgis.sip.in @@ -371,6 +371,13 @@ The development version typedef QFlags BabelFormatCapabilities; + enum class BabelCommandFlag + { + QuoteFilePaths, + }; + typedef QFlags BabelCommandFlags; + + enum class GpsFeatureType { Waypoint, @@ -457,6 +464,8 @@ QFlags operator|(Qgis::SqlLayerDefinitionCap QFlags operator|(Qgis::BabelFormatCapability f1, QFlags f2); +QFlags operator|(Qgis::BabelCommandFlag f1, QFlags f2); + diff --git a/src/core/gps/qgsbabelformat.cpp b/src/core/gps/qgsbabelformat.cpp index cab05215875..3647d8f754a 100644 --- a/src/core/gps/qgsbabelformat.cpp +++ b/src/core/gps/qgsbabelformat.cpp @@ -43,12 +43,12 @@ QString QgsAbstractBabelFormat::name() const return mName; } -QStringList QgsAbstractBabelFormat::importCommand( const QString &, Qgis::GpsFeatureType, const QString &, const QString & ) const +QStringList QgsAbstractBabelFormat::importCommand( const QString &, Qgis::GpsFeatureType, const QString &, const QString &, Qgis::BabelCommandFlags ) const { return QStringList(); } -QStringList QgsAbstractBabelFormat::exportCommand( const QString &, Qgis::GpsFeatureType, const QString &, const QString & ) const +QStringList QgsAbstractBabelFormat::exportCommand( const QString &, Qgis::GpsFeatureType, const QString &, const QString &, Qgis::BabelCommandFlags ) const { return QStringList(); } @@ -77,14 +77,16 @@ QgsBabelSimpleImportFormat::QgsBabelSimpleImportFormat( const QString &format, c QStringList QgsBabelSimpleImportFormat::importCommand( const QString &babel, Qgis::GpsFeatureType featureType, const QString &input, - const QString &output )const + const QString &output, + Qgis::BabelCommandFlags flags ) const { - return { QStringLiteral( "\"%1\"" ).arg( babel ), + return { flags &Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( babel ) : babel, featureTypeToArgument( featureType ), QStringLiteral( "-i" ), name(), QStringLiteral( "-o" ), QStringLiteral( "gpx" ), - QStringLiteral( "\"%1\"" ).arg( input ), - QStringLiteral( "\"%1\"" ).arg( output ) }; + flags &Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( input ) : input, + flags &Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( output ) : output + }; } diff --git a/src/core/gps/qgsbabelformat.h b/src/core/gps/qgsbabelformat.h index 449c15c5d2f..544fe55f7bb 100644 --- a/src/core/gps/qgsbabelformat.h +++ b/src/core/gps/qgsbabelformat.h @@ -53,13 +53,15 @@ class CORE_EXPORT QgsAbstractBabelFormat * \param featureType type of GPS feature to import * \param input input data path * \param output output path + * \param flags optional flags to control how babel command is generated * * Returns an empty list if the format does not support imports (see capabilities()). */ virtual QStringList importCommand( const QString &babel, Qgis::GpsFeatureType featureType, const QString &input, - const QString &output ) const; + const QString &output, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const; /** * Generates a command for exporting GPS data into a different format using babel. @@ -68,13 +70,15 @@ class CORE_EXPORT QgsAbstractBabelFormat * \param featureType type of GPS feature to export * \param input input data path * \param output output path + * \param flags optional flags to control how babel command is generated * * Returns an empty list if the format does not support exports (see capabilities()). */ virtual QStringList exportCommand( const QString &babel, Qgis::GpsFeatureType featureType, const QString &input, - const QString &output ) const; + const QString &output, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const; protected: @@ -135,7 +139,8 @@ class CORE_EXPORT QgsBabelSimpleImportFormat : public QgsAbstractBabelFormat QStringList importCommand( const QString &babel, Qgis::GpsFeatureType featureType, const QString &input, - const QString &output ) const override; + const QString &output, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const override; private: QString mDescription; QStringList mExtensions; diff --git a/src/core/gps/qgsbabelgpsdevice.cpp b/src/core/gps/qgsbabelgpsdevice.cpp index 6a232e082a9..9ba6f109c2e 100644 --- a/src/core/gps/qgsbabelgpsdevice.cpp +++ b/src/core/gps/qgsbabelgpsdevice.cpp @@ -56,7 +56,7 @@ QgsBabelGpsDeviceFormat::QgsBabelGpsDeviceFormat( const QString &waypointDownloa QStringList QgsBabelGpsDeviceFormat::importCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, - const QString &out ) const + const QString &out, Qgis::BabelCommandFlags flags ) const { QStringList original; @@ -78,13 +78,13 @@ QStringList QgsBabelGpsDeviceFormat::importCommand( const QString &babel, for ( const QString &iter : std::as_const( original ) ) { if ( iter == QLatin1String( "%babel" ) ) - copy.append( babel ); + copy.append( flags & Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( babel ) : babel ); else if ( iter == QLatin1String( "%type" ) ) copy.append( featureTypeToArgument( type ) ); else if ( iter == QLatin1String( "%in" ) ) - copy.append( QStringLiteral( "\"%1\"" ).arg( in ) ); + copy.append( flags & Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( in ) : in ); else if ( iter == QLatin1String( "%out" ) ) - copy.append( QStringLiteral( "\"%1\"" ).arg( out ) ); + copy.append( flags & Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( out ) : out ); else copy.append( iter ); } @@ -94,7 +94,7 @@ QStringList QgsBabelGpsDeviceFormat::importCommand( const QString &babel, QStringList QgsBabelGpsDeviceFormat::exportCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, - const QString &out ) const + const QString &out, Qgis::BabelCommandFlags flags ) const { QStringList original; switch ( type ) @@ -115,13 +115,13 @@ QStringList QgsBabelGpsDeviceFormat::exportCommand( const QString &babel, for ( const QString &iter : std::as_const( original ) ) { if ( iter == QLatin1String( "%babel" ) ) - copy.append( babel ); + copy.append( flags & Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( babel ) : babel ); else if ( iter == QLatin1String( "%type" ) ) copy.append( featureTypeToArgument( type ) ); else if ( iter == QLatin1String( "%in" ) ) - copy.append( QStringLiteral( "\"%1\"" ).arg( in ) ); + copy.append( flags & Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( in ) : in ); else if ( iter == QLatin1String( "%out" ) ) - copy.append( QStringLiteral( "\"%1\"" ).arg( out ) ); + copy.append( flags & Qgis::BabelCommandFlag::QuoteFilePaths ? QStringLiteral( "\"%1\"" ).arg( out ) : out ); else copy.append( iter ); } diff --git a/src/core/gps/qgsbabelgpsdevice.h b/src/core/gps/qgsbabelgpsdevice.h index a566e49ca5b..7912bcdaf6f 100644 --- a/src/core/gps/qgsbabelgpsdevice.h +++ b/src/core/gps/qgsbabelgpsdevice.h @@ -56,8 +56,10 @@ class CORE_EXPORT QgsBabelGpsDeviceFormat : public QgsAbstractBabelFormat const QString &trackDownloadCommand, const QString &trackUploadCommand ); - QStringList importCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out ) const override; - QStringList exportCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out ) const override; + QStringList importCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const override; + QStringList exportCommand( const QString &babel, Qgis::GpsFeatureType type, const QString &in, const QString &out, + Qgis::BabelCommandFlags flags = Qgis::BabelCommandFlags() ) const override; private: diff --git a/src/core/qgis.h b/src/core/qgis.h index ba9f49a9ef5..ef990a9e084 100644 --- a/src/core/qgis.h +++ b/src/core/qgis.h @@ -563,6 +563,19 @@ class CORE_EXPORT Qgis Q_DECLARE_FLAGS( BabelFormatCapabilities, BabelFormatCapability ) Q_ENUM( BabelFormatCapability ) + /** + * Babel command flags, which control how commands and arguments + * are generated for executing GPSBabel processes. + * + * \since QGIS 3.22 + */ + enum class BabelCommandFlag : int + { + QuoteFilePaths = 1 << 0, //!< File paths should be enclosed in quotations and escaped + }; + Q_DECLARE_FLAGS( BabelCommandFlags, BabelCommandFlag ) + Q_ENUM( BabelCommandFlag ) + /** * GPS feature types. * @@ -696,6 +709,7 @@ Q_DECLARE_OPERATORS_FOR_FLAGS( Qgis::BrowserItemCapabilities ) Q_DECLARE_OPERATORS_FOR_FLAGS( Qgis::SublayerQueryFlags ) Q_DECLARE_OPERATORS_FOR_FLAGS( Qgis::SqlLayerDefinitionCapabilities ) Q_DECLARE_OPERATORS_FOR_FLAGS( Qgis::BabelFormatCapabilities ) +Q_DECLARE_OPERATORS_FOR_FLAGS( Qgis::BabelCommandFlags ) // hack to workaround warnings when casting void pointers // retrieved from QLibrary::resolve to function pointers. diff --git a/tests/src/python/test_qgsbabelgpsformat.py b/tests/src/python/test_qgsbabelgpsformat.py index 078f91058c2..2da0d083735 100644 --- a/tests/src/python/test_qgsbabelgpsformat.py +++ b/tests/src/python/test_qgsbabelgpsformat.py @@ -52,26 +52,38 @@ class TestQgsBabelGpsFormat(unittest.TestCase): self.assertEqual( f.importCommand('babel.exe', Qgis.GpsFeatureType.Waypoint, 'c:/test/test.shp', 'c:/test/test.gpx'), - ['"babel.exe"', + ['babel.exe', '-w', '-i', 'shapefile', '-o', 'gpx', - '"c:/test/test.shp"', - '"c:/test/test.gpx"']) + 'c:/test/test.shp', + 'c:/test/test.gpx']) self.assertEqual( f.importCommand('babel.exe', Qgis.GpsFeatureType.Track, 'c:/test/test.shp', 'c:/test/test.gpx'), - ['"babel.exe"', + ['babel.exe', '-t', '-i', 'shapefile', '-o', 'gpx', - '"c:/test/test.shp"', - '"c:/test/test.gpx"']) + 'c:/test/test.shp', + 'c:/test/test.gpx']) self.assertEqual( f.importCommand('babel.exe', Qgis.GpsFeatureType.Route, 'c:/test/test.shp', 'c:/test/test.gpx'), + ['babel.exe', + '-r', + '-i', + 'shapefile', + '-o', + 'gpx', + 'c:/test/test.shp', + 'c:/test/test.gpx']) + + # with quoted paths + self.assertEqual( + f.importCommand('babel.exe', Qgis.GpsFeatureType.Route, 'c:/test/test.shp', 'c:/test/test.gpx', Qgis.BabelCommandFlag.QuoteFilePaths), ['"babel.exe"', '-r', '-i', @@ -80,6 +92,7 @@ class TestQgsBabelGpsFormat(unittest.TestCase): 'gpx', '"c:/test/test.shp"', '"c:/test/test.gpx"']) + # export not supported self.assertEqual( f.exportCommand('babel.exe', Qgis.GpsFeatureType.Waypoint, 'c:/test/test.shp', 'c:/test/test.gpx'), []) @@ -101,7 +114,6 @@ class TestQgsBabelGpsFormat(unittest.TestCase): # self.assertEqual(f.capabilities(), Qgis.BabelFormatCapabilities( # Qgis.BabelFormatCapability.Waypoints | Qgis.BabelFormatCapability.Import)) - # TODO -- babel command should possibly be quoted (or NOT in QgsBabelSimpleImportFormat) self.assertEqual( f.importCommand('babel.exe', Qgis.GpsFeatureType.Waypoint, 'c:/test/test.shp', 'c:/test/test.gpx'), ['babel.exe', @@ -110,8 +122,8 @@ class TestQgsBabelGpsFormat(unittest.TestCase): 'garmin', '-o', 'gpx', - '"c:/test/test.shp"', - '"c:/test/test.gpx"']) + 'c:/test/test.shp', + 'c:/test/test.gpx']) self.assertEqual( f.importCommand('babel.exe', Qgis.GpsFeatureType.Track, 'c:/test/test.shp', 'c:/test/test.gpx'), ['babel.exe', @@ -120,8 +132,8 @@ class TestQgsBabelGpsFormat(unittest.TestCase): 'garmin', '-o', 'gpx', - '"c:/test/test.shp"', - '"c:/test/test.gpx"']) + 'c:/test/test.shp', + 'c:/test/test.gpx']) self.assertEqual( f.importCommand('babel.exe', Qgis.GpsFeatureType.Route, 'c:/test/test.shp', 'c:/test/test.gpx'), ['babel.exe', @@ -130,8 +142,8 @@ class TestQgsBabelGpsFormat(unittest.TestCase): 'garmin', '-o', 'gpx', - '"c:/test/test.shp"', - '"c:/test/test.gpx"']) + 'c:/test/test.shp', + 'c:/test/test.gpx']) self.assertEqual( f.exportCommand('babel.exe', Qgis.GpsFeatureType.Waypoint, 'c:/test/test.shp', 'c:/test/test.gpx'), @@ -141,8 +153,8 @@ class TestQgsBabelGpsFormat(unittest.TestCase): 'gpx', '-o', 'garmin', - '"c:/test/test.shp"', - '"c:/test/test.gpx"']) + 'c:/test/test.shp', + 'c:/test/test.gpx']) self.assertEqual( f.exportCommand('babel.exe', Qgis.GpsFeatureType.Track, 'c:/test/test.shp', 'c:/test/test.gpx'), ['babel.exe', @@ -151,11 +163,23 @@ class TestQgsBabelGpsFormat(unittest.TestCase): 'gpx', '-o', 'garmin', - '"c:/test/test.shp"', - '"c:/test/test.gpx"']) + 'c:/test/test.shp', + 'c:/test/test.gpx']) self.assertEqual( f.exportCommand('babel.exe', Qgis.GpsFeatureType.Route, 'c:/test/test.shp', 'c:/test/test.gpx'), ['babel.exe', + '-r', + '-i', + 'gpx', + '-o', + 'garmin', + 'c:/test/test.shp', + 'c:/test/test.gpx']) + + # with quoted paths + self.assertEqual( + f.exportCommand('babel.exe', Qgis.GpsFeatureType.Route, 'c:/test/test.shp', 'c:/test/test.gpx', Qgis.BabelCommandFlag.QuoteFilePaths), + ['"babel.exe"', '-r', '-i', 'gpx', @@ -194,7 +218,7 @@ class TestQgsBabelGpsFormat(unittest.TestCase): self.assertEqual(registry.deviceNames(), ['Garmin serial']) self.assertIsNotNone(registry.deviceFormat('Garmin serial')) self.assertEqual(registry.deviceFormat('Garmin serial').importCommand('bb', Qgis.GpsFeatureType.Waypoint, 'in_file.shp', 'out_file.gpx'), - ['bb', '-w', '-i', 'garmin', '-o', 'gpx', '"in_file.shp"', '"out_file.gpx"']) + ['bb', '-w', '-i', 'garmin', '-o', 'gpx', 'in_file.shp', 'out_file.gpx']) if __name__ == '__main__':