[pitivi] undo: Use a context manager to commit the transaction

[Alexandru Băluț <alexbalut src gnome org>]

commit 2b7cacfc408bec5ccaa533d0a1e0e85dd29bb07f
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Sat Apr 9 23:01:16 2016 +0200

    undo: Use a context manager to commit the transaction
    
    Differential Revision: https://phabricator.freedesktop.org/D900

 pitivi/clipproperties.py    |   47 +++++++---------
 pitivi/effects.py           |    5 +-
 pitivi/medialibrary.py      |   12 ++--
 pitivi/project.py           |   26 ++++-----
 pitivi/timeline/layer.py    |    7 +-
 pitivi/timeline/timeline.py |  132 ++++++++++++++++++++-----------------------
 pitivi/titleeditor.py       |   13 ++--
 pitivi/undo/undo.py         |   31 ++++++++++
 8 files changed, 142 insertions(+), 131 deletions(-)
---
diff --git a/pitivi/clipproperties.py b/pitivi/clipproperties.py
index 5bdbc4f..8bc7188 100644
--- a/pitivi/clipproperties.py
+++ b/pitivi/clipproperties.py
@@ -299,12 +299,11 @@ class EffectProperties(Gtk.Expander, Loggable):
         self._removeEffect(effect)
 
     def _removeEffect(self, effect):
-        self.app.action_log.begin("remove effect")
-        self.__remove_configuration_widget()
-        self.effects_properties_manager.cleanCache(effect)
-        effect.get_parent().remove(effect)
-        self._project.timeline.commit()
-        self.app.action_log.commit()
+        with self.app.action_log.started("remove effect"):
+            self.__remove_configuration_widget()
+            self.effects_properties_manager.cleanCache(effect)
+            effect.get_parent().remove(effect)
+            self._project.timeline.commit()
         self._updateTreeview()
 
     def addEffectToClip(self, clip, factory_name, priority=None):
@@ -317,13 +316,12 @@ class EffectProperties(Gtk.Expander, Loggable):
             if track_type == GES.TrackType.AUDIO and media_type == AUDIO_EFFECT or \
                     track_type == GES.TrackType.VIDEO and media_type == VIDEO_EFFECT:
                 # Actually add the effect
-                self.app.action_log.begin("add effect")
-                effect = GES.Effect.new(bin_description=factory_name)
-                clip.add(effect)
-                if priority is not None and priority < len(model):
-                    clip.set_top_effect_priority(effect, priority)
-                self._project.timeline.commit()
-                self.app.action_log.commit()
+                with self.app.action_log.started("add effect"):
+                    effect = GES.Effect.new(bin_description=factory_name)
+                    clip.add(effect)
+                    if priority is not None and priority < len(model):
+                        clip.set_top_effect_priority(effect, priority)
+                    self._project.timeline.commit()
                 self.__updateAll()
                 break
 
@@ -413,10 +411,9 @@ class EffectProperties(Gtk.Expander, Loggable):
         # The paths are different.
         effects = clip.get_top_effects()
         effect = effects[source_index]
-        self.app.action_log.begin("move effect")
-        clip.set_top_effect_priority(effect, drop_index)
-        self._project.timeline.commit()
-        self.app.action_log.commit()
+        with self.app.action_log.started("move effect"):
+            clip.set_top_effect_priority(effect, drop_index)
+            self._project.timeline.commit()
         self._project.pipeline.flushSeek()
         new_path = Gtk.TreePath.new()
         new_path.append_index(drop_index)
@@ -440,12 +437,11 @@ class EffectProperties(Gtk.Expander, Loggable):
     def _effectActiveToggleCb(self, cellrenderertoggle, path):
         _iter = self.storemodel.get_iter(path)
         tck_effect = self.storemodel.get_value(_iter, COL_TRACK_EFFECT)
-        self.app.action_log.begin("change active state")
-        tck_effect.set_active(not tck_effect.is_active())
-        cellrenderertoggle.set_active(tck_effect.is_active())
-        self._updateTreeview()
-        self._project.timeline.commit()
-        self.app.action_log.commit()
+        with self.app.action_log.started("change active state"):
+            tck_effect.set_active(not tck_effect.is_active())
+            cellrenderertoggle.set_active(tck_effect.is_active())
+            self._updateTreeview()
+            self._project.timeline.commit()
 
     def _expandedCb(self, unused_expander, unused_params):
         self.__updateAll()
@@ -627,9 +623,8 @@ class TransformationProperties(Gtk.Expander, Loggable):
         res, cvalue = self.source.get_child_property(prop)
         assert res
         if value != cvalue:
-            self.app.action_log.begin("Transformation property change")
-            self.source.set_child_property(prop, value)
-            self.app.action_log.commit()
+            with self.app.action_log.started("Transformation property change"):
+                self.source.set_child_property(prop, value)
             self._project.pipeline.commit_timeline()
             self.app.gui.viewer.target.overlay_stack.update(self.source)
 
diff --git a/pitivi/effects.py b/pitivi/effects.py
index 4ef38e9..cd84209 100644
--- a/pitivi/effects.py
+++ b/pitivi/effects.py
@@ -591,9 +591,8 @@ class EffectsPropertiesManager:
             value = Gst.Fraction(int(value.num), int(value.denom))
 
         if value != self._current_element_values.get(prop.name):
-            self.app.action_log.begin("Effect property change")
-            effect.set_child_property(prop.name, value)
-            self.app.action_log.commit()
+            with self.app.action_log.started("Effect property change"):
+                effect.set_child_property(prop.name, value)
 
             self.app.project_manager.current_project.pipeline.flushSeek()
             self._current_element_values[prop.name] = value
diff --git a/pitivi/medialibrary.py b/pitivi/medialibrary.py
index 4e089c3..3db82f0 100644
--- a/pitivi/medialibrary.py
+++ b/pitivi/medialibrary.py
@@ -541,13 +541,11 @@ class MediaLibraryWidget(Gtk.Box, Loggable):
         rows = [Gtk.TreeRowReference.new(model, path)
                 for path in paths]
 
-        self.app.action_log.begin("remove asset from media library")
-        for row in rows:
-            asset = model[row.get_path()][COL_ASSET]
-            self._project.remove_asset(asset)
-            self.app.gui.timeline_ui.purgeAsset(asset.props.id)
-
-        self.app.action_log.commit()
+        with self.app.action_log.started("remove asset from media library"):
+            for row in rows:
+                asset = model[row.get_path()][COL_ASSET]
+                self._project.remove_asset(asset)
+                self.app.gui.timeline_ui.purgeAsset(asset.props.id)
 
         # The treeview can make some of the remaining items selected, so
         # make sure none are selected.
diff --git a/pitivi/project.py b/pitivi/project.py
index fdaaedc..0ed89bc 100644
--- a/pitivi/project.py
+++ b/pitivi/project.py
@@ -1827,20 +1827,18 @@ class ProjectSettingsDialog(object):
         self.year_spinbutton.get_adjustment().set_value(year)
 
     def updateProject(self):
-        self.app.action_log.begin("change project settings")
-        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())
-
-        self.project.videowidth = int(self.width_spinbutton.get_value())
-        self.project.videoheight = int(self.height_spinbutton.get_value())
-        self.project.videopar = self.par_fraction_widget.getWidgetValue()
-        self.project.videorate = self.frame_rate_fraction_widget.getWidgetValue(
-        )
-
-        self.project.audiochannels = get_combo_value(self.channels_combo)
-        self.project.audiorate = get_combo_value(self.sample_rate_combo)
-        self.app.action_log.commit()
+        with self.app.action_log.started("change project settings"):
+            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())
+
+            self.project.videowidth = int(self.width_spinbutton.get_value())
+            self.project.videoheight = int(self.height_spinbutton.get_value())
+            self.project.videopar = self.par_fraction_widget.getWidgetValue()
+            self.project.videorate = self.frame_rate_fraction_widget.getWidgetValue()
+
+            self.project.audiochannels = get_combo_value(self.channels_combo)
+            self.project.audiorate = get_combo_value(self.sample_rate_combo)
 
     def _responseCb(self, unused_widget, response):
         """Handle the dialog being closed."""
diff --git a/pitivi/timeline/layer.py b/pitivi/timeline/layer.py
index 11ec4a5..0fed9d1 100644
--- a/pitivi/timeline/layer.py
+++ b/pitivi/timeline/layer.py
@@ -253,10 +253,9 @@ class LayerControls(Gtk.EventBox, Loggable):
         return menu_model, action_group
 
     def _deleteLayerCb(self, unused_action, unused_parametter):
-        self.app.action_log.begin("delete layer")
-        self.ges_timeline.remove_layer(self.ges_layer)
-        self.ges_timeline.get_asset().pipeline.commit_timeline()
-        self.app.action_log.commit()
+        with self.app.action_log.started("delete layer"):
+            self.ges_timeline.remove_layer(self.ges_layer)
+            self.ges_timeline.get_asset().pipeline.commit_timeline()
 
     def _moveLayerCb(self, unused_simple_action, unused_parametter, step):
         index = self.ges_layer.get_priority()
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index ac835bf..3ae342a 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -781,16 +781,15 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
 
             self.debug("Creating %s at %s", asset.props.id, Gst.TIME_ARGS(placement))
 
-            self.app.action_log.begin("add clip")
-            ges_clip = ges_layer.add_asset(asset,
-                                           placement,
-                                           0,
-                                           clip_duration,
-                                           asset.get_supported_formats())
-            placement += clip_duration
-            self.current_group.add(ges_clip.get_toplevel_parent())
-            self.selection.setSelection([], SELECT_ADD)
-            self.app.action_log.commit()
+            with self.app.action_log.started("add clip"):
+                ges_clip = ges_layer.add_asset(asset,
+                                               placement,
+                                               0,
+                                               clip_duration,
+                                               asset.get_supported_formats())
+                placement += clip_duration
+                self.current_group.add(ges_clip.get_toplevel_parent())
+                self.selection.setSelection([], SELECT_ADD)
             self._project.pipeline.commit_timeline()
 
             if not self.draggingElement:
@@ -1255,28 +1254,27 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         initial_position = self.__getInsertPosition(position)
         clip_position = initial_position
 
-        self.app.action_log.begin("add asset")
-        for obj in objs:
-            if isinstance(obj, GES.Clip):
-                obj.set_start(clip_position)
-                layer.add_clip(obj)
-                duration = obj.get_duration()
-            elif isinstance(obj, GES.Asset):
-                if obj.is_image():
-                    duration = self.app.settings.imageClipLength * \
-                        Gst.SECOND / 1000.0
-                else:
+        with self.app.action_log.started("add asset"):
+            for obj in objs:
+                if isinstance(obj, GES.Clip):
+                    obj.set_start(clip_position)
+                    layer.add_clip(obj)
                     duration = obj.get_duration()
+                elif isinstance(obj, GES.Asset):
+                    if obj.is_image():
+                        duration = self.app.settings.imageClipLength * \
+                            Gst.SECOND / 1000.0
+                    else:
+                        duration = obj.get_duration()
 
-                layer.add_asset(obj,
-                                start=clip_position,
-                                inpoint=0,
-                                duration=duration,
-                                track_types=obj.get_supported_formats())
-            else:
-                raise TimelineError("Cannot insert: %s" % type(obj))
-            clip_position += duration
-        self.app.action_log.commit()
+                    layer.add_asset(obj,
+                                    start=clip_position,
+                                    inpoint=0,
+                                    duration=duration,
+                                    track_types=obj.get_supported_formats())
+                else:
+                    raise TimelineError("Cannot insert: %s" % type(obj))
+                clip_position += duration
         self._project.pipeline.commit_timeline()
 
         if zoom_was_fitted:
@@ -1585,16 +1583,14 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
 
     def _deleteSelected(self, unused_action, unused_parameter):
         if self.ges_timeline:
-            self.app.action_log.begin("delete clip")
-
-            for clip in self.timeline.selection:
-                layer = clip.get_layer()
-                if isinstance(clip, GES.TransitionClip):
-                    continue
-                layer.remove_clip(clip)
+            with self.app.action_log.started("delete clip"):
+                for clip in self.timeline.selection:
+                    layer = clip.get_layer()
+                    if isinstance(clip, GES.TransitionClip):
+                        continue
+                    layer.remove_clip(clip)
 
-            self._project.pipeline.commit_timeline()
-            self.app.action_log.commit()
+                self._project.pipeline.commit_timeline()
 
             self.timeline.selection.setSelection([], SELECT)
 
@@ -1603,18 +1599,16 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
             self.info("No ges_timeline set yet!")
             return
 
-        self.app.action_log.begin("ungroup")
+        with self.app.action_log.started("ungroup"):
+            for obj in self.timeline.selection:
+                toplevel = obj.get_toplevel_parent()
+                if toplevel == self.timeline.current_group:
+                    for child in toplevel.get_children(False):
+                        child.ungroup(False)
 
-        for obj in self.timeline.selection:
-            toplevel = obj.get_toplevel_parent()
-            if toplevel == self.timeline.current_group:
-                for child in toplevel.get_children(False):
-                    child.ungroup(False)
-
-        self.timeline.resetSelectionGroup()
-        self.timeline.selection.setSelection([], SELECT)
+            self.timeline.resetSelectionGroup()
+            self.timeline.selection.setSelection([], SELECT)
 
-        self.app.action_log.commit()
         self._project.pipeline.commit_timeline()
 
     def _groupSelected(self, unused_action, unused_parameter):
@@ -1622,28 +1616,27 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
             self.info("No timeline set yet?")
             return
 
-        self.app.action_log.begin("group")
-        containers = set()
-        new_group = None
-        for obj in self.timeline.selection:
-            toplevel = obj.get_toplevel_parent()
-            if toplevel == self.timeline.current_group:
-                for child in toplevel.get_children(False):
-                    containers.add(child)
-                toplevel.ungroup(False)
-            else:
-                containers.add(toplevel)
+        with self.app.action_log.started("group"):
+            containers = set()
+            new_group = None
+            for obj in self.timeline.selection:
+                toplevel = obj.get_toplevel_parent()
+                if toplevel == self.timeline.current_group:
+                    for child in toplevel.get_children(False):
+                        containers.add(child)
+                    toplevel.ungroup(False)
+                else:
+                    containers.add(toplevel)
 
-        if containers:
-            new_group = GES.Container.group(list(containers))
+            if containers:
+                new_group = GES.Container.group(list(containers))
 
-        self.timeline.resetSelectionGroup()
+            self.timeline.resetSelectionGroup()
 
-        if new_group:
-            self.timeline.current_group.add(new_group)
+            if new_group:
+                self.timeline.current_group.add(new_group)
 
-        self._project.pipeline.commit_timeline()
-        self.app.action_log.commit()
+            self._project.pipeline.commit_timeline()
 
     def __copyClipsCb(self, unused_action, unused_parameter):
         if self.timeline.current_group:
@@ -1686,9 +1679,8 @@ 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.
         """
-        self.app.action_log.begin("split clip")
-        self._splitElements(self.timeline.selection.selected)
-        self.app.action_log.commit()
+        with self.app.action_log.started("split clip"):
+            self._splitElements(self.timeline.selection.selected)
 
         self.timeline.hideSnapBar()
         self._project.pipeline.commit_timeline()
diff --git a/pitivi/titleeditor.py b/pitivi/titleeditor.py
index d3812fc..07f1ffb 100644
--- a/pitivi/titleeditor.py
+++ b/pitivi/titleeditor.py
@@ -103,13 +103,12 @@ class TitleEditor(Loggable):
             self.settings["halignment"].append(en, n)
 
     def _setChildProperty(self, name, value):
-        self.app.action_log.begin("Title %s change" % name)
-        self._setting_props = True
-        try:
-            assert self.source.set_child_property(name, value)
-        finally:
-            self._setting_props = False
-        self.app.action_log.commit()
+        with self.app.action_log.started("Title %s change" % name):
+            self._setting_props = True
+            try:
+                assert self.source.set_child_property(name, value)
+            finally:
+                self._setting_props = False
 
     def _backgroundColorButtonCb(self, widget):
         color = gdk_rgba_to_argb(widget.get_rgba())
diff --git a/pitivi/undo/undo.py b/pitivi/undo/undo.py
index 690becc..dd6eb84 100644
--- a/pitivi/undo/undo.py
+++ b/pitivi/undo/undo.py
@@ -21,6 +21,7 @@
 """
 Base classes for undo/redo.
 """
+import contextlib
 import weakref
 
 from gi.repository import GObject
@@ -146,7 +147,19 @@ class UndoableActionLog(GObject.Object, Loggable):
         self.running = False
         self._checkpoint = self._takeSnapshot()
 
+    @contextlib.contextmanager
+    def started(self, action_group_name, finalizing_action=None):
+        """
+        Returns a context manager which commits the transaction at the end.
+        """
+        self.begin(action_group_name, finalizing_action)
+        yield
+        self.commit()
+
     def begin(self, action_group_name, finalizing_action=None):
+        """
+        Starts a transaction aka a high-level operation.
+        """
         if self.running:
             self.debug("Abort because running")
             return
@@ -158,6 +171,9 @@ class UndoableActionLog(GObject.Object, Loggable):
         self.emit("begin", stack)
 
     def push(self, action):
+        """
+        Adds an action to the current transaction.
+        """
         if action is not None:
             try:
                 st = action.asScenarioAction()
@@ -181,6 +197,9 @@ class UndoableActionLog(GObject.Object, Loggable):
         self.emit("push", stack, action)
 
     def rollback(self):
+        """
+        Forgets about the last started transaction.
+        """
         if self.running:
             self.debug("Ignore rollback because running")
             return
@@ -193,6 +212,9 @@ class UndoableActionLog(GObject.Object, Loggable):
         stack.undo()
 
     def commit(self):
+        """
+        Commits the last started transaction.
+        """
         if self.running:
             self.debug("Ignore commit because running")
             return
@@ -212,6 +234,9 @@ class UndoableActionLog(GObject.Object, Loggable):
         self.emit("commit", stack)
 
     def undo(self):
+        """
+        Undo the last recorded operation.
+        """
         if self.stacks:
             raise UndoWrongStateError("Recording a transaction")
         if not self.undo_stacks:
@@ -223,6 +248,9 @@ class UndoableActionLog(GObject.Object, Loggable):
         self.emit("undo", stack)
 
     def redo(self):
+        """
+        Redo the last undone operation.
+        """
         if self.stacks:
             raise UndoWrongStateError("Recording a transaction")
         if not self.redo_stacks:
@@ -265,6 +293,9 @@ class UndoableActionLog(GObject.Object, Loggable):
         return stack
 
     def is_in_transaction(self):
+        """
+        Whether currently recording an operation.
+        """
         return bool(self.stacks)