GNOME.org
 

[pitivi] undo: Make sure toplevel actions are toplevel

commit 01cfde88c862174e1a77d6faf7f5e62f1043fd89
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Fri Apr 21 16:24:37 2017 +0200

    undo: Make sure toplevel actions are toplevel
    
    This helps prevent a toplevel undoable action to be included in an
    undoable action stack.
    
    The effect is that an error will be printed faster, which should be
    good. We should see how to better use this after we see this happening.
    
    Fixes T7694
    
    Reviewed-by: Thibault Saunier <tsaunier gnome org>
    Differential Revision: https://phabricator.freedesktop.org/D1728

 pitivi/clipproperties.py            |   15 ++++++++++-----
 pitivi/effects.py                   |    6 ++++--
 pitivi/medialibrary.py              |    3 ++-
 pitivi/project.py                   |    3 ++-
 pitivi/timeline/elements.py         |   15 ++++++++++-----
 pitivi/timeline/layer.py            |    6 ++++--
 pitivi/timeline/timeline.py         |   26 ++++++++++++++++----------
 pitivi/titleeditor.py               |    3 ++-
 pitivi/transitions.py               |    2 +-
 pitivi/undo/undo.py                 |    9 ++++++---
 pitivi/utils/timeline.py            |    6 ++++--
 pitivi/viewer/move_scale_overlay.py |    5 +++--
 tests/test_undo.py                  |   15 +++++++++++++++
 13 files changed, 79 insertions(+), 35 deletions(-)
---
diff --git a/pitivi/clipproperties.py b/pitivi/clipproperties.py
index 51ec754..5d2c29a 100644
--- a/pitivi/clipproperties.py
+++ b/pitivi/clipproperties.py
@@ -304,7 +304,8 @@ class EffectProperties(Gtk.Expander, Loggable):
     def _removeEffect(self, effect):
         pipeline = self._project.pipeline
         with self.app.action_log.started("remove effect",
-                                         CommitTimelineFinalizingAction(pipeline)):
+                                         finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                         toplevel=True):
             self.__remove_configuration_widget()
             self.effects_properties_manager.cleanCache(effect)
             effect.get_parent().remove(effect)
@@ -335,7 +336,8 @@ class EffectProperties(Gtk.Expander, Loggable):
             effect_info = self.app.effects.getInfo(factory_name)
             pipeline = self._project.pipeline
             with self.app.action_log.started("add effect",
-                                             CommitTimelineFinalizingAction(pipeline)):
+                                             finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                             toplevel=True):
                 effect = self.clip.ui.add_effect(effect_info)
                 if effect:
                     self.clip.set_top_effect_priority(effect, drop_index)
@@ -388,7 +390,8 @@ class EffectProperties(Gtk.Expander, Loggable):
         effect = effects[source_index]
         pipeline = self._project.ges_timeline.get_parent()
         with self.app.action_log.started("move effect",
-                                         CommitTimelineFinalizingAction(pipeline)):
+                                         finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                         toplevel=True):
             clip.set_top_effect_priority(effect, drop_index)
 
         new_path = Gtk.TreePath.new()
@@ -413,7 +416,8 @@ class EffectProperties(Gtk.Expander, Loggable):
         effect = self.storemodel.get_value(_iter, COL_TRACK_EFFECT)
         pipeline = self._project.ges_timeline.get_parent()
         with self.app.action_log.started("change active state",
-                                         CommitTimelineFinalizingAction(pipeline)):
+                                         finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                         toplevel=True):
             effect.props.active = not effect.props.active
         # This is not strictly necessary, but makes sure
         # the UI reflects the current status.
@@ -587,7 +591,8 @@ class TransformationProperties(Gtk.Expander, Loggable):
         assert res
         if value != cvalue:
             with self.app.action_log.started("Transformation property change",
-                                             CommitTimelineFinalizingAction(self._project.pipeline)):
+                                             
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline),
+                                             toplevel=True):
                 self.source.set_child_property(prop, value)
             self.app.gui.viewer.overlay_stack.update(self.source)
 
diff --git a/pitivi/effects.py b/pitivi/effects.py
index 9ab5e82..6c9413f 100644
--- a/pitivi/effects.py
+++ b/pitivi/effects.py
@@ -513,7 +513,8 @@ class EffectListWidget(Gtk.Box, Loggable):
         pipeline = timeline.ges_timeline.get_parent()
         from pitivi.undo.timeline import CommitTimelineFinalizingAction
         with self.app.action_log.started("add effect",
-                                         CommitTimelineFinalizingAction(pipeline)):
+                                         finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                         toplevel=True):
             clip.ui.add_effect(effect_info)
 
     def getSelectedEffect(self):
@@ -630,6 +631,7 @@ class EffectsPropertiesManager:
 
             pipeline = self.app.project_manager.current_project.pipeline
             with self.app.action_log.started("Effect property change",
-                                             CommitTimelineFinalizingAction(pipeline)):
+                                             finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                             toplevel=True):
                 effect.set_child_property(prop.name, value)
             self._current_element_values[prop.name] = value
diff --git a/pitivi/medialibrary.py b/pitivi/medialibrary.py
index 08d357e..b2983b9 100644
--- a/pitivi/medialibrary.py
+++ b/pitivi/medialibrary.py
@@ -648,7 +648,8 @@ class MediaLibraryWidget(Gtk.Box, Loggable):
         rows = [Gtk.TreeRowReference.new(model, path)
                 for path in paths]
 
-        with self.app.action_log.started("remove asset from media library"):
+        with self.app.action_log.started("remove asset from media library",
+                                         toplevel=True):
             for row in rows:
                 asset = model[row.get_path()][COL_ASSET]
                 target = asset.get_proxy_target()
diff --git a/pitivi/project.py b/pitivi/project.py
index aff722d..4af319d 100644
--- a/pitivi/project.py
+++ b/pitivi/project.py
@@ -1923,7 +1923,8 @@ class ProjectSettingsDialog(object):
         self.year_spinbutton.get_adjustment().set_value(year)
 
     def updateProject(self):
-        with self.app.action_log.started("change project settings"):
+        with self.app.action_log.started("change project settings",
+                                         toplevel=True):
             self.project.name = self.title_entry.get_text()
             self.project.author = self.author_entry.get_text()
             self.project.year = str(self.year_spinbutton.get_value_as_int())
diff --git a/pitivi/timeline/elements.py b/pitivi/timeline/elements.py
index 83e9909..7948f3f 100644
--- a/pitivi/timeline/elements.py
+++ b/pitivi/timeline/elements.py
@@ -213,7 +213,8 @@ class KeyframeCurve(FigureCanvas, Loggable):
             res, value = self.__source.control_source_get_value(event.xdata)
             assert res
             self.debug("Create keyframe at (%lf, %lf)", event.xdata, value)
-            with self.__timeline.app.action_log.started("Keyframe added"):
+            with self.__timeline.app.action_log.started("Keyframe added",
+                                                        toplevel=True):
                 self.__source.set(event.xdata, value)
 
     def toggle_keyframe(self, offset):
@@ -265,11 +266,13 @@ class KeyframeCurve(FigureCanvas, Loggable):
                     return
                 # A keyframe has been double-clicked, remove it.
                 self.debug("Removing keyframe at timestamp %lf", offset)
-                with self.__timeline.app.action_log.started("Remove keyframe"):
+                with self.__timeline.app.action_log.started("Remove keyframe",
+                                                            toplevel=True):
                     self.__source.unset(offset)
             else:
                 # Remember the clicked frame for drag&drop.
-                self.__timeline.app.action_log.begin("Move keyframe")
+                self.__timeline.app.action_log.begin("Move keyframe",
+                                                     toplevel=True)
                 self.__offset = offset
                 self.handling_motion = True
             return
@@ -278,7 +281,8 @@ class KeyframeCurve(FigureCanvas, Loggable):
         if result[0]:
             # The line has been clicked.
             self.debug("The keyframe curve has been clicked")
-            self.__timeline.app.action_log.begin("Move keyframe curve segment")
+            self.__timeline.app.action_log.begin("Move keyframe curve segment",
+                                                 toplevel=True)
             x = event.xdata
             offsets = self.__keyframes.get_offsets()
             keyframes = offsets[:, 0]
@@ -912,7 +916,8 @@ class Clip(Gtk.EventBox, Zoomable, Loggable):
             effect_info = self.app.effects.getInfo(self.timeline.dropData)
             pipeline = self.timeline.ges_timeline.get_parent()
             with self.app.action_log.started("add effect",
-                                             CommitTimelineFinalizingAction(pipeline)):
+                                             finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                             toplevel=True):
                 self.add_effect(effect_info)
             self.timeline.cleanDropData()
             success = True
diff --git a/pitivi/timeline/layer.py b/pitivi/timeline/layer.py
index a489c9b..6a00fd2 100644
--- a/pitivi/timeline/layer.py
+++ b/pitivi/timeline/layer.py
@@ -128,7 +128,8 @@ class LayerControls(Gtk.EventBox, Loggable):
         if name == current_name:
             return
 
-        with self.app.action_log.started("change layer name"):
+        with self.app.action_log.started("change layer name",
+                                         toplevel=True):
             self.ges_layer.ui.setName(name)
 
     def __layerPriorityChangedCb(self, unused_ges_layer, unused_pspec):
@@ -194,7 +195,8 @@ class LayerControls(Gtk.EventBox, Loggable):
     def __delete_layer_cb(self, unused_action, unused_parameter):
         pipeline = self.ges_timeline.get_asset().pipeline
         with self.app.action_log.started("delete layer",
-                                         CommitTimelineFinalizingAction(pipeline)):
+                                         finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                         toplevel=True):
             self.ges_timeline.remove_layer(self.ges_layer)
             removed_priority = self.ges_layer.props.priority
             for ges_layer in self.ges_timeline.get_layers():
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index 86c038a..d353c42 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -684,7 +684,8 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
                 if layer_controls:
                     self.__moving_layer = layer_controls.ges_layer
                     self.app.action_log.begin("move layer",
-                                              CommitTimelineFinalizingAction(self._project.pipeline))
+                                              
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline),
+                                              toplevel=True)
                 else:
                     self.layout.marquee.set_start_position(event)
 
@@ -901,7 +902,8 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
             if self.__last_clips_on_leave:
                 pipeline = self._project.pipeline
                 with self.app.action_log.started("add clip",
-                                                 CommitTimelineFinalizingAction(pipeline)):
+                                                 finalizing_action=CommitTimelineFinalizingAction(pipeline),
+                                                 toplevel=True):
                     if self.__on_separators:
                         priority = self.separator_priority(self.__on_separators[1])
                         created_layer = self.create_layer(priority)
@@ -1308,7 +1310,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
     def insert_clips_on_first_layer(self, clips, position=None):
         """Adds clips to the timeline on the first layer."""
         with self.app.action_log.started("insert on first layer",
-                                         CommitTimelineFinalizingAction(self._project.pipeline)):
+                                         
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline)):
             layers = self.ges_timeline.get_layers()
             first_layer = layers[0]
             start = self.__getInsertPosition(position)
@@ -1331,7 +1333,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         clip_position = initial_position
 
         with self.app.action_log.started("add asset",
-                                         CommitTimelineFinalizingAction(self._project.pipeline)):
+                                         
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline)):
             for obj in objs:
                 if isinstance(obj, GES.Clip):
                     obj.set_start(clip_position)
@@ -1606,7 +1608,8 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
     def _deleteSelected(self, unused_action, unused_parameter):
         if self.ges_timeline:
             with self.app.action_log.started("delete clip",
-                                             CommitTimelineFinalizingAction(self._project.pipeline)):
+                                             
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline),
+                                             toplevel=True):
                 for clip in self.timeline.selection:
                     layer = clip.get_layer()
                     if isinstance(clip, GES.TransitionClip):
@@ -1621,7 +1624,8 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
             return
 
         with self.app.action_log.started("ungroup",
-                                         CommitTimelineFinalizingAction(self._project.pipeline)):
+                                         
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline),
+                                         toplevel=True):
             for obj in self.timeline.selection:
                 toplevel = obj.get_toplevel_parent()
                 if toplevel == self.timeline.current_group:
@@ -1637,7 +1641,8 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
             return
 
         with self.app.action_log.started("group",
-                                         CommitTimelineFinalizingAction(self._project.pipeline)):
+                                         
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline),
+                                         toplevel=True):
             containers = set()
             new_group = None
             for obj in self.timeline.selection:
@@ -1668,7 +1673,8 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
             return
 
         with self.app.action_log.started("paste",
-                                         CommitTimelineFinalizingAction(self._project.pipeline)):
+                                         
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline),
+                                         toplevel=True):
             save = self.__copiedGroup.copy(True)
             position = self._project.pipeline.getPosition()
             self.__copiedGroup.paste(position)
@@ -1680,7 +1686,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
 
         progress_dialog = AlignmentProgressDialog(self.app)
         progress_dialog.window.show()
-        self.app.action_log.begin("align")
+        self.app.action_log.begin("align", toplevel=True)
 
         def alignedCb():  # Called when alignment is complete
             self.app.action_log.commit()
@@ -1701,7 +1707,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         If clips are selected, split them at the current playhead position.
         Otherwise, split all clips at the playhead position.
         """
-        with self.app.action_log.started("split clip"):
+        with self.app.action_log.started("split clip", toplevel=True):
             self._splitElements(self.timeline.selection.selected)
 
     def _splitElements(self, clips=None):
diff --git a/pitivi/titleeditor.py b/pitivi/titleeditor.py
index 289079a..1657d4a 100644
--- a/pitivi/titleeditor.py
+++ b/pitivi/titleeditor.py
@@ -116,7 +116,8 @@ class TitleEditor(Loggable):
             self.settings["halignment"].append(en, n)
 
     def _setChildProperty(self, name, value):
-        with self.app.action_log.started("Title %s change" % name):
+        with self.app.action_log.started("Title change property",
+                                         toplevel=True):
             self._setting_props = True
             try:
                 assert self.source.set_child_property(name, value)
diff --git a/pitivi/transitions.py b/pitivi/transitions.py
index 7eba7b4..0ce7680 100644
--- a/pitivi/transitions.py
+++ b/pitivi/transitions.py
@@ -161,7 +161,7 @@ class TransitionsListWidget(Gtk.Box, Loggable):
         if child:
             if not self.container_focused:
                 self.container_focused = True
-                action_log.begin("Change transaction")
+                action_log.begin("Change transaction", toplevel=True)
         else:
             if self.container_focused:
                 self.container_focused = False
diff --git a/pitivi/undo/undo.py b/pitivi/undo/undo.py
index 1108804..6b9b066 100644
--- a/pitivi/undo/undo.py
+++ b/pitivi/undo/undo.py
@@ -185,13 +185,13 @@ class UndoableActionLog(GObject.Object, Loggable):
         self._checkpoint = self._takeSnapshot()
 
     @contextlib.contextmanager
-    def started(self, action_group_name, finalizing_action=None):
+    def started(self, action_group_name, **kwargs):
         """Gets a context manager which commits the transaction at the end."""
-        self.begin(action_group_name, finalizing_action)
+        self.begin(action_group_name, **kwargs)
         yield
         self.commit(action_group_name)
 
-    def begin(self, action_group_name, finalizing_action=None):
+    def begin(self, action_group_name, finalizing_action=None, toplevel=False):
         """Starts recording a high-level operation which later can be undone.
 
         The recording can be stopped by calling the `commit` method or
@@ -204,6 +204,9 @@ class UndoableActionLog(GObject.Object, Loggable):
             self.debug("Abort because running")
             return
 
+        if toplevel and self.is_in_transaction():
+            raise UndoWrongStateError("Toplevel operation started as suboperation", self.stacks)
+
         stack = UndoableActionStack(action_group_name, finalizing_action)
         self.stacks.append(stack)
         self.debug("begin action group %s, nested %s",
diff --git a/pitivi/utils/timeline.py b/pitivi/utils/timeline.py
index b99387b..b5730bb 100644
--- a/pitivi/utils/timeline.py
+++ b/pitivi/utils/timeline.py
@@ -236,8 +236,10 @@ class EditingContext(GObject.Object, Loggable):
         from pitivi.undo.timeline import CommitTimelineFinalizingAction
         self.__log_actions = log_actions
         if log_actions:
-            self.app.action_log.begin("move-clip", CommitTimelineFinalizingAction(
-                self.timeline.get_asset().pipeline))
+            self.app.action_log.begin("move-clip",
+                                      finalizing_action=CommitTimelineFinalizingAction(
+                                          self.timeline.get_asset().pipeline),
+                                      toplevel=True)
 
     def finish(self):
         if self.__log_actions:
diff --git a/pitivi/viewer/move_scale_overlay.py b/pitivi/viewer/move_scale_overlay.py
index 99aee2c..6d73f4b 100644
--- a/pitivi/viewer/move_scale_overlay.py
+++ b/pitivi/viewer/move_scale_overlay.py
@@ -413,8 +413,9 @@ class MoveScaleOverlay(Overlay):
         self.__clicked_handle = None
 
         self.__action_log.begin("Video position change",
-                                CommitTimelineFinalizingAction(
-                                    self._source.get_timeline().get_parent()))
+                                finalizing_action=CommitTimelineFinalizingAction(
+                                    self._source.get_timeline().get_parent()),
+                                toplevel=True)
         if self.hovered_handle:
             self.hovered_handle.on_click()
             self.__clicked_handle = self.hovered_handle
diff --git a/tests/test_undo.py b/tests/test_undo.py
index 214bf7a..9c5f386 100644
--- a/tests/test_undo.py
+++ b/tests/test_undo.py
@@ -373,6 +373,21 @@ class TestUndoableActionLog(TestCase):
                                 mock.call.action2.undo(),
                                 mock.call.action1.undo()])
 
+    def test_toplevel_operation(self):
+        """Checks the toplevel operations nesting."""
+        self.log.begin("one", toplevel=False)
+        self.log.commit("one")
+
+        self.log.begin("two", toplevel=True)
+        self.log.commit("two")
+
+        self.log.begin("three")
+        self.assertRaises(UndoWrongStateError,
+                          self.log.begin,
+                          "four", toplevel=True)
+        self.log.begin("nested1")
+        self.log.begin("nested2", toplevel=False)
+
 
 class TestGObjectObserver(TestCase):
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]