GNOME.org
 

[pitivi] undo: Make sure the proper stack is committed

commit 366e3c4c917a1b0b76ac9b19d75f93e095de1437
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Sun Apr 10 00:16:47 2016 +0200

    undo: Make sure the proper stack is committed
    
    Differential Revision: https://phabricator.freedesktop.org/D902

 pitivi/project.py                   |    2 +-
 pitivi/timeline/timeline.py         |    4 +-
 pitivi/undo/undo.py                 |    6 ++-
 pitivi/utils/timeline.py            |    2 +-
 pitivi/viewer/move_scale_overlay.py |    2 +-
 tests/test_undo.py                  |   20 +++++---
 tests/test_undo_timeline.py         |   90 ++++++++++++++--------------------
 7 files changed, 58 insertions(+), 68 deletions(-)
---
diff --git a/pitivi/project.py b/pitivi/project.py
index 0ed89bc..05ceb63 100644
--- a/pitivi/project.py
+++ b/pitivi/project.py
@@ -1101,7 +1101,7 @@ class Project(Loggable, GES.Project):
                 self.app.proxy_manager.addJob(asset, asset.force_proxying)
 
             if not self.loading_assets:
-                self.app.action_log.commit()
+                self.app.action_log.commit("Adding assets")
         else:
             self.debug("Project still loading, not using proxies: "
                        "%s", asset.props.id)
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index 3ae342a..2192df2 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -835,7 +835,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
                 for clip in clips:
                     clip.get_layer().remove_clip(clip)
                 self._project.pipeline.commit_timeline()
-                self.app.action_log.commit()
+                self.app.action_log.commit("add dragged clip")
 
             self.draggingElement = None
             self.__got_dragged = False
@@ -858,7 +858,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         self.cleanDropData()
         if target == URI_TARGET_ENTRY.target:
             if self.__last_clips_on_leave:
-                self.app.action_log.begin("add clip")
+                self.app.action_log.begin("add dragged clip")
 
                 if self.__on_separators:
                     created_layer = self.__getDroppedLayer()
diff --git a/pitivi/undo/undo.py b/pitivi/undo/undo.py
index cf38e2c..27a26f5 100644
--- a/pitivi/undo/undo.py
+++ b/pitivi/undo/undo.py
@@ -161,7 +161,7 @@ class UndoableActionLog(GObject.Object, Loggable):
         """
         self.begin(action_group_name, finalizing_action)
         yield
-        self.commit()
+        self.commit(action_group_name)
 
     def begin(self, action_group_name, finalizing_action=None):
         """
@@ -218,7 +218,7 @@ class UndoableActionLog(GObject.Object, Loggable):
         self.emit("rollback", stack)
         stack.undo()
 
-    def commit(self):
+    def commit(self, action_group_name):
         """
         Commits the last started transaction.
         """
@@ -228,6 +228,8 @@ class UndoableActionLog(GObject.Object, Loggable):
 
         self.debug("Committing")
         stack = self._get_last_stack(pop=True)
+        if action_group_name != stack.action_group_name:
+            raise UndoWrongStateError("Unexpected commit", action_group_name, stack, self.stacks)
         if not self.stacks:
             self.undo_stacks.append(stack)
         else:
diff --git a/pitivi/utils/timeline.py b/pitivi/utils/timeline.py
index ab4aa8d..8ba13fa 100644
--- a/pitivi/utils/timeline.py
+++ b/pitivi/utils/timeline.py
@@ -263,7 +263,7 @@ class EditingContext(GObject.Object, Loggable):
         self.app.action_log.begin("move-clip")
 
     def finish(self):
-        self.app.action_log.commit()
+        self.app.action_log.commit("move-clip")
         self.timeline.get_asset().pipeline.commit_timeline()
         self.timeline.ui.app.gui.viewer.clipTrimPreviewFinished()
 
diff --git a/pitivi/viewer/move_scale_overlay.py b/pitivi/viewer/move_scale_overlay.py
index faf7c0d..afe222e 100644
--- a/pitivi/viewer/move_scale_overlay.py
+++ b/pitivi/viewer/move_scale_overlay.py
@@ -436,7 +436,7 @@ class MoveScaleOverlay(Overlay):
         self.update_from_source()
         self.on_hover(cursor_position)
 
-        self.__action_log.commit()
+        self.__action_log.commit("Video position change")
         if self.__clicked_handle:
             if not self.__clicked_handle.hovered:
                 self.stack.reset_cursor()
diff --git a/tests/test_undo.py b/tests/test_undo.py
index c5d6061..cbfc351 100644
--- a/tests/test_undo.py
+++ b/tests/test_undo.py
@@ -184,7 +184,7 @@ class TestUndoableActionLog(TestCase):
         self.assertRaises(UndoWrongStateError, self.log.rollback)
 
     def testCommitWrongState(self):
-        self.assertRaises(UndoWrongStateError, self.log.commit)
+        self.assertRaises(UndoWrongStateError, self.log.commit, "")
 
     def testPushWrongState(self):
         # no error in this case
@@ -208,7 +208,7 @@ class TestUndoableActionLog(TestCase):
         self.assertFalse(self.log.dirty())
         self.log.begin("meh")
         self.log.push(DummyUndoableAction())
-        self.log.commit()
+        self.log.commit("meh")
         self.assertTrue(self.log.dirty())
         self.log.checkpoint()
         self.assertFalse(self.log.dirty())
@@ -230,7 +230,7 @@ class TestUndoableActionLog(TestCase):
         self.assertTrue(self.log.is_in_transaction())
 
         self.assertEqual(self.log.undo_stacks, [])
-        self.log.commit()
+        self.log.commit("meh")
         self.assertEqual(len(self.signals), 2)
         name, (stack,) = self.signals[1]
         self.assertEqual(name, "commit")
@@ -238,6 +238,10 @@ class TestUndoableActionLog(TestCase):
         self.assertEqual(len(self.log.undo_stacks), 1)
         self.assertEqual(len(self.log.redo_stacks), 0)
 
+    def test_commit_proper(self):
+        self.log.begin("meh")
+        self.assertRaises(UndoWrongStateError, self.log.commit, "notmeh")
+
     def testNestedCommit(self):
         """
         Do two nested commits.
@@ -259,7 +263,7 @@ class TestUndoableActionLog(TestCase):
         self.assertTrue(self.log.is_in_transaction())
 
         self.assertEqual(self.log.undo_stacks, [])
-        self.log.commit()
+        self.log.commit("nested")
         self.assertEqual(len(self.signals), 3)
         name, (stack,) = self.signals[2]
         self.assertEqual(name, "commit")
@@ -268,7 +272,7 @@ class TestUndoableActionLog(TestCase):
         self.assertEqual(len(self.log.redo_stacks), 0)
 
         self.assertEqual(self.log.undo_stacks, [])
-        self.log.commit()
+        self.log.commit("meh")
         self.assertEqual(len(self.signals), 4)
         name, (stack,) = self.signals[3]
         self.assertEqual(name, "commit")
@@ -361,7 +365,7 @@ class TestUndoableActionLog(TestCase):
         # commit
         self.assertEqual(len(self.log.undo_stacks), 0)
         self.assertEqual(len(self.log.redo_stacks), 0)
-        self.log.commit()
+        self.log.commit("meh")
         self.assertEqual(len(self.signals), 4)
         name, (stack,) = self.signals[3]
         self.assertEqual(name, "commit")
@@ -422,9 +426,9 @@ class TestUndoableActionLog(TestCase):
         self.log.push(action1)
         self.log.begin("nested")
         self.log.push(action2)
-        self.log.commit()
+        self.log.commit("nested")
         self.log.push(action3)
-        self.log.commit()
+        self.log.commit("meh")
 
         self.log.undo()
         self.assertEqual(call_sequence, ["undo3", "undo2", "undo1"])
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index 0260dbd..79fa8cb 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -125,9 +125,8 @@ class TestTimelineUndo(TestCase):
         layer3 = self.timeline.append_layer()
         self.assertEqual([layer1, layer2, layer3], self.timeline.get_layers())
 
-        self.action_log.begin("layer removed")
-        self.timeline.remove_layer(layer2)
-        self.action_log.commit()
+        with self.action_log.started("layer removed"):
+            self.timeline.remove_layer(layer2)
         self.assertEqual([layer1, layer3], self.timeline.get_layers())
 
         self.action_log.undo()
@@ -147,9 +146,8 @@ class TestTimelineUndo(TestCase):
         control_source.props.mode = GstController.InterpolationMode.LINEAR
         source.set_control_source(control_source, "alpha", "direct")
 
-        self.action_log.begin("keyframe added")
-        self.assertTrue(control_source.set(Gst.SECOND * 0.5, 0.2))
-        self.action_log.commit()
+        with self.action_log.started("keyframe added"):
+            self.assertTrue(control_source.set(Gst.SECOND * 0.5, 0.2))
 
         self.assertEqual(1, len(control_source.get_all()))
         self.action_log.undo()
@@ -173,9 +171,8 @@ class TestTimelineUndo(TestCase):
         source.set_control_source(control_source, "alpha", "direct")
         self.assertTrue(control_source.set(Gst.SECOND * 0.5, 0.2))
 
-        self.action_log.begin("keyframe removed")
-        self.assertTrue(control_source.unset(Gst.SECOND * 0.5))
-        self.action_log.commit()
+        with self.action_log.started("keyframe removed"):
+            self.assertTrue(control_source.unset(Gst.SECOND * 0.5))
 
         self.assertEqual(0, len(control_source.get_all()))
         self.action_log.undo()
@@ -191,9 +188,8 @@ class TestTimelineUndo(TestCase):
         self.action_log.connect("commit", TestTimelineUndo.commitCb, stacks)
 
         clip1 = GES.TitleClip()
-        self.action_log.begin("add clip")
-        self.layer.add_clip(clip1)
-        self.action_log.commit()
+        with self.action_log.started("add clip"):
+            self.layer.add_clip(clip1)
 
         self.assertEqual(1, len(stacks))
         stack = stacks[0]
@@ -212,11 +208,10 @@ class TestTimelineUndo(TestCase):
         clip1 = GES.TitleClip()
         self.layer.add_clip(clip1)
 
-        self.action_log.begin("Title text change")
-        source = clip1.get_children(False)[0]
-        source.set_child_property("text", "pigs fly!")
-        self.assertEqual(source.get_child_property("text")[1], "pigs fly!")
-        self.action_log.commit()
+        with self.action_log.started("Title text change"):
+            source = clip1.get_children(False)[0]
+            source.set_child_property("text", "pigs fly!")
+            self.assertEqual(source.get_child_property("text")[1], "pigs fly!")
 
         self.action_log.undo()
         self.assertEqual(source.get_child_property("text")[1], "")
@@ -229,9 +224,8 @@ class TestTimelineUndo(TestCase):
 
         clip1 = GES.TitleClip()
         self.layer.add_clip(clip1)
-        self.action_log.begin("remove clip")
-        self.layer.remove_clip(clip1)
-        self.action_log.commit()
+        with self.action_log.started("remove clip"):
+            self.layer.remove_clip(clip1)
 
         self.assertEqual(1, len(stacks))
         stack = stacks[0]
@@ -254,9 +248,8 @@ class TestTimelineUndo(TestCase):
         self.layer.add_clip(clip1)
 
         effect1 = GES.Effect.new("agingtv")
-        self.action_log.begin("add effect")
-        clip1.add(effect1)
-        self.action_log.commit()
+        with self.action_log.started("add effect"):
+            clip1.add(effect1)
 
         self.assertEqual(1, len(stacks))
         stack = stacks[0]
@@ -285,9 +278,8 @@ class TestTimelineUndo(TestCase):
         self.layer.add_clip(clip1)
 
         effect1 = GES.Effect.new("agingtv")
-        self.action_log.begin("add effect")
-        clip1.add(effect1)
-        self.action_log.commit()
+        with self.action_log.started("add effect"):
+            clip1.add(effect1)
 
         self.assertEqual(1, len(stacks))
         stack = stacks[0]
@@ -300,9 +292,8 @@ class TestTimelineUndo(TestCase):
                                  clip1.get_children(True)
                                  if isinstance(effect, GES.Effect)]))
 
-        self.action_log.begin("remove effect")
-        clip1.remove(effect1)
-        self.action_log.commit()
+        with self.action_log.started("remove effect"):
+            clip1.remove(effect1)
 
         self.assertEqual(0, len([effect for effect in
                                  clip1.get_children(True)
@@ -326,9 +317,8 @@ class TestTimelineUndo(TestCase):
         self.layer.add_clip(clip1)
 
         effect1 = GES.Effect.new("agingtv")
-        self.action_log.begin("add effect")
-        clip1.add(effect1)
-        self.action_log.commit()
+        with self.action_log.started("add effect"):
+            clip1.add(effect1)
 
         self.assertEqual(1, len(stacks))
         stack = stacks[0]
@@ -341,9 +331,8 @@ class TestTimelineUndo(TestCase):
                                  clip1.get_children(True)
                                  if isinstance(effect, GES.Effect)]))
 
-        self.action_log.begin("change child property")
-        effect1.set_child_property("scratch-lines", 0)
-        self.action_log.commit()
+        with self.action_log.started("change child property"):
+            effect1.set_child_property("scratch-lines", 0)
 
         self.assertEqual(effect1.get_child_property("scratch-lines")[1], 0)
         self.action_log.undo()
@@ -364,9 +353,8 @@ class TestTimelineUndo(TestCase):
         clip1.set_start(5 * Gst.SECOND)
         clip1.set_duration(20 * Gst.SECOND)
         self.layer.add_clip(clip1)
-        self.action_log.begin("modify clip")
-        clip1.set_start(10 * Gst.SECOND)
-        self.action_log.commit()
+        with self.action_log.started("modify clip"):
+            clip1.set_start(10 * Gst.SECOND)
 
         self.assertEqual(1, len(stacks))
         stack = stacks[0]
@@ -381,9 +369,8 @@ class TestTimelineUndo(TestCase):
         self.assertEqual(10 * Gst.SECOND, clip1.get_start())
 
         clip1.set_priority(10)
-        self.action_log.begin("priority change")
-        clip1.set_priority(20)
-        self.action_log.commit()
+        with self.action_log.started("priority change"):
+            clip1.set_priority(20)
 
         self.assertEqual(20, clip1.get_priority())
         self.action_log.undo()
@@ -404,10 +391,9 @@ class TestTimelineUndo(TestCase):
         self.assertEqual(5 * Gst.SECOND, timeline_clips[0].get_start())
         self.assertEqual(0.5 * Gst.SECOND, timeline_clips[0].get_duration())
 
-        self.action_log.begin("ungroup")
-        ungrouped = GES.Container.ungroup(clip1, False)
-        self.assertEqual(2, len(ungrouped), ungrouped)
-        self.action_log.commit()
+        with self.action_log.started("ungroup"):
+            ungrouped = GES.Container.ungroup(clip1, False)
+            self.assertEqual(2, len(ungrouped), ungrouped)
         timeline_clips = list(self.getTimelineClips())
         self.assertEqual(2, len(timeline_clips), timeline_clips)
         self.assertEqual(5 * Gst.SECOND, timeline_clips[0].get_start())
@@ -428,15 +414,13 @@ class TestTimelineUndo(TestCase):
 
         self.layer.add_clip(clip)
 
-        self.action_log.begin("split clip")
-        clip1 = clip.split(10 * Gst.SECOND)
-        self.assertEqual(2, len(self.layer.get_clips()))
-        self.action_log.commit()
+        with self.action_log.started("split clip"):
+            clip1 = clip.split(10 * Gst.SECOND)
+            self.assertEqual(2, len(self.layer.get_clips()))
 
-        self.action_log.begin("split clip")
-        clip2 = clip1.split(15 * Gst.SECOND)
-        self.assertEqual(3, len(self.layer.get_clips()))
-        self.action_log.commit()
+        with self.action_log.started("split clip"):
+            clip2 = clip1.split(15 * Gst.SECOND)
+            self.assertEqual(3, len(self.layer.get_clips()))
 
         self.action_log.undo()
         self.assertEqual(2, len(self.layer.get_clips()))
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]