Fix handling of overlapping item commands

qgis · Oct 9, 2017 · 30eab10 · 30eab10
1 parent 8fb2c3c
commit 30eab10
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 8 deletions.
diff --git a/src/core/layout/qgslayoutundostack.cpp b/src/core/layout/qgslayoutundostack.cpp
@@ -44,27 +44,31 @@ void QgsLayoutUndoStack::beginCommand( QgsLayoutUndoObjectInterface *object, con
     return;
   }
 
-  mActiveCommand.reset( object->createCommand( commandText, id, nullptr ) );
-  mActiveCommand->saveBeforeState();
+  mActiveCommands.emplace_back( std::unique_ptr< QgsAbstractLayoutUndoCommand >( object->createCommand( commandText, id, nullptr ) ) );
+  mActiveCommands.back()->saveBeforeState();
 }
 
 void QgsLayoutUndoStack::endCommand()
 {
-  if ( !mActiveCommand )
+  if ( mActiveCommands.empty() )
     return;
 
-  mActiveCommand->saveAfterState();
-  if ( mActiveCommand->containsChange() ) //protect against empty commands
+  mActiveCommands.back()->saveAfterState();
+  if ( mActiveCommands.back()->containsChange() ) //protect against empty commands
   {
-    mUndoStack->push( mActiveCommand.release() );
+    mUndoStack->push( mActiveCommands.back().release() );
+    mActiveCommands.pop_back();
 
     mLayout->project()->setDirty( true );
   }
 }
 
 void QgsLayoutUndoStack::cancelCommand()
 {
-  mActiveCommand.reset();
+  if ( mActiveCommands.empty() )
+    return;
+
+  mActiveCommands.pop_back();
 }
 
 QUndoStack *QgsLayoutUndoStack::stack()

diff --git a/src/core/layout/qgslayoutundostack.h b/src/core/layout/qgslayoutundostack.h
@@ -104,7 +104,7 @@ class CORE_EXPORT QgsLayoutUndoStack
 
     std::unique_ptr< QUndoStack > mUndoStack;
 
-    std::unique_ptr< QgsAbstractLayoutUndoCommand > mActiveCommand;
+    std::vector< std::unique_ptr< QgsAbstractLayoutUndoCommand > > mActiveCommands;
 
 #ifdef SIP_RUN
     QgsLayoutUndoStack( const QgsLayoutUndoStack &other );

diff --git a/tests/src/core/testqgslayoutitem.cpp b/tests/src/core/testqgslayoutitem.cpp
@@ -149,6 +149,7 @@ class TestQgsLayoutItem: public QObject
     void writeReadXmlProperties();
     void undoRedo();
     void multiItemUndo();
+    void overlappingUndo();
 
   private:
 
@@ -1481,6 +1482,36 @@ void TestQgsLayoutItem::multiItemUndo()
   QCOMPARE( item->positionWithUnits(), QgsLayoutPoint( 10, 10 ) );
 }
 
+void TestQgsLayoutItem::overlappingUndo()
+{
+  QgsProject proj;
+  QgsLayout l( &proj );
+
+  QgsLayoutItemRectangularShape *item = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item );
+  item->attemptMove( QgsLayoutPoint( 10, 10 ) );
+  QgsLayoutItemRectangularShape *item2 = new QgsLayoutItemRectangularShape( &l );
+  l.addLayoutItem( item2 );
+  item2->attemptMove( QgsLayoutPoint( 20, 20 ) );
+
+  //commands overlap
+  l.undoStack()->beginCommand( item, tr( "Item moved" ), QgsLayoutItem::UndoIncrementalMove );
+  item->attemptMove( QgsLayoutPoint( 1, 1 ) );
+  l.undoStack()->beginCommand( item2, tr( "Item moved" ), QgsLayoutItem::UndoIncrementalMove );
+  item2->attemptMove( QgsLayoutPoint( 21, 21 ) );
+  l.undoStack()->endCommand();
+  l.undoStack()->endCommand();
+
+  // undo should remove item move
+  l.undoStack()->stack()->undo();
+  QCOMPARE( item2->positionWithUnits(), QgsLayoutPoint( 21, 21 ) );
+  QCOMPARE( item->positionWithUnits(), QgsLayoutPoint( 10, 10 ) );
+  l.undoStack()->stack()->undo();
+  QCOMPARE( item2->positionWithUnits(), QgsLayoutPoint( 20, 20 ) );
+  QCOMPARE( item->positionWithUnits(), QgsLayoutPoint( 10, 10 ) );
+
+}
+
 QgsLayoutItem *TestQgsLayoutItem::createCopyViaXml( QgsLayout *layout, QgsLayoutItem *original )
 {
   //save original item to xml