Update labeling tools and make more undo/redo friendly

- Change QgsVectorLayer::redoEditCommand to only strip invalid QVariants, not null ones
- Update undo/redo command text to start with action and end with sample of label text
- Update pin/unpin labels tool to be fully undo/redo-able
- Store generated label text in QgsLabelPosition (sans direction symbols)
- Update change label properties dialog to show whether label text is expression

python/core/qgsmaprenderer.sip

@@ -6,7 +6,7 @@ class QgsLabelPosition
 #include <qgsmaprenderer.h>
 %End
   public:
-    QgsLabelPosition( int id, double r, const QVector< QgsPoint >& corners, const QgsRectangle& rect, double w, double h, const QString& layer, bool upside_down, bool diagram = false, bool pinned = false );
+    QgsLabelPosition( int id, double r, const QVector< QgsPoint >& corners, const QgsRectangle& rect, double w, double h, const QString& layer, const QString& labeltext, bool upside_down, bool diagram = false, bool pinned = false );
     QgsLabelPosition();
     int featureId;
     double rotation;
@@ -15,6 +15,7 @@ class QgsLabelPosition
     double width;
     double height;
     QString layerID;
+    QString labelText;
     bool upsideDown;
     bool isDiagram;
     bool isPinned;

src/app/qgslabelpropertydialog.cpp

@@ -22,20 +22,20 @@
 #include <QColorDialog>
 #include <QFontDialog>
 
-QgsLabelPropertyDialog::QgsLabelPropertyDialog( const QString& layerId, int featureId, QgsMapRenderer* renderer, QWidget * parent, Qt::WindowFlags f ):
+QgsLabelPropertyDialog::QgsLabelPropertyDialog( const QString& layerId, int featureId, const QString& labelText, QgsMapRenderer* renderer, QWidget * parent, Qt::WindowFlags f ):
     QDialog( parent, f ), mMapRenderer( renderer ), mCurrentLabelField( -1 )
 {
   setupUi( this );
   fillHaliComboBox();
   fillValiComboBox();
-  init( layerId, featureId );
+  init( layerId, featureId, labelText );
 }
 
 QgsLabelPropertyDialog::~QgsLabelPropertyDialog()
 {
 }
 
-void QgsLabelPropertyDialog::init( const QString& layerId, int featureId )
+void QgsLabelPropertyDialog::init( const QString& layerId, int featureId, const QString& labelText )
 {
   if ( !mMapRenderer )
   {
@@ -65,30 +65,40 @@ void QgsLabelPropertyDialog::init( const QString& layerId, int featureId )
 
   blockElementSignals( true );
 
+  QgsPalLayerSettings& layerSettings = lbl->layer( layerId );
+
   //get label field and fill line edit
-  QString labelFieldName = vlayer->customProperty( "labeling/fieldName" ).toString();
-  if ( !labelFieldName.isEmpty() )
+  if ( layerSettings.isExpression && !labelText.isNull() )
   {
-    mCurrentLabelField = vlayer->fieldNameIndex( labelFieldName );
-    mLabelTextLineEdit->setText( attributeValues[mCurrentLabelField].toString() );
-    const QgsFieldMap& layerFields = vlayer->pendingFields();
-    switch ( layerFields[mCurrentLabelField].type() )
+    mLabelTextLineEdit->setText( labelText );
+    mLabelTextLineEdit->setEnabled( false );
+    mLabelTextLabel->setText( tr( "Expression result" ) );
+  }
+  else
+  {
+    QString labelFieldName = vlayer->customProperty( "labeling/fieldName" ).toString();
+    if ( !labelFieldName.isEmpty() )
     {
-      case QVariant::Double:
-        mLabelTextLineEdit->setValidator( new QDoubleValidator( this ) );
-        break;
-      case QVariant::Int:
-      case QVariant::UInt:
-      case QVariant::LongLong:
-        mLabelTextLineEdit->setValidator( new QIntValidator( this ) );
-        break;
-      default:
-        break;
+      mCurrentLabelField = vlayer->fieldNameIndex( labelFieldName );
+      mLabelTextLineEdit->setText( attributeValues[mCurrentLabelField].toString() );
+      const QgsFieldMap& layerFields = vlayer->pendingFields();
+      switch ( layerFields[mCurrentLabelField].type() )
+      {
+        case QVariant::Double:
+          mLabelTextLineEdit->setValidator( new QDoubleValidator( this ) );
+          break;
+        case QVariant::Int:
+        case QVariant::UInt:
+        case QVariant::LongLong:
+          mLabelTextLineEdit->setValidator( new QIntValidator( this ) );
+          break;
+        default:
+          break;
+      }
     }
   }
 
   //get attributes of the feature and fill data defined values
-  QgsPalLayerSettings& layerSettings = lbl->layer( layerId );
   mLabelFont = layerSettings.textFont;
 
   //set all the gui elements to the default values

src/app/qgslabelpropertydialog.h

@@ -30,7 +30,7 @@ class QgsLabelPropertyDialog: public QDialog, private Ui::QgsLabelPropertyDialog
 {
     Q_OBJECT
   public:
-    QgsLabelPropertyDialog( const QString& layerId, int featureId, QgsMapRenderer* renderer, QWidget * parent = 0, Qt::WindowFlags f = 0 );
+    QgsLabelPropertyDialog( const QString& layerId, int featureId, const QString& labelText, QgsMapRenderer* renderer, QWidget * parent = 0, Qt::WindowFlags f = 0 );
     ~QgsLabelPropertyDialog();
 
     /**Returns properties changed by the user*/
@@ -56,7 +56,7 @@ class QgsLabelPropertyDialog: public QDialog, private Ui::QgsLabelPropertyDialog
 
   private:
     /**Sets activation / values to the gui elements depending on the label settings and feature values*/
-    void init( const QString& layerId, int featureId );
+    void init( const QString& layerId, int featureId, const QString& labelText );
     void disableGuiElements();
     /**Block / unblock all input element signals*/
     void blockElementSignals( bool block );

src/app/qgsmaptoolchangelabelproperties.cpp

@@ -53,18 +53,26 @@ void QgsMapToolChangeLabelProperties::canvasReleaseEvent( QMouseEvent *e )
   QgsVectorLayer* vlayer = currentLayer();
   if ( mLabelRubberBand && mCanvas && vlayer )
   {
-    QgsLabelPropertyDialog d( mCurrentLabelPos.layerID, mCurrentLabelPos.featureId, mCanvas->mapRenderer() );
+    QString labeltext = QString(); // NULL QString signifies no expression
+    bool settingsOk;
+    QgsPalLayerSettings& labelSettings = currentLabelSettings( &settingsOk );
+    if ( settingsOk && labelSettings.isExpression )
+    {
+      labeltext = mCurrentLabelPos.labelText;
+    }
+
+    QgsLabelPropertyDialog d( mCurrentLabelPos.layerID, mCurrentLabelPos.featureId, labeltext, mCanvas->mapRenderer() );
     if ( d.exec() == QDialog::Accepted )
     {
       const QgsAttributeMap& changes = d.changedProperties();
       if ( changes.size() > 0 )
       {
-        vlayer->beginEditCommand( tr( "Label properties changed" ) );
+        vlayer->beginEditCommand( tr( "Changed properties for label" ) + QString( " '%1'" ).arg( currentLabelText( 24 ) ) );
 
         QgsAttributeMap::const_iterator changeIt = changes.constBegin();
         for ( ; changeIt != changes.constEnd(); ++changeIt )
         {
-          vlayer->changeAttributeValue( mCurrentLabelPos.featureId, changeIt.key(), changeIt.value(), false );
+          vlayer->changeAttributeValue( mCurrentLabelPos.featureId, changeIt.key(), changeIt.value(), true );
         }
 
         vlayer->endEditCommand();

src/app/qgsmaptoollabel.cpp

@@ -145,22 +145,49 @@ QgsPalLayerSettings& QgsMapToolLabel::currentLabelSettings( bool* ok )
   return mInvalidLabelSettings;
 }
 
-QString QgsMapToolLabel::currentLabelText()
+QString QgsMapToolLabel::currentLabelText( int trunc )
 {
-  QgsVectorLayer* vlayer = currentLayer();
-  if ( !vlayer )
+  bool settingsOk;
+  QgsPalLayerSettings& labelSettings = currentLabelSettings( &settingsOk );
+  if ( !settingsOk )
   {
     return "";
   }
 
-  QString labelField = vlayer->customProperty( "labeling/fieldName" ).toString();
-  if ( !labelField.isEmpty() )
+  if ( labelSettings.isExpression )
   {
-    int labelFieldId = vlayer->fieldNameIndex( labelField );
-    QgsFeature f;
-    if ( vlayer->featureAtId( mCurrentLabelPos.featureId, f, false, true ) )
+    QString labelText = mCurrentLabelPos.labelText;
+
+    if ( trunc > 0 && labelText.length() > trunc )
     {
-      return f.attributeMap()[labelFieldId].toString();
+      labelText.truncate( trunc );
+      labelText += "...";
+    }
+    return labelText;
+  }
+  else
+  {
+    QgsVectorLayer* vlayer = currentLayer();
+    if ( !vlayer )
+    {
+      return "";
+    }
+
+    QString labelField = vlayer->customProperty( "labeling/fieldName" ).toString();
+    if ( !labelField.isEmpty() )
+    {
+      int labelFieldId = vlayer->fieldNameIndex( labelField );
+      QgsFeature f;
+      if ( vlayer->featureAtId( mCurrentLabelPos.featureId, f, false, true ) )
+      {
+        QString labelText = f.attributeMap()[labelFieldId].toString();
+        if ( trunc > 0 && labelText.length() > trunc )
+        {
+          labelText.truncate( trunc );
+          labelText += "...";
+        }
+        return labelText;
+      }
     }
   }
   return "";

src/app/qgsmaptoollabel.h

@@ -85,7 +85,9 @@ class QgsMapToolLabel: public QgsMapTool
     /**Returns layer settings of current label position*/
     QgsPalLayerSettings& currentLabelSettings( bool* ok );
 
-    QString currentLabelText();
+    /**Returns current label's text
+      @param trunc number of chars to truncate to, with ... added (added in 1.9)*/
+    QString currentLabelText( int trunc = 0 );
 
     void currentAlignment( QString& hali, QString& vali );
 

src/app/qgsmaptoolmovelabel.cpp

@@ -149,9 +149,9 @@ void QgsMapToolMoveLabel::canvasReleaseEvent( QMouseEvent * e )
     }
   }
 
-  vlayer->beginEditCommand( tr( "Label moved" ) );
-  vlayer->changeAttributeValue( mCurrentLabelPos.featureId, xCol, xPosNew, false );
-  vlayer->changeAttributeValue( mCurrentLabelPos.featureId, yCol, yPosNew, false );
+  vlayer->beginEditCommand( tr( "Moved label" ) + QString( " '%1'" ).arg( currentLabelText( 24 ) ) );
+  vlayer->changeAttributeValue( mCurrentLabelPos.featureId, xCol, xPosNew, true );
+  vlayer->changeAttributeValue( mCurrentLabelPos.featureId, yCol, yPosNew, true );
 
   // set rotation to that of label, if data-defined and no rotation set yet
   // honor whether to preserve preexisting data on pin
@@ -167,7 +167,7 @@ void QgsMapToolMoveLabel::canvasReleaseEvent( QMouseEvent * e )
     if ( dataDefinedRotation( vlayer, mCurrentLabelPos.featureId, defRot, rSuccess ) )
     {
       double labelRot = mCurrentLabelPos.rotation * 180 / M_PI;
-      vlayer->changeAttributeValue( mCurrentLabelPos.featureId, rCol, labelRot, false );
+      vlayer->changeAttributeValue( mCurrentLabelPos.featureId, rCol, labelRot, true );
     }
   }
   vlayer->endEditCommand();

src/app/qgsmaptoolpinlabels.cpp

@@ -134,7 +134,7 @@ void QgsMapToolPinLabels::updatePinnedLabels()
   if ( mShowPinned )
   {
     QgsDebugMsg( QString( "Updating highlighting due to layer editing mode change" ) );
-    mCanvas->refresh();
+    highlightPinnedLabels();
   }
 }
 
@@ -287,11 +287,11 @@ void QgsMapToolPinLabels::pinUnpinLabels( const QgsRectangle& ext, QMouseEvent *
     mCurrentLabelPos = *it;
 
 #ifdef QGISDEBUG
-    QString labeltxt = currentLabelText();
     QString labellyr = currentLayer()->name();
+    QString labeltxt = currentLabelText();
 #endif
-    QgsDebugMsg( QString( "Label: %0" ).arg( labeltxt ) );
     QgsDebugMsg( QString( "Layer: %0" ).arg( labellyr ) );
+    QgsDebugMsg( QString( "Label: %0" ).arg( labeltxt ) );
 
     QgsMapLayer* layer = QgsMapLayerRegistry::instance()->mapLayer( mCurrentLabelPos.layerID );
     if ( !layer )
@@ -390,7 +390,8 @@ bool QgsMapToolPinLabels::pinUnpinLabel( QgsVectorLayer* vlayer,
   // edit attribute table
   int fid = labelpos.featureId;
 
-  QString failedWrite = QString( "Failed write to attribute table" );
+  bool writeFailed = false;
+  QString labelText = currentLabelText( 24 );
 
   if ( pin )
   {
@@ -416,49 +417,42 @@ bool QgsMapToolPinLabels::pinUnpinLabel( QgsVectorLayer* vlayer,
       labelY = transformedPoint.y();
     }
 
-    vlayer->beginEditCommand( tr( "Label pinned" ) );
-    if ( !vlayer->changeAttributeValue( fid, xCol, labelX, false ) )
-    {
-      QgsDebugMsg( failedWrite );
-      return false;
-    }
-    if ( !vlayer->changeAttributeValue( fid, yCol, labelY, false ) )
-    {
-      QgsDebugMsg( failedWrite );
-      return false;
-    }
+    vlayer->beginEditCommand( tr( "Pinned label" ) + QString( " '%1'" ).arg( labelText ) );
+    writeFailed = !vlayer->changeAttributeValue( fid, xCol, labelX, true );
+    writeFailed = !vlayer->changeAttributeValue( fid, yCol, labelY, true );
     if ( hasRCol && !preserveRot )
     {
-      if ( !vlayer->changeAttributeValue( fid, rCol, labelR, false ) )
-      {
-        QgsDebugMsg( failedWrite );
-        return false;
-      }
+      writeFailed = !vlayer->changeAttributeValue( fid, rCol, labelR, true );
     }
     vlayer->endEditCommand();
   }
   else
   {
-    vlayer->beginEditCommand( tr( "Label unpinned" ) );
-    if ( !vlayer->changeAttributeValue( fid, xCol, QVariant(), false ) )
-    {
-      QgsDebugMsg( failedWrite );
-      return false;
-    }
-    if ( !vlayer->changeAttributeValue( fid, yCol, QVariant(), false ) )
-    {
-      QgsDebugMsg( failedWrite );
-      return false;
-    }
+    vlayer->beginEditCommand( tr( "Unpinned label" ) + QString( " '%1'" ).arg( labelText ) );
+    writeFailed = !vlayer->changeAttributeValue( fid, xCol, QVariant( QString::null ), true );
+    writeFailed = !vlayer->changeAttributeValue( fid, yCol, QVariant( QString::null ), true );
     if ( hasRCol && !preserveRot )
     {
-      if ( !vlayer->changeAttributeValue( fid, rCol, QVariant(), false ) )
-      {
-        QgsDebugMsg( failedWrite );
-        return false;
-      }
+      writeFailed = !vlayer->changeAttributeValue( fid, rCol, QVariant( QString::null ), true );
     }
     vlayer->endEditCommand();
   }
+
+  if ( writeFailed )
+  {
+    QgsDebugMsg( QString( "Write to attribute table failed" ) );
+
+//    QgsDebugMsg( QString( "Undoing and removing failed command from layer's undo stack" ) );
+//    int lastCmdIndx = vlayer->undoStack()->count();
+//    const QgsUndoCommand* lastCmd = qobject_cast<const QgsUndoCommand *>( vlayer->undoStack()->command( lastCmdIndx ) );
+//    if ( lastCmd )
+//    {
+//      vlayer->undoEditCommand( lastCmd );
+//      delete vlayer->undoStack()->command( lastCmdIndx );
+//    }
+
+    return false;
+  }
+
   return true;
 }

src/app/qgsmaptoolrotatelabel.cpp

@@ -171,8 +171,8 @@ void QgsMapToolRotateLabel::canvasReleaseEvent( QMouseEvent *e )
     return;
   }
 
-  vlayer->beginEditCommand( tr( "Label rotated" ) );
-  vlayer->changeAttributeValue( mCurrentLabelPos.featureId, rotationCol, rotation, false );
+  vlayer->beginEditCommand( tr( "Rotated label" ) + QString( " '%1'" ).arg( currentLabelText( 24 ) ) );
+  vlayer->changeAttributeValue( mCurrentLabelPos.featureId, rotationCol, rotation, true );
   vlayer->endEditCommand();
   mCanvas->refresh();
 }

src/app/qgsmaptoolshowhidelabels.cpp

@@ -280,12 +280,31 @@ bool QgsMapToolShowHideLabels::showHideLabel( QgsVectorLayer* vlayer,
     return false;
   }
 
-  // edit attribute table
-  QString editTxt = hide ? tr( "Label hidden" ) : tr( "Label shown" );
-  vlayer->beginEditCommand( editTxt );
-  if ( !vlayer->changeAttributeValue( fid, showCol, ( hide ? 0 : 1 ), false ) )
+  // check if attribute value is already the same
+  QgsFeature f;
+  if ( !vlayer->featureAtId( fid, f, false, true ) )
+  {
+    return false;
+  }
+
+  int colVal = hide ? 0 : 1;
+  QVariant fQVal = f.attributeMap()[showCol];
+  bool convToInt;
+  int fVal = fQVal.toInt( &convToInt );
+
+  if ( !convToInt || fVal == colVal )
+  {
+    return false;
+  }
+
+  // different attribute value, edit table
+  QString labelText = currentLabelText( 24 );
+  QString editTxt = hide ? tr( "Hid label" ) : tr( "Showed label" );
+  vlayer->beginEditCommand( editTxt + QString( " '%1'" ).arg( labelText ) );
+  if ( !vlayer->changeAttributeValue( fid, showCol, colVal, true ) )
   {
     QgsDebugMsg( "Failed write to attribute table" );
+    vlayer->endEditCommand();
     return false;
   }
   vlayer->endEditCommand();