safer handling of command macros

apps/opencs/CMakeLists.txt

@@ -26,7 +26,7 @@ opencs_units_noqt (model/world
     universalid record commands columnbase columnimp scriptcontext cell refidcollection
     refidadapter refiddata refidadapterimp ref collectionbase refcollection columns infocollection tablemimedata cellcoordinates cellselection resources resourcesmanager scope
     pathgrid landtexture land nestedtablewrapper nestedcollection nestedcoladapterimp nestedinfocollection
-    idcompletionmanager metadata defaultgmsts infoselectwrapper
+    idcompletionmanager metadata defaultgmsts infoselectwrapper commandmacro
     )
 
 opencs_hdrs_noqt (model/world

apps/opencs/model/world/commanddispatcher.cpp

@@ -11,6 +11,7 @@
 #include "record.hpp"
 #include "commands.hpp"
 #include "idtableproxymodel.hpp"
+#include "commandmacro.hpp"
 
 std::vector<std::string> CSMWorld::CommandDispatcher::getDeletableRecords() const
 {
@@ -171,10 +172,9 @@ void CSMWorld::CommandDispatcher::executeModify (QAbstractItemModel *model, cons
 
     if (modifyCell.get())
     {
-        mDocument.getUndoStack().beginMacro (modifyData->text());
+        CommandMacro macro (mDocument.getUndoStack());
-        mDocument.getUndoStack().push (modifyData.release());
+        macro.push (modifyData.release());
-        mDocument.getUndoStack().push (modifyCell.release());
+        macro.push (modifyCell.release());
-        mDocument.getUndoStack().endMacro();
     }
     else
         mDocument.getUndoStack().push (modifyData.release());
@@ -194,9 +194,7 @@ void CSMWorld::CommandDispatcher::executeDelete()
 
     int columnIndex = model.findColumnIndex (Columns::ColumnId_Id);
 
-    if (rows.size()>1)
+    CommandMacro macro (mDocument.getUndoStack(), rows.size()>1 ? "Delete multiple records" : "");
-        mDocument.getUndoStack().beginMacro (tr ("Delete multiple records"));
-
     for (std::vector<std::string>::const_iterator iter (rows.begin()); iter!=rows.end(); ++iter)
     {
         std::string id = model.data (model.getModelIndex (*iter, columnIndex)).
@@ -204,7 +202,7 @@ void CSMWorld::CommandDispatcher::executeDelete()
 
         if (mId.getType() == UniversalId::Type_Referenceables)
         {
-            mDocument.getUndoStack().push ( new CSMWorld::DeleteCommand (model, id,
+            macro.push (new CSMWorld::DeleteCommand (model, id,
                 static_cast<CSMWorld::UniversalId::Type>(model.data (model.index (
                     model.getModelIndex (id, columnIndex).row(),
                     model.findColumnIndex (CSMWorld::Columns::ColumnId_RecordType))).toInt())));
@@ -212,9 +210,6 @@ void CSMWorld::CommandDispatcher::executeDelete()
         else
             mDocument.getUndoStack().push (new CSMWorld::DeleteCommand (model, id));
     }
-
-    if (rows.size()>1)
-        mDocument.getUndoStack().endMacro();
 }
 
 void CSMWorld::CommandDispatcher::executeRevert()
@@ -231,25 +226,19 @@ void CSMWorld::CommandDispatcher::executeRevert()
 
     int columnIndex = model.findColumnIndex (Columns::ColumnId_Id);
 
-    if (rows.size()>1)
+    CommandMacro macro (mDocument.getUndoStack(), rows.size()>1 ? "Revert multiple records" : "");
-        mDocument.getUndoStack().beginMacro (tr ("Revert multiple records"));
-
     for (std::vector<std::string>::const_iterator iter (rows.begin()); iter!=rows.end(); ++iter)
     {
         std::string id = model.data (model.getModelIndex (*iter, columnIndex)).
             toString().toUtf8().constData();
 
-        mDocument.getUndoStack().push (new CSMWorld::RevertCommand (model, id));
+        macro.push (new CSMWorld::RevertCommand (model, id));
     }
-
-    if (rows.size()>1)
-        mDocument.getUndoStack().endMacro();
 }
 
 void CSMWorld::CommandDispatcher::executeExtendedDelete()
 {
-    if (mExtendedTypes.size()>1)
+    CommandMacro macro (mDocument.getUndoStack(), mExtendedTypes.size()>1 ? tr ("Extended delete of multiple records") : "");
-        mDocument.getUndoStack().beginMacro (tr ("Extended delete of multiple records"));
 
     for (std::vector<UniversalId>::const_iterator iter (mExtendedTypes.begin());
         iter!=mExtendedTypes.end(); ++iter)
@@ -276,20 +265,15 @@ void CSMWorld::CommandDispatcher::executeExtendedDelete()
                     Misc::StringUtils::lowerCase (record.get().mCell)))
                     continue;
 
-                mDocument.getUndoStack().push (
+                macro.push (new CSMWorld::DeleteCommand (model, record.get().mId));
-                    new CSMWorld::DeleteCommand (model, record.get().mId));
             }
         }
     }
-
-    if (mExtendedTypes.size()>1)
-        mDocument.getUndoStack().endMacro();
 }
 
 void CSMWorld::CommandDispatcher::executeExtendedRevert()
 {
-    if (mExtendedTypes.size()>1)
+    CommandMacro macro (mDocument.getUndoStack(), mExtendedTypes.size()>1 ? tr ("Extended revert of multiple records") : "");
-        mDocument.getUndoStack().beginMacro (tr ("Extended revert of multiple records"));
 
     for (std::vector<UniversalId>::const_iterator iter (mExtendedTypes.begin());
         iter!=mExtendedTypes.end(); ++iter)
@@ -313,12 +297,8 @@ void CSMWorld::CommandDispatcher::executeExtendedRevert()
                     Misc::StringUtils::lowerCase (record.get().mCell)))
                     continue;
 
-                mDocument.getUndoStack().push (
+                macro.push (new CSMWorld::RevertCommand (model, record.get().mId));
-                    new CSMWorld::RevertCommand (model, record.get().mId));
             }
         }
     }
-
-    if (mExtendedTypes.size()>1)
-        mDocument.getUndoStack().endMacro();
 }

apps/opencs/model/world/commandmacro.cpp

@@ -0,0 +1,26 @@
+
+#include "commandmacro.hpp"
+
+#include <QUndoStack>
+#include <QUndoCommand>
+
+CSMWorld::CommandMacro::CommandMacro (QUndoStack& undoStack, const QString& description)
+: mUndoStack (undoStack), mDescription (description), mStarted (false)
+{}
+
+CSMWorld::CommandMacro::~CommandMacro()
+{
+    if (mStarted)
+        mUndoStack.endMacro();
+}
+
+void CSMWorld::CommandMacro::push (QUndoCommand *command)
+{
+    if (!mStarted)
+    {
+        mUndoStack.beginMacro (mDescription.isEmpty() ? command->text() : mDescription);
+        mStarted = true;
+    }
+
+    mUndoStack.push (command);
+}

apps/opencs/model/world/commandmacro.hpp

@@ -0,0 +1,34 @@
+#ifndef CSM_WOLRD_COMMANDMACRO_H
+#define CSM_WOLRD_COMMANDMACRO_H
+
+class QUndoStack;
+class QUndoCommand;
+
+#include <QString>
+
+namespace CSMWorld
+{
+    class CommandMacro
+    {
+            QUndoStack& mUndoStack;
+            QString mDescription;
+            bool mStarted;
+
+            /// not implemented
+            CommandMacro (const CommandMacro&);
+
+            /// not implemented
+            CommandMacro& operator= (const CommandMacro&);
+
+        public:
+
+            /// If \a description is empty, the description of the first command is used.
+            CommandMacro (QUndoStack& undoStack, const QString& description = "");
+
+            ~CommandMacro();
+
+            void push (QUndoCommand *command);
+    };
+}
+
+#endif

apps/opencs/view/render/instancemode.cpp

@@ -8,6 +8,7 @@
 #include "../../model/world/idtable.hpp"
 #include "../../model/world/idtree.hpp"
 #include "../../model/world/commands.hpp"
+#include "../../model/world/commandmacro.hpp"
 
 #include "../widget/scenetoolbar.hpp"
 #include "../widget/scenetoolmode.hpp"
@@ -259,7 +260,8 @@ void CSVRender::InstanceMode::dragCompleted()
         case DragMode_None: break;
     }
 
-    undoStack.beginMacro (description);
+
+    CSMWorld::CommandMacro macro (undoStack, description);
 
     for (std::vector<osg::ref_ptr<TagBase> >::iterator iter (selection.begin());
         iter!=selection.end(); ++iter)
@@ -270,8 +272,6 @@ void CSVRender::InstanceMode::dragCompleted()
         }
     }
 
-    undoStack.endMacro();
-
     mDragMode = DragMode_None;
 }
 
@@ -435,11 +435,10 @@ void CSVRender::InstanceMode::dropEvent (QDropEvent* event)
                         new CSMWorld::ModifyCommand (cellTable, countIndex, count+1));
                 }
 
-                document.getUndoStack().beginMacro (createCommand->text());
+                CSMWorld::CommandMacro macro (document.getUndoStack());
-                document.getUndoStack().push (createCommand.release());
+                macro.push (createCommand.release());
                 if (incrementCommand.get())
-                    document.getUndoStack().push (incrementCommand.release());
+                    macro.push (incrementCommand.release());
-                document.getUndoStack().endMacro();
 
                 dropped = true;
             }

apps/opencs/view/world/referencecreator.cpp

@@ -9,6 +9,7 @@
 #include "../../model/world/columns.hpp"
 #include "../../model/world/idtable.hpp"
 #include "../../model/world/idcompletionmanager.hpp"
+#include "../../model/world/commandmacro.hpp"
 
 #include "../widget/droplineedit.hpp"
 
@@ -53,10 +54,9 @@ void CSVWorld::ReferenceCreator::pushCommand (std::auto_ptr<CSMWorld::CreateComm
     std::auto_ptr<CSMWorld::ModifyCommand> increment (new CSMWorld::ModifyCommand
         (cellTable, countIndex, count+1));
 
-    getUndoStack().beginMacro (command->text());
+    CSMWorld::CommandMacro macro (getUndoStack(), command->text());
     GenericCreator::pushCommand (command, id);
-    getUndoStack().push (increment.release());
+    macro.push (increment.release());
-    getUndoStack().endMacro();
 }
 
 int CSVWorld::ReferenceCreator::getRefNumCount() const
@@ -147,11 +147,11 @@ void CSVWorld::ReferenceCreator::cloneMode(const std::string& originId,
     cellChanged(); //otherwise ok button will remain disabled
 }
 
-CSVWorld::Creator *CSVWorld::ReferenceCreatorFactory::makeCreator (CSMDoc::Document& document, 
+CSVWorld::Creator *CSVWorld::ReferenceCreatorFactory::makeCreator (CSMDoc::Document& document,
                                                                    const CSMWorld::UniversalId& id) const
 {
-    return new ReferenceCreator(document.getData(), 
+    return new ReferenceCreator(document.getData(),
-                                document.getUndoStack(), 
+                                document.getUndoStack(),
                                 id,
                                 document.getIdCompletionManager());
 }

apps/opencs/view/world/regionmap.cpp

@@ -17,6 +17,7 @@
 #include "../../model/world/commands.hpp"
 #include "../../model/world/columns.hpp"
 #include "../../model/world/tablemimedata.hpp"
+#include "../../model/world/commandmacro.hpp"
 
 void CSVWorld::RegionMap::contextMenuEvent (QContextMenuEvent *event)
 {
@@ -159,8 +160,7 @@ void CSVWorld::RegionMap::setRegion (const std::string& regionId)
 
     QString regionId2 = QString::fromUtf8 (regionId.c_str());
 
-    if (selected.size()>1)
+    CSMWorld::CommandMacro macro (mDocument.getUndoStack(), selected.size()>1 ? tr ("Set Region") : "");
-        mDocument.getUndoStack().beginMacro (tr ("Set Region"));
 
     for (QModelIndexList::const_iterator iter (selected.begin()); iter!=selected.end(); ++iter)
     {
@@ -170,12 +170,8 @@ void CSVWorld::RegionMap::setRegion (const std::string& regionId)
         QModelIndex index = cellsModel->getModelIndex (cellId,
             cellsModel->findColumnIndex (CSMWorld::Columns::ColumnId_Region));
 
-        mDocument.getUndoStack().push (
+        macro.push (new CSMWorld::ModifyCommand (*cellsModel, index, regionId2));
-            new CSMWorld::ModifyCommand (*cellsModel, index, regionId2));
     }
-
-    if (selected.size()>1)
-        mDocument.getUndoStack().endMacro();
 }
 
 CSVWorld::RegionMap::RegionMap (const CSMWorld::UniversalId& universalId,
@@ -258,19 +254,15 @@ void CSVWorld::RegionMap::createCells()
     CSMWorld::IdTable *cellsModel = &dynamic_cast<CSMWorld::IdTable&> (*
         mDocument.getData().getTableModel (CSMWorld::UniversalId::Type_Cells));
 
-    if (selected.size()>1)
+    CSMWorld::CommandMacro macro (mDocument.getUndoStack(), selected.size()>1 ? tr ("Create cells"): "");
-        mDocument.getUndoStack().beginMacro (tr ("Create cells"));
 
     for (QModelIndexList::const_iterator iter (selected.begin()); iter!=selected.end(); ++iter)
     {
         std::string cellId = model()->data (*iter, CSMWorld::RegionMap::Role_CellId).
             toString().toUtf8().constData();
 
-        mDocument.getUndoStack().push (new CSMWorld::CreateCommand (*cellsModel, cellId));
+        macro.push (new CSMWorld::CreateCommand (*cellsModel, cellId));
     }
-
-    if (selected.size()>1)
-        mDocument.getUndoStack().endMacro();
 }
 
 void CSVWorld::RegionMap::setRegion()

apps/opencs/view/world/vartypedelegate.cpp

@@ -4,6 +4,7 @@
 
 #include "../../model/world/commands.hpp"
 #include "../../model/world/columns.hpp"
+#include "../../model/world/commandmacro.hpp"
 
 void CSVWorld::VarTypeDelegate::addCommands (QAbstractItemModel *model, const QModelIndex& index, int type)
     const
@@ -36,13 +37,10 @@ void CSVWorld::VarTypeDelegate::addCommands (QAbstractItemModel *model, const QM
         default: break; // ignore the rest
     }
 
-    getUndoStack().beginMacro (
+    CSMWorld::CommandMacro macro (getUndoStack(), "Modify " + model->headerData (index.column(), Qt::Horizontal, Qt::DisplayRole).toString());
-        "Modify " + model->headerData (index.column(), Qt::Horizontal, Qt::DisplayRole).toString());
 
-    getUndoStack().push (new CSMWorld::ModifyCommand (*model, index, type));
+    macro.push (new CSMWorld::ModifyCommand (*model, index, type));
-    getUndoStack().push (new CSMWorld::ModifyCommand (*model, next, value));
+    macro.push (new CSMWorld::ModifyCommand (*model, next, value));
-
-    getUndoStack().endMacro();
 }
 
 CSVWorld::VarTypeDelegate::VarTypeDelegate (const std::vector<std::pair<int, QString> >& values,