Fix docs and other code review fixes

qgis · May 17, 2020 · 576c2f3 · 576c2f3
1 parent ae0233e
commit 576c2f3
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 69 deletions.
diff --git a/python/gui/auto_generated/qgssublayersdialog.sip.in b/python/gui/auto_generated/qgssublayersdialog.sip.in
@@ -52,15 +52,15 @@ class QgsSublayersDialog : QDialog
                         const QString &name,
                         QWidget *parent /TransferThis/ = 0,
                         Qt::WindowFlags fl = 0,
-                        const QString &providerSource = QString() );
+                        const QString &dataSourceUri = QString() );
 %Docstring
-Construct a new QgsSublayersDialog object - a dialog to select which sub layers to be imported from a file (e.g. from geopackage or zipfile)
+Construct a new QgsSublayersDialog object - a dialog to select which sub layers to be imported from a data source (e.g. from geopackage or zipfile)
 
-@param providerType provider type
-@param name provider type name
-@param parent parent widget of the dialog
-@param fl window flags
-@param providerSource full file name
+:param providerType: provider type
+:param name: provider type name
+:param parent: parent widget of the dialog
+:param fl: window flags
+:param dataSourceUri: data source URI
 %End
 
     ~QgsSublayersDialog();

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -5627,56 +5627,57 @@ bool QgisApp::askUserForZipItemLayers( const QString &path )
     return false;
   }
 
-  // if promptLayers=Load all, load all layers without prompting
-  if ( promptLayers == QgsSublayersDialog::PromptLoadAll )
+  switch ( promptLayers )
   {
-    childItems = zipItem->children();
-  }
-  // exit if promptLayers=Never
-  else if ( promptLayers == QgsSublayersDialog::PromptNever )
-  {
-    delete zipItem;
-    return false;
-  }
-  else
-  {
-    // We initialize a selection dialog and display it.
-    QgsSublayersDialog chooseSublayersDialog( QgsSublayersDialog::Vsifile, QStringLiteral( "vsi" ), this, nullptr, path );
-    QgsSublayersDialog::LayerDefinitionList layers;
+    // load all layers without prompting
+    case QgsSublayersDialog::PromptLoadAll:
+      childItems = zipItem->children();
+      break;
+    // return because we should not prompt at all
+    case QgsSublayersDialog::PromptNever:
+      delete zipItem;
+      return false;
+    // initialize a selection dialog and display it.
+    case QgsSublayersDialog::PromptAlways:
+    case QgsSublayersDialog::PromptIfNeeded:
+      QgsSublayersDialog chooseSublayersDialog( QgsSublayersDialog::Vsifile, QStringLiteral( "vsi" ), this, nullptr, path );
+      QgsSublayersDialog::LayerDefinitionList layers;
 
-    for ( int i = 0; i < zipItem->children().size(); i++ )
-    {
-      QgsDataItem *item = zipItem->children().at( i );
-      QgsLayerItem *layerItem = qobject_cast<QgsLayerItem *>( item );
-      if ( !layerItem )
-        continue;
+      for ( int i = 0; i < zipItem->children().size(); i++ )
+      {
+        QgsDataItem *item = zipItem->children().at( i );
+        QgsLayerItem *layerItem = qobject_cast<QgsLayerItem *>( item );
+        if ( !layerItem )
+          continue;
 
-      QgsDebugMsgLevel( QStringLiteral( "item path=%1 provider=%2" ).arg( item->path(), layerItem->providerKey() ), 2 );
+        QgsDebugMsgLevel( QStringLiteral( "item path=%1 provider=%2" ).arg( item->path(), layerItem->providerKey() ), 2 );
 
-      QgsSublayersDialog::LayerDefinition def;
-      def.layerId = i;
-      def.layerName = item->name();
-      if ( layerItem->providerKey() == QLatin1String( "gdal" ) )
-      {
-        def.type = tr( "Raster" );
-      }
-      else if ( layerItem->providerKey() == QLatin1String( "ogr" ) )
-      {
-        def.type = tr( "Vector" );
+        QgsSublayersDialog::LayerDefinition def;
+        def.layerId = i;
+        def.layerName = item->name();
+        if ( layerItem->providerKey() == QLatin1String( "gdal" ) )
+        {
+          def.type = tr( "Raster" );
+        }
+        else if ( layerItem->providerKey() == QLatin1String( "ogr" ) )
+        {
+          def.type = tr( "Vector" );
+        }
+        layers << def;
       }
-      layers << def;
-    }
 
-    chooseSublayersDialog.populateLayerTable( layers );
+      chooseSublayersDialog.populateLayerTable( layers );
 
-    if ( chooseSublayersDialog.exec() )
-    {
-      const auto constSelection = chooseSublayersDialog.selection();
-      for ( const QgsSublayersDialog::LayerDefinition &def : constSelection )
+      if ( chooseSublayersDialog.exec() )
       {
-        childItems << zipItem->children().at( def.layerId );
+        const auto constSelection = chooseSublayersDialog.selection();
+        for ( const QgsSublayersDialog::LayerDefinition &def : constSelection )
+        {
+          childItems << zipItem->children().at( def.layerId );
+        }
       }
-    }
+
+      break;
   }
 
   if ( childItems.isEmpty() )

diff --git a/src/app/qgsoptions.cpp b/src/app/qgsoptions.cpp
@@ -422,9 +422,9 @@ QgsOptions::QgsOptions( QWidget *parent, Qt::WindowFlags fl, const QList<QgsOpti
 
   cmbPromptSublayers->clear();
   cmbPromptSublayers->addItem( tr( "Always" ), QgsSublayersDialog::PromptAlways );
-  cmbPromptSublayers->addItem( tr( "If needed" ), QgsSublayersDialog::PromptIfNeeded ); //this means, prompt if there are sublayers but no band in the main dataset
+  cmbPromptSublayers->addItem( tr( "If Needed" ), QgsSublayersDialog::PromptIfNeeded ); //this means, prompt if there are sublayers but no band in the main dataset
   cmbPromptSublayers->addItem( tr( "Never" ), QgsSublayersDialog::PromptNever );
-  cmbPromptSublayers->addItem( tr( "Load all" ), QgsSublayersDialog::PromptLoadAll ); // check if this is true
+  cmbPromptSublayers->addItem( tr( "Load All" ), QgsSublayersDialog::PromptLoadAll ); // check if this is true
   cmbPromptSublayers->setCurrentIndex( cmbPromptSublayers->findData( mSettings->enumValue( QStringLiteral( "/qgis/promptForSublayers" ), QgsSublayersDialog::PromptAlways ) ) );
 
   // Scan for valid items in the browser dock

diff --git a/src/gui/qgssublayersdialog.cpp b/src/gui/qgssublayersdialog.cpp
@@ -47,7 +47,7 @@ QgsSublayersDialog::QgsSublayersDialog( ProviderType providerType,
                                         const QString &name,
                                         QWidget *parent,
                                         Qt::WindowFlags fl,
-                                        const QString &providerSource )
+                                        const QString &dataSourceUri )
   : QDialog( parent, fl )
   , mName( name )
 {
@@ -80,18 +80,14 @@ QgsSublayersDialog::QgsSublayersDialog( ProviderType providerType,
       mShowType = true;
   }
 
-  QString fileFullPath = providerType == QgsSublayersDialog::Vsifile
-                         ? providerSource
-                         : QgsProviderRegistry::instance()->decodeUri( name, providerSource )
-                         .value( QStringLiteral( "path" ) )
-                         .toString();
-  QString filename = QFileInfo( fileFullPath ).fileName();
+  const QVariantMap dataSourceUriParsed = QgsProviderRegistry::instance()->decodeUri( name, dataSourceUri );
+  const QString dataSourceFilePath = dataSourceUriParsed.value( QStringLiteral( "path" ) ).toString();
+  const QString filePath = dataSourceFilePath.isEmpty() ? dataSourceUri : dataSourceFilePath;
+  const QString fileName = QFileInfo( filePath ).fileName();
 
-  setWindowTitle( filename.isEmpty() ? title : QStringLiteral( "%1 | %2" ).arg( title, filename ) );
-  mLblFilePath->setText( QDir::toNativeSeparators( QFileInfo( fileFullPath ).canonicalFilePath() ) );
-
-  if ( filename.isEmpty() )
-    mLblFilePath->setVisible( false );
+  setWindowTitle( fileName.isEmpty() ? title : QStringLiteral( "%1 | %2" ).arg( title, fileName ) );
+  mLblFilePath->setText( QDir::toNativeSeparators( QFileInfo( filePath ).canonicalFilePath() ) );
+  mLblFilePath->setVisible( ! fileName.isEmpty() );
 
   // add a "Select All" button - would be nicer with an icon
   connect( mBtnSelectAll, &QAbstractButton::pressed, layersTable, &QTreeView::selectAll );

diff --git a/src/gui/qgssublayersdialog.h b/src/gui/qgssublayersdialog.h
@@ -90,19 +90,19 @@ class GUI_EXPORT QgsSublayersDialog : public QDialog, private Ui::QgsSublayersDi
     //! Constructor for QgsSublayersDialog
 
     /**
-     * Construct a new QgsSublayersDialog object - a dialog to select which sub layers to be imported from a file (e.g. from geopackage or zipfile)
+     * Construct a new QgsSublayersDialog object - a dialog to select which sub layers to be imported from a data source (e.g. from geopackage or zipfile)
      *
-     * @param providerType provider type
-     * @param name provider type name
-     * @param parent parent widget of the dialog
-     * @param fl window flags
-     * @param providerSource full file name
+     * \param providerType provider type
+     * \param name provider type name
+     * \param parent parent widget of the dialog
+     * \param fl window flags
+     * \param dataSourceUri data source URI
      */
     QgsSublayersDialog( ProviderType providerType,
                         const QString &name,
                         QWidget *parent SIP_TRANSFERTHIS = nullptr,
                         Qt::WindowFlags fl = nullptr,
-                        const QString &providerSource = QString() );
+                        const QString &dataSourceUri = QString() );
 
     ~QgsSublayersDialog() override;