Merge pull request #495 from matthias-kuhn/fix-7485

jef-n · jef-n · commit c4645ea0a849 · 2013-04-02T13:13:48.000-07:00
[FIX #7485]: Don't crash when expression fails to parse in dual view/preview column.
diff --git a/src/gui/attributetable/qgsdualview.cpp b/src/gui/attributetable/qgsdualview.cpp
@@ -27,6 +27,7 @@
 #include <QDialog>
 #include <QMenu>
 #include <QProgressDialog>
+#include <QMessageBox>
 
 QgsDualView::QgsDualView( QWidget* parent )
     : QStackedWidget( parent )
@@ -262,7 +263,16 @@ void QgsDualView::previewColumnChanged( QObject* action )
 
   if ( previewAction )
   {
-    mFeatureList->setDisplayExpression( previewAction->text() );
+    if ( !mFeatureList->setDisplayExpression( previewAction->text() ) )
+    {
+      QMessageBox::warning( this
+                            , tr( "Could not set preview column" )
+                            , tr( "Could not set column '%1' as preview column.\nParser error:\n%2" )
+                              .arg( previewAction->text() )
+                              .arg( mFeatureList->parserErrorString() )
+                          );
+    }
+
     mFeatureListPreviewButton->setDefaultAction( previewAction );
     mFeatureListPreviewButton->setPopupMode( QToolButton::InstantPopup );
   }
diff --git a/src/gui/attributetable/qgsfeaturelistmodel.cpp b/src/gui/attributetable/qgsfeaturelistmodel.cpp
@@ -107,7 +107,7 @@ QItemSelectionModel* QgsFeatureListModel::masterSelection()
   return mFilterModel->masterSelection();
 }
 
-void QgsFeatureListModel::setDisplayExpression( const QString expression )
+bool QgsFeatureListModel::setDisplayExpression( const QString expression )
 {
   const QgsFields fields = mFilterModel->layer()->dataProvider()->fields();
 
@@ -117,15 +117,21 @@ void QgsFeatureListModel::setDisplayExpression( const QString expression )
 
   if ( exp->hasParserError() )
   {
-    QString msg = exp->parserErrorString();
+    mParserErrorString = exp->parserErrorString();
     delete exp;
-    throw QgsException( msg );
+    return false;
   }
 
   delete mExpression;
   mExpression = exp;
 
   emit( dataChanged( index( 0, 0 ), index( rowCount() - 1, 0 ) ) );
+  return true;
+}
+
+QString QgsFeatureListModel::parserErrorString()
+{
+  return mParserErrorString;
 }
 
 const QString& QgsFeatureListModel::displayExpression() const
diff --git a/src/gui/attributetable/qgsfeaturelistmodel.h b/src/gui/attributetable/qgsfeaturelistmodel.h
@@ -53,10 +53,18 @@ class QgsFeatureListModel : public QAbstractProxyModel
     QItemSelectionModel* masterSelection();
 
     /**
-     * @throws QgsException in case the expression cannot be parsed
+     *  @param  expression   A {@link QgsExpression} compatible string.
+     *  @return true if the expression could be set, false if there was a parse error.
+     *          If it fails, the old expression will still be applied. Call {@link parserErrorString()}
+     *          for a meaningful error message.
      */
-    void setDisplayExpression( const QString expression );
+    bool setDisplayExpression( const QString expression );
 
+    /**
+     * @brief Returns a detailed message about errors while parsing a QgsExpression.
+     * @return A message containg information about the parser error.
+     */
+    QString parserErrorString();
 
     const QString& displayExpression() const;
     bool featureByIndex( const QModelIndex& index, QgsFeature& feat );
@@ -87,6 +95,7 @@ class QgsFeatureListModel : public QAbstractProxyModel
   private:
     QgsExpression* mExpression;
     QgsAttributeTableFilterModel* mFilterModel;
+    QString mParserErrorString;
 };
 
 Q_DECLARE_METATYPE( QgsFeatureListModel::FeatureInfo )
diff --git a/src/gui/attributetable/qgsfeaturelistview.cpp b/src/gui/attributetable/qgsfeaturelistview.cpp
@@ -71,16 +71,21 @@ void QgsFeatureListView::setModel( QgsFeatureListModel* featureListModel )
   connect( mCurrentEditSelectionModel, SIGNAL( selectionChanged( QItemSelection, QItemSelection ) ), SLOT( editSelectionChanged( QItemSelection, QItemSelection ) ) );
 }
 
-void QgsFeatureListView::setDisplayExpression( const QString expression )
+bool QgsFeatureListView::setDisplayExpression( const QString expression )
 {
-  mModel->setDisplayExpression( expression );
+  return mModel->setDisplayExpression( expression );
 }
 
 const QString& QgsFeatureListView::displayExpression() const
 {
   return mModel->displayExpression();
 }
 
+QString QgsFeatureListView::parserErrorString()
+{
+  return mModel->parserErrorString();
+}
+
 void QgsFeatureListView::mouseMoveEvent( QMouseEvent *event )
 {
   QPoint pos = event->pos();
diff --git a/src/gui/attributetable/qgsfeaturelistview.h b/src/gui/attributetable/qgsfeaturelistview.h
@@ -80,7 +80,7 @@ class GUI_EXPORT QgsFeatureListView : public QListView
      *
      * @see QgsExpression
      */
-    void setDisplayExpression( const QString displayExpression );
+    bool setDisplayExpression( const QString displayExpression );
 
     /**
      * Returns the expression which is currently used to render the features.
@@ -91,6 +91,12 @@ class GUI_EXPORT QgsFeatureListView : public QListView
      */
     const QString& displayExpression() const;
 
+    /**
+     * Returns a detailed message about errors while parsing a QgsExpression.
+     *
+     * @return A message containg information about the parser error.
+     */
+    QString parserErrorString();
 
   protected:
     virtual void mouseMoveEvent( QMouseEvent *event );

Original file line number	Diff line number	Diff line change
`@@ -107,7 +107,7 @@ QItemSelectionModel* QgsFeatureListModel::masterSelection()`
`107`	`107`	`return mFilterModel->masterSelection();`
`108`	`108`	`}`
`109`	`109`
`110`		`-void QgsFeatureListModel::setDisplayExpression( const QString expression )`
	`110`	`+bool QgsFeatureListModel::setDisplayExpression( const QString expression )`
`111`	`111`	`{`
`112`	`112`	`const QgsFields fields = mFilterModel->layer()->dataProvider()->fields();`
`113`	`113`
`@@ -117,15 +117,21 @@ void QgsFeatureListModel::setDisplayExpression( const QString expression )`
`117`	`117`
`118`	`118`	`if ( exp->hasParserError() )`
`119`	`119`	`{`
`120`		`- QString msg = exp->parserErrorString();`
	`120`	`+ mParserErrorString = exp->parserErrorString();`
`121`	`121`	`delete exp;`
`122`		`- throw QgsException( msg );`
	`122`	`+ return false;`
`123`	`123`	`}`
`124`	`124`
`125`	`125`	`delete mExpression;`
`126`	`126`	`mExpression = exp;`
`127`	`127`
`128`	`128`	`emit( dataChanged( index( 0, 0 ), index( rowCount() - 1, 0 ) ) );`
	`129`	`+ return true;`
	`130`	`+}`
	`131`	`+`
	`132`	`+QString QgsFeatureListModel::parserErrorString()`
	`133`	`+{`
	`134`	`+ return mParserErrorString;`
`129`	`135`	`}`
`130`	`136`
`131`	`137`	`const QString& QgsFeatureListModel::displayExpression() const`
Original file line number	Diff line number	Diff line change
`@@ -71,16 +71,21 @@ void QgsFeatureListView::setModel( QgsFeatureListModel* featureListModel )`
`71`	`71`	`connect( mCurrentEditSelectionModel, SIGNAL( selectionChanged( QItemSelection, QItemSelection ) ), SLOT( editSelectionChanged( QItemSelection, QItemSelection ) ) );`
`72`	`72`	`}`
`73`	`73`
`74`		`-void QgsFeatureListView::setDisplayExpression( const QString expression )`
	`74`	`+bool QgsFeatureListView::setDisplayExpression( const QString expression )`
`75`	`75`	`{`
`76`		`- mModel->setDisplayExpression( expression );`
	`76`	`+ return mModel->setDisplayExpression( expression );`
`77`	`77`	`}`
`78`	`78`
`79`	`79`	`const QString& QgsFeatureListView::displayExpression() const`
`80`	`80`	`{`
`81`	`81`	`return mModel->displayExpression();`
`82`	`82`	`}`
`83`	`83`
	`84`	`+QString QgsFeatureListView::parserErrorString()`
	`85`	`+{`
	`86`	`+ return mModel->parserErrorString();`
	`87`	`+}`
	`88`	`+`
`84`	`89`	`void QgsFeatureListView::mouseMoveEvent( QMouseEvent *event )`
`85`	`90`	`{`
`86`	`91`	`QPoint pos = event->pos();`