[pitivi] undo: remove some of the complexity by removing support for async actions.

[Edward Hervey <edwardrv src gnome org>]

commit 4f29f89a41567973b508b6f35b24984fef4e6975
Author: Alessandro Decina <alessandro d gmail com>
Date:   Thu Jun 11 13:28:25 2009 +0200

    undo: remove some of the complexity by removing support for async actions.

 pitivi/ui/mainwindow.py |    4 ++
 pitivi/undo.py          |  138 +++++++++++++----------------------------------
 tests/test_undo.py      |   79 +++------------------------
 3 files changed, 50 insertions(+), 171 deletions(-)
---
diff --git a/pitivi/ui/mainwindow.py b/pitivi/ui/mainwindow.py
index 1e23b0d..5c24663 100644
--- a/pitivi/ui/mainwindow.py
+++ b/pitivi/ui/mainwindow.py
@@ -183,6 +183,7 @@ class PitiviMainWindow(gtk.Window, Loggable):
         self.app.action_log.connect("commit", self._actionLogCommit)
         self.app.action_log.connect("undo", self._actionLogUndo)
         self.app.action_log.connect("redo", self._actionLogRedo)
+        self.app.action_log.connect("cleaned", self._actionLogCleaned)
 
         # if no webcams available, hide the webcam action
         self.app.deviceprobe.connect("device-added", self._deviceChangeCb)
@@ -742,6 +743,9 @@ class PitiviMainWindow(gtk.Window, Loggable):
 
         self._syncDoUndo(action_log)
 
+    def _actionLogCleaned(self, action_log):
+        self._syncDoUndo(action_log)
+
     def _actionLogUndo(self, action_log, stack):
         self._syncDoUndo(action_log)
 
diff --git a/pitivi/undo.py b/pitivi/undo.py
index b72435c..da3eca0 100644
--- a/pitivi/undo.py
+++ b/pitivi/undo.py
@@ -33,7 +33,6 @@ class UndoableAction(Signallable):
         "done": [],
         "undone": [],
         "undone": [],
-        "error": ["exception"]
     }
 
     def do(self):
@@ -48,14 +47,11 @@ class UndoableAction(Signallable):
     def _undone(self):
         self.emit("undone")
 
-    def _error(self, exception):
-        self.emit("error", exception)
-
 class UndoableActionStack(UndoableAction):
     __signals__ = {
         "done": [],
         "undone": [],
-        "error": ["exception"],
+        "cleaned": [],
     }
 
     def __init__(self, action_group_name):
@@ -67,69 +63,28 @@ class UndoableActionStack(UndoableAction):
     def push(self, action):
         self.done_actions.append(action)
 
-    def _runAction(self, action_list, methodName, signalName,
-            continueCallback, finishCallback):
-        try:
-            action = action_list.pop(-1)
-        except IndexError:
-            finishCallback()
-            return
-
-        if action_list is self.done_actions:
-            self.undone_actions.append(action)
-        else:
-            self.done_actions.append(action)
-
-        self._connectToAction(action, action_list,
-                signalName, continueCallback, finishCallback)
-
-        method = getattr(action, methodName)
-        try:
+    def _runAction(self, action_list, method_name):
+        for action in action_list[::-1]:
+            method = getattr(action, method_name)
             method()
-        except Exception, e:
-            self._actionErrorCb(action, e, finishCallback)
 
     def do(self):
-        self._runAction(self.undone_actions, "do", "done",
-                continueCallback=self.do, finishCallback=self._done)
-
-    def undo(self):
-        self._runAction(self.done_actions, "undo", "undone",
-                continueCallback=self.undo, finishCallback=self._undone)
-
-    def _connectToAction(self, action, action_list, signalName,
-            continueCallback, finishCallback):
-        action.connect(signalName, self._actionDoneOrUndoneCb,
-                action_list, continueCallback, finishCallback)
-        action.connect("error", self._actionErrorCb, finishCallback)
-
-    def _disconnectFromAction(self, action):
-        action.disconnect_by_func(self._actionDoneOrUndoneCb)
-        action.disconnect_by_func(self._actionErrorCb)
-
-    def _actionDoneOrUndoneCb(self, action, action_list,
-            continueCallback, finishCallback):
-        self._disconnectFromAction(action)
-
-        if not action_list:
-            finishCallback()
-            return
-
-        continueCallback()
-
-    def _actionErrorCb(self, action, exception, finishCallback):
-        self._disconnectFromAction(action)
-
-        self._error(exception)
-
-    def _done(self):
+        self._runAction(self.undone_actions, "do")
+        self.done_actions = self.undone_actions[::-1]
         self.emit("done")
 
-    def _undone(self):
+    def undo(self):
+        self._runAction(self.done_actions, "undo")
+        self.undone_actions = self.done_actions[::-1]
         self.emit("undone")
 
-    def _error(self, exception):
-        self.emit("error", exception)
+    def clean(self):
+        actions = self.done_actions + self.undone_actions
+        self.undone_actions = []
+        self.done_actions = []
+        self._runAction(actions, "clean")
+        self.emit("cleaned")
+
 
 class UndoableActionLog(Signallable):
     __signals__ = {
@@ -139,7 +94,7 @@ class UndoableActionLog(Signallable):
         "commit": ["stack", "nested"],
         "undo": ["stack"],
         "redo": ["stack"],
-        "error": ["exception"]
+        "cleaned": [],
     }
     def __init__(self):
         self.undo_stacks = []
@@ -160,9 +115,11 @@ class UndoableActionLog(Signallable):
         if self.running:
             return
 
-        stack = self._getTopmostStack()
-        if stack is None:
+        try:
+            stack = self._getTopmostStack()
+        except UndoWrongStateError:
             return
+
         stack.push(action)
         self.emit("push", stack, action)
 
@@ -197,8 +154,7 @@ class UndoableActionLog(Signallable):
 
     def undo(self):
         if self.stacks or not self.undo_stacks:
-            self._error(UndoWrongStateError())
-            return
+            raise UndoWrongStateError()
 
         stack = self.undo_stacks.pop(-1)
 
@@ -209,7 +165,7 @@ class UndoableActionLog(Signallable):
 
     def redo(self):
         if self.stacks or not self.redo_stacks:
-            return self._error(UndoWrongStateError())
+            raise UndoWrongStateError()
 
         stack = self.redo_stacks.pop(-1)
 
@@ -217,34 +173,21 @@ class UndoableActionLog(Signallable):
         self.undo_stacks.append(stack)
         self.emit("redo", stack)
 
-    def _runStack(self, stack, run):
-        self._connectToRunningStack(stack)
-        self.running = True
-        run()
-
-    def _connectToRunningStack(self, stack):
-        stack.connect("done", self._stackDoneCb)
-        stack.connect("undone", self._stackUndoneCb)
-        stack.connect("error", self._stackErrorCb)
-
-    def _disconnectFromRunningStack(self, stack):
-        for method in (self._stackDoneCb, self._stackUndoneCb,
-                self._stackErrorCb):
-            stack.disconnect_by_func(method)
-
-    def _stackDoneCb(self, stack):
-        self.running = False
-        self._disconnectFromRunningStack(stack)
-
-    def _stackUndoneCb(self, stack):
-        self.running = False
-        self._disconnectFromRunningStack(stack)
+    def clean(self):
+        stacks = self.redo_stacks + self.undo_stacks
+        self.redo_stacks = []
+        self.undo_stacks = []
 
-    def _stackErrorCb(self, stack, exception):
-        self.running = False
-        self._disconnectFromRunningStack(stack)
+        for stack in stacks:
+            self._runStack(stack, stack.clean)
+        self.emit("cleaned")
 
-        self.emit("error", exception)
+    def _runStack(self, stack, run):
+        self.running = True
+        try:
+            run()
+        finally:
+            self.running = False
 
     def _getTopmostStack(self, pop=False):
         stack = None
@@ -254,16 +197,13 @@ class UndoableActionLog(Signallable):
             else:
                 stack = self.stacks[-1]
         except IndexError:
-            return self._error(UndoWrongStateError())
+            raise UndoWrongStateError()
 
         return stack
 
     def _stackIsNested(self, stack):
         return bool(len(self.stacks))
 
-    def _error(self, exception):
-        self.emit("error", exception)
-
 class DebugActionLogObserver(Loggable):
     def startObserving(self, log):
         self._connectToActionLog(log)
@@ -276,7 +216,6 @@ class DebugActionLogObserver(Loggable):
         log.connect("commit", self._actionLogCommitCb)
         log.connect("rollback", self._actionLogRollbackCb)
         log.connect("push", self._actionLogPushCb)
-        log.connect("error", self._actionLogErrorCb)
 
     def _disconnectFromActionLog(self, log):
         for method in (self._actionLogBeginCb, self._actionLogCommitCb,
@@ -297,6 +236,3 @@ class DebugActionLogObserver(Loggable):
 
     def _actionLogPushCb(self, log, stack, action):
         self.debug("push %s in %s", action, stack.action_group_name)
-
-    def _actionLogErrorCb(self, log, exception):
-        self.warning("error %r: %s", exception, exception)
diff --git a/tests/test_undo.py b/tests/test_undo.py
index de71b75..30a9e40 100644
--- a/tests/test_undo.py
+++ b/tests/test_undo.py
@@ -112,13 +112,10 @@ class TestUndoableActionStack(TestCase):
         """
         Undo a stack containing a failing action.
         """
-        state = {"done": True, "error": None}
+        state = {"done": True}
         def doneCb(action, value):
             state["done"] = value
 
-        def errorCb(action, exception):
-            state["error"] = exception
-
         state["actions"] = 2
         class Action(UndoableAction):
             def undo(self):
@@ -129,57 +126,18 @@ class TestUndoableActionStack(TestCase):
                 self._undone()
 
             def undo_fail(self):
-                self._error(UndoError("boom"))
+                raise UndoError("meh")
 
         stack = UndoableActionStack("meh")
         stack.connect("done", doneCb)
-        stack.connect("error", errorCb)
-        action1 = Action()
-        action2 = Action()
-        stack.push(action1)
-        stack.push(action2)
-
-        stack.undo()
-        self.failUnlessEqual(state["actions"], 1)
-        self.failUnless(state["done"])
-        self.failUnless(state["error"])
-
-    def testUndoBrokenAction(self):
-        """
-        Undo a stack containing a broken action (raises an exception whereas it
-        should be emitting the "error" signal.
-        """
-        state = {"done": True, "error": None}
-        def doneCb(action, value):
-            state["done"] = value
-
-        def errorCb(action, exception):
-            state["error"] = exception
-
-        state["actions"] = 2
-        class Action(UndoableAction):
-            def undo(self):
-                state["actions"] -= 1
-                if state["actions"] == 1:
-                    self.__class__.undo = self.__class__.undo_fail
-
-                self._undone()
-
-            def undo_fail(self):
-                raise Exception("boom")
-
-        stack = UndoableActionStack("meh")
-        stack.connect("undone", doneCb, False)
-        stack.connect("error", errorCb)
         action1 = Action()
         action2 = Action()
         stack.push(action1)
         stack.push(action2)
 
-        stack.undo()
+        self.failUnlessRaises(UndoError, stack.undo)
         self.failUnlessEqual(state["actions"], 1)
         self.failUnless(state["done"])
-        self.failUnless(state["error"])
 
 
 class TestUndoableActionLog(TestCase):
@@ -198,46 +156,27 @@ class TestUndoableActionLog(TestCase):
 
     def _connectToUndoableActionLog(self, log):
         for signalName in ("begin", "push", "rollback", "commit",
-                    "undo", "redo", "error"):
+                    "undo", "redo"):
             log.connect(signalName, self._undoActionLogSignalCb, signalName)
 
     def _disconnectFromUndoableActionLog(self, log):
         self.log.disconnect_by_func(self._undoActionLogSignalCb)
 
     def testRollbackWrongState(self):
-        self.log.rollback()
-        self.failUnlessEqual(len(self.signals), 1)
-        name, (exception,) = self.signals[0]
-        self.failUnlessEqual(name, "error")
-        self.failUnless(isinstance(exception, UndoWrongStateError))
+        self.failUnlessRaises(UndoWrongStateError, self.log.rollback)
 
     def testCommitWrongState(self):
-        self.log.commit()
-        self.failUnlessEqual(len(self.signals), 1)
-        name, (exception,) = self.signals[0]
-        self.failUnlessEqual(name, "error")
-        self.failUnless(isinstance(exception, UndoWrongStateError))
+        self.failUnlessRaises(UndoWrongStateError, self.log.commit)
 
     def testPushWrongState(self):
+        # no error in this case
         self.log.push(None)
-        self.failUnlessEqual(len(self.signals), 1)
-        name, (exception,) = self.signals[0]
-        self.failUnlessEqual(name, "error")
-        self.failUnless(isinstance(exception, UndoWrongStateError))
 
     def testUndoWrongState(self):
-        self.log.undo()
-        self.failUnlessEqual(len(self.signals), 1)
-        name, (exception,) = self.signals[0]
-        self.failUnlessEqual(name, "error")
-        self.failUnless(isinstance(exception, UndoWrongStateError))
+        self.failUnlessRaises(UndoWrongStateError, self.log.undo)
 
     def testRedoWrongState(self):
-        self.log.redo()
-        self.failUnlessEqual(len(self.signals), 1)
-        name, (exception,) = self.signals[0]
-        self.failUnlessEqual(name, "error")
-        self.failUnless(isinstance(exception, UndoWrongStateError))
+        self.failUnlessRaises(UndoWrongStateError, self.log.redo)
 
     def testCommit(self):
         """