New Shapefile Layer dialog: avoid warning message on user cancel

Also add better error message reporting, and deprecate horrible API.
qgis · Jan 29, 2019 · 9a723d9 · 9a723d9
1 parent cf1cf0f
commit 9a723d9
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 23 deletions.
diff --git a/python/gui/auto_generated/qgsnewvectorlayerdialog.sip.in b/python/gui/auto_generated/qgsnewvectorlayerdialog.sip.in
@@ -17,14 +17,40 @@ class QgsNewVectorLayerDialog: QDialog
 %End
   public:
 
-    static QString runAndCreateLayer( QWidget *parent = 0, QString *enc = 0, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(),
-                                      const QString &initialPath = QString() );
+ static QString runAndCreateLayer( QWidget *parent = 0, QString *enc = 0, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(),
+        const QString &initialPath = QString() ) /Deprecated/;
 %Docstring
 Runs the dialog and creates a layer matching the dialog parameters.
 
 If the ``initialPath`` argument is specified, then the dialog will default to the specified filename.
 
 :return: fileName on success, empty string use aborted, QString() if creation failed
+
+.. deprecated:: in QGIS 3.4.5 - use execAndCreateLayer() instead.
+%End
+
+    static QString execAndCreateLayer( QString &errorMessage /Out/, QWidget *parent = 0, const QString &initialPath = QString(), QString *encoding /Out/ = 0, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem() );
+%Docstring
+Runs the dialog and creates a layer matching the dialog parameters.
+
+If the ``initialPath`` argument is specified, then the dialog will default to the specified filename.
+
+Returns a filename if the dialog was accepted, or an empty string if the dialog was canceled.
+If the dialog was accepted but an error occurred while creating the file, then the function will
+return an empty string and ``errorMessage`` will contain the error message.
+
+If ``encoding`` is specified, it will be set to the encoding of the created file.
+
+:param parent: parent widget for dialog
+:param initialPath: initial file path to show in dialog
+:param encoding: if specified, will be set to file encoding of created layer
+:param crs: default layer CRS to show in dialog
+
+:return: - Newly created file name, or an empty string if user canceled or an error occurred.
+         - errorMessage:  will be set to any error message encountered during layer creation
+
+
+.. versionadded:: 3.4.5
 %End
 
     QgsNewVectorLayerDialog( QWidget *parent /TransferThis/ = 0, Qt::WindowFlags fl = QgsGuiUtils::ModalDialogFlags );

diff --git a/src/app/browser/qgsinbuiltdataitemproviders.cpp b/src/app/browser/qgsinbuiltdataitemproviders.cpp
@@ -106,13 +106,18 @@ void QgsAppDirectoryItemGuiProvider::populateContextMenu( QgsDataItem *item, QMe
   {
     QString enc;
     QDir dir( directoryItem->dirPath() );
-    const QString newFile = QgsNewVectorLayerDialog::runAndCreateLayer( QgisApp::instance(), &enc, QgsProject::instance()->defaultCrsForNewLayers(), dir.filePath( QStringLiteral( "new_layer.shp" ) ) );
+    QString error;
+    const QString newFile = QgsNewVectorLayerDialog::execAndCreateLayer( error, QgisApp::instance(), dir.filePath( QStringLiteral( "new_layer.shp" ) ), &enc, QgsProject::instance()->defaultCrsForNewLayers() );
     if ( !newFile.isEmpty() )
     {
       context.messageBar()->pushSuccess( tr( "New ShapeFile" ), tr( "Created <a href=\"%1\">%2</a>" ).arg(
                                            QUrl::fromLocalFile( newFile ).toString(), QDir::toNativeSeparators( newFile ) ) );
       item->refresh();
     }
+    else if ( !error.isEmpty() )
+    {
+      context.messageBar()->pushCritical( tr( "New ShapeFile" ), tr( "Layer creation failed: %1" ).arg( error ) );
+    }
   } );
   newMenu->addAction( createShp );
 

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -5681,7 +5681,8 @@ void QgisApp::fileNewFromTemplateAction( QAction *qAction )
 void QgisApp::newVectorLayer()
 {
   QString enc;
-  QString fileName = QgsNewVectorLayerDialog::runAndCreateLayer( this, &enc, QgsProject::instance()->defaultCrsForNewLayers() );
+  QString error;
+  QString fileName = QgsNewVectorLayerDialog::execAndCreateLayer( error, this, QString(), &enc, QgsProject::instance()->defaultCrsForNewLayers() );
 
   if ( !fileName.isEmpty() )
   {
@@ -5691,12 +5692,12 @@ void QgisApp::newVectorLayer()
     //todo: the last parameter will change accordingly to layer type
     addVectorLayers( fileNames, enc, QStringLiteral( "file" ) );
   }
-  else if ( fileName.isNull() )
+  else if ( !error.isEmpty() )
   {
-    QLabel *msgLabel = new QLabel( tr( "Layer creation failed. Please check the <a href=\"#messageLog\">message log</a> for further information." ), messageBar() );
+    QLabel *msgLabel = new QLabel( tr( "Layer creation failed: %1" ).arg( error ), messageBar() );
     msgLabel->setWordWrap( true );
     connect( msgLabel, &QLabel::linkActivated, mLogDock, &QWidget::show );
-    QgsMessageBarItem *item = new QgsMessageBarItem( msgLabel, Qgis::Warning );
+    QgsMessageBarItem *item = new QgsMessageBarItem( msgLabel, Qgis::Critical );
     messageBar()->pushItem( item );
   }
 }

diff --git a/src/gui/qgsnewvectorlayerdialog.cpp b/src/gui/qgsnewvectorlayerdialog.cpp
@@ -261,6 +261,16 @@ void QgsNewVectorLayerDialog::checkOk()
 // this is static
 QString QgsNewVectorLayerDialog::runAndCreateLayer( QWidget *parent, QString *pEnc, const QgsCoordinateReferenceSystem &crs, const QString &initialPath )
 {
+  QString error;
+  QString res = execAndCreateLayer( error, parent, initialPath, pEnc, crs );
+  if ( res.isEmpty() && error.isEmpty() )
+    res = QString( "" ); // maintain gross earlier API compatibility
+  return res;
+}
+
+QString QgsNewVectorLayerDialog::execAndCreateLayer( QString &errorMessage, QWidget *parent, const QString &initialPath, QString *encoding, const QgsCoordinateReferenceSystem &crs )
+{
+  errorMessage.clear();
   QgsNewVectorLayerDialog geomDialog( parent );
   geomDialog.setCrs( crs );
   if ( !initialPath.isEmpty() )
@@ -301,33 +311,35 @@ QString QgsNewVectorLayerDialog::runAndCreateLayer( QWidget *parent, QString *pE
     QgsDebugMsg( QStringLiteral( "ogr provider loaded" ) );
 
     typedef bool ( *createEmptyDataSourceProc )( const QString &, const QString &, const QString &, QgsWkbTypes::Type,
-        const QList< QPair<QString, QString> > &, const QgsCoordinateReferenceSystem & );
+        const QList< QPair<QString, QString> > &, const QgsCoordinateReferenceSystem &, QString & );
     createEmptyDataSourceProc createEmptyDataSource = ( createEmptyDataSourceProc ) cast_to_fptr( myLib->resolve( "createEmptyDataSource" ) );
     if ( createEmptyDataSource )
     {
       if ( geometrytype != QgsWkbTypes::Unknown )
       {
         QgsCoordinateReferenceSystem srs = geomDialog.crs();
-        if ( !createEmptyDataSource( fileName, fileformat, enc, geometrytype, attributes, srs ) )
+        if ( !createEmptyDataSource( fileName, fileformat, enc, geometrytype, attributes, srs, errorMessage ) )
         {
           return QString();
         }
       }
       else
       {
-        QgsDebugMsg( QStringLiteral( "geometry type not recognised" ) );
+        errorMessage = QObject::tr( "Geometry type not recognised" );
+        QgsDebugMsg( errorMessage );
         return QString();
       }
     }
     else
     {
-      QgsDebugMsg( QStringLiteral( "Resolving newEmptyDataSource(...) failed" ) );
+      errorMessage = QObject::tr( "Resolving newEmptyDataSource(...) failed" );
+      QgsDebugMsg( errorMessage );
       return QString();
     }
   }
 
-  if ( pEnc )
-    *pEnc = enc;
+  if ( encoding )
+    *encoding = enc;
 
   return fileName;
 }

diff --git a/src/gui/qgsnewvectorlayerdialog.h b/src/gui/qgsnewvectorlayerdialog.h
@@ -23,6 +23,7 @@
 
 #include "qgswkbtypes.h"
 #include "qgis_gui.h"
+#include "qgis_sip.h"
 
 /**
  * \ingroup gui
@@ -40,9 +41,34 @@ class GUI_EXPORT QgsNewVectorLayerDialog: public QDialog, private Ui::QgsNewVect
      * If the \a initialPath argument is specified, then the dialog will default to the specified filename.
      *
      * \returns fileName on success, empty string use aborted, QString() if creation failed
+     *
+     * \deprecated in QGIS 3.4.5 - use execAndCreateLayer() instead.
+     */
+    Q_DECL_DEPRECATED static QString runAndCreateLayer( QWidget *parent = nullptr, QString *enc = nullptr, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(),
+        const QString &initialPath = QString() ) SIP_DEPRECATED;
+
+    /**
+     * Runs the dialog and creates a layer matching the dialog parameters.
+     *
+     * If the \a initialPath argument is specified, then the dialog will default to the specified filename.
+     *
+     * Returns a filename if the dialog was accepted, or an empty string if the dialog was canceled.
+     * If the dialog was accepted but an error occurred while creating the file, then the function will
+     * return an empty string and \a errorMessage will contain the error message.
+     *
+     * If \a encoding is specified, it will be set to the encoding of the created file.
+     *
+     * \param errorMessage will be set to any error message encountered during layer creation
+     * \param parent parent widget for dialog
+     * \param initialPath initial file path to show in dialog
+     * \param encoding if specified, will be set to file encoding of created layer
+     * \param crs default layer CRS to show in dialog
+     *
+     * \returns Newly created file name, or an empty string if user canceled or an error occurred.
+     *
+     * \since QGIS 3.4.5
      */
-    static QString runAndCreateLayer( QWidget *parent = nullptr, QString *enc = nullptr, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem(),
-                                      const QString &initialPath = QString() );
+    static QString execAndCreateLayer( QString &errorMessage SIP_OUT, QWidget *parent = nullptr, const QString &initialPath = QString(), QString *encoding SIP_OUT = nullptr, const QgsCoordinateReferenceSystem &crs = QgsCoordinateReferenceSystem() );
 
     /**
      * New dialog constructor.

diff --git a/src/plugins/geometry_checker/qgsgeometrycheckerresulttab.cpp b/src/plugins/geometry_checker/qgsgeometrycheckerresulttab.cpp
@@ -261,13 +261,14 @@ bool QgsGeometryCheckerResultTab::exportErrorsDo( const QString &file )
   {
     return false;
   }
-  typedef bool ( *createEmptyDataSourceProc )( const QString &, const QString &, const QString &, QgsWkbTypes::Type, const QList< QPair<QString, QString> > &, const QgsCoordinateReferenceSystem & );
+  typedef bool ( *createEmptyDataSourceProc )( const QString &, const QString &, const QString &, QgsWkbTypes::Type, const QList< QPair<QString, QString> > &, const QgsCoordinateReferenceSystem &, QString & );
   createEmptyDataSourceProc createEmptyDataSource = ( createEmptyDataSourceProc ) cast_to_fptr( ogrLib.resolve( "createEmptyDataSource" ) );
   if ( !createEmptyDataSource )
   {
     return false;
   }
-  if ( !createEmptyDataSource( file, driver, "UTF-8", QgsWkbTypes::Point, attributes, QgsProject::instance()->crs() ) )
+  QString createError;
+  if ( !createEmptyDataSource( file, driver, "UTF-8", QgsWkbTypes::Point, attributes, QgsProject::instance()->crs(), createError ) )
   {
     return false;
   }

diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -3301,9 +3301,11 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
                                        const QString &encoding,
                                        QgsWkbTypes::Type vectortype,
                                        const QList< QPair<QString, QString> > &attributes,
-                                       const QgsCoordinateReferenceSystem &srs = QgsCoordinateReferenceSystem() )
+                                       const QgsCoordinateReferenceSystem &srs,
+                                       QString &errorMessage )
 {
   QgsDebugMsg( QStringLiteral( "Creating empty vector layer with format: %1" ).arg( format ) );
+  errorMessage.clear();
 
   QgsApplication::registerOgrDrivers();
   OGRSFDriverH driver = OGRGetDriverByName( format.toLatin1() );
@@ -3318,7 +3320,8 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
   {
     if ( !uri.endsWith( QLatin1String( ".shp" ), Qt::CaseInsensitive ) )
     {
-      QgsDebugMsg( QStringLiteral( "uri %1 doesn't end with .shp" ).arg( uri ) );
+      errorMessage = QObject::tr( "URI %1 doesn't end with .shp" ).arg( uri );
+      QgsDebugMsg( errorMessage );
       return false;
     }
 
@@ -3330,7 +3333,8 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
       QString name = fldIt->first.left( 10 );
       if ( fieldNames.contains( name ) )
       {
-        QgsMessageLog::logMessage( QObject::tr( "Duplicate field (10 significant characters): %1" ).arg( name ), QObject::tr( "OGR" ) );
+        errorMessage = QObject::tr( "Duplicate field (10 significant characters): %1" ).arg( name );
+        QgsMessageLog::logMessage( errorMessage, QObject::tr( "OGR" ) );
         return false;
       }
       fieldNames << name;
@@ -3347,7 +3351,8 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
   dataSource.reset( OGR_Dr_CreateDataSource( driver, uri.toUtf8().constData(), nullptr ) );
   if ( !dataSource )
   {
-    QgsMessageLog::logMessage( QObject::tr( "Creating the data source %1 failed: %2" ).arg( uri, QString::fromUtf8( CPLGetLastErrorMsg() ) ), QObject::tr( "OGR" ) );
+    errorMessage = QObject::tr( "Creating the data source %1 failed: %2" ).arg( uri, QString::fromUtf8( CPLGetLastErrorMsg() ) );
+    QgsMessageLog::logMessage( errorMessage, QObject::tr( "OGR" ) );
     return false;
   }
 
@@ -3381,7 +3386,8 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
     case QgsWkbTypes::GeometryCollectionZM:
     case QgsWkbTypes::Unknown:
     {
-      QgsMessageLog::logMessage( QObject::tr( "Unknown vector type of %1" ).arg( ( int )( vectortype ) ), QObject::tr( "OGR" ) );
+      errorMessage = QObject::tr( "Unknown vector type of %1" ).arg( static_cast< int >( vectortype ) );
+      QgsMessageLog::logMessage( errorMessage, QObject::tr( "OGR" ) );
       return false;
     }
 
@@ -3410,7 +3416,8 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
 
   if ( !layer )
   {
-    QgsMessageLog::logMessage( QObject::tr( "Creation of OGR data source %1 failed: %2" ).arg( uri, QString::fromUtf8( CPLGetLastErrorMsg() ) ), QObject::tr( "OGR" ) );
+    errorMessage = QString::fromUtf8( CPLGetLastErrorMsg() );
+    QgsMessageLog::logMessage( errorMessage, QObject::tr( "OGR" ) );
     return false;
   }