[pitivi] timeline: Allow snapping clips to previous/next clip

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

commit 1a5b8b290d43b876c0674d3eb5fe5e582bacdb1f
Author: will_swiston <wswiston gmail com>
Date:   Fri Apr 30 23:40:34 2021 -0500

    timeline: Allow snapping clips to previous/next clip
    
    Fixes #2470

 .pre-commit-config.yaml         |  4 ++-
 pitivi/timeline/timeline.py     | 47 ++++++++++++++++++++++--
 tests/test_timeline_timeline.py | 79 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+), 4 deletions(-)
---
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index eb00985e1..7a08cad55 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -44,7 +44,9 @@ repos:
     rev: 'v0.800'
     hooks:
       - id: mypy
-        files: '.*pitivi/clipproperties.py$'
+        files: '^pitivi/(clipproperties.py|timeline/timeline.py)$'
+        args:
+          - --no-strict-optional
   - repo: local
     hooks:
       - id: pylint
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index acae28f89..100b24337 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -16,6 +16,7 @@
 # License along with this program; if not, see <http://www.gnu.org/licenses/>.
 import os
 from gettext import gettext as _
+from typing import List
 from typing import Optional
 
 from gi.repository import Gdk
@@ -1446,7 +1447,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         Gtk.Grid.__init__(self)
         Loggable.__init__(self)
 
-        self.app = app
+        self.app: Gtk.Application = app
         self.editor_state = editor_state
         self._settings = self.app.settings
         self.shift_mask = False
@@ -1792,6 +1793,20 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
                                self.shift_backward_action,
                                _("Shift selected clips one frame backward"))
 
+        self.snap_clips_forward_action = Gio.SimpleAction.new("snap-clips-forward", None)
+        self.snap_clips_forward_action.connect("activate", self._snap_clips_forward_cb)
+        group.add_action(self.snap_clips_forward_action)
+        self.app.shortcuts.add("timeline.snap-clips-forward", ["<Alt>s"],
+                               self.snap_clips_forward_action,
+                               _("Snap selected clips to the next clip"))
+
+        self.snap_clips_backward_action = Gio.SimpleAction.new("snap-clips-backward", None)
+        self.snap_clips_backward_action.connect("activate", self._snap_clips_backward_cb)
+        group.add_action(self.snap_clips_backward_action)
+        self.app.shortcuts.add("timeline.snap-clips-backward", ["<Alt>a"],
+                               self.snap_clips_backward_action,
+                               _("Snap selected clips to the previous clip"))
+
         self.add_effect_action = Gio.SimpleAction.new("add-effect", None)
         self.add_effect_action.connect("activate", self.__add_effect_cb)
         group.add_action(self.add_effect_action)
@@ -2039,7 +2054,7 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
             priority = len(self.ges_timeline.get_layers())
             self.timeline.create_layer(priority)
 
-    def first_clip_edge(self, before=None, after=None):
+    def first_clip_edge(self, layers: Optional[List[GES.Layer]] = None, before: Optional[int] = None, after: 
Optional[int] = None) -> Optional[int]:
         assert (after is not None) != (before is not None)
 
         if after is not None:
@@ -2054,7 +2069,9 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
         if start >= end:
             return None
 
-        for layer in self.ges_timeline.layers:
+        if not layers:
+            layers = self.ges_timeline.layers
+        for layer in layers:
             clips = layer.get_clips_in_interval(start, end)
             for clip in clips:
                 if clip.start > start:
@@ -2232,6 +2249,30 @@ class TimelineContainer(Gtk.Grid, Zoomable, Loggable):
             self.ges_timeline.set_snapping_distance(previous_snapping_distance)
         self.app.action_log.commit("Shift clip delta frames")
 
+    def _snap_clips_forward_cb(self, action, parameter):
+        self.snap_clips(forward=True)
+
+    def _snap_clips_backward_cb(self, action, parameter):
+        self.snap_clips(forward=False)
+
+    def snap_clips(self, forward: bool):
+        """Snap clips to next or previous clip."""
+        clips = list(self.timeline.selection.selected)
+        clips.sort(key=lambda clip: clip.start, reverse=forward)
+        with self.app.action_log.started("Snaps to closest clip",
+                                         
finalizing_action=CommitTimelineFinalizingAction(self._project.pipeline),
+                                         toplevel=True):
+            for clip in clips:
+                layer = clip.props.layer
+                if not forward:
+                    position = self.first_clip_edge(layers=[layer], before=clip.start)
+                else:
+                    position = self.first_clip_edge(layers=[layer], after=clip.start + clip.duration)
+                    if position is not None:
+                        position -= clip.duration
+                if position is not None:
+                    clip.set_start(position)
+
     def do_focus_in_event(self, unused_event):
         self.log("Timeline has grabbed focus")
         self.update_actions()
diff --git a/tests/test_timeline_timeline.py b/tests/test_timeline_timeline.py
index ac9d2c27e..d90393872 100644
--- a/tests/test_timeline_timeline.py
+++ b/tests/test_timeline_timeline.py
@@ -944,3 +944,82 @@ class TestKeyboardShiftClips(common.TestCase):
         self.assertEqual(10 * Gst.SECOND, ges_clip2.start)
         self.assertEqual(13 * Gst.SECOND, ges_clip3.start)
         self.assertEqual(15 * Gst.SECOND, ges_clip4.start)
+
+
+class TestSnapClips(common.TestCase):
+
+    @common.setup_timeline
+    def test_snap_clip_single_layer_single_clip(self):
+        """Test whether a single clip is able to snap right, and left to an adjacent clip."""
+        ges_clip1 = self.add_clip(self.layer, 5 * Gst.SECOND, duration=2 * Gst.SECOND)
+        ges_clip2 = self.add_clip(self.layer, 9 * Gst.SECOND, duration=2 * Gst.SECOND)
+
+        self.toggle_clip_selection(ges_clip1, expect_selected=True)
+
+        self.timeline_container.snap_clips_forward_action.emit("activate", None)
+        self.assertEqual(ges_clip1.start, ges_clip2.start - ges_clip1.duration)
+
+        self.timeline_container.snap_clips_backward_action.emit("activate", None)
+        self.assertEqual(ges_clip1.start, 0)
+
+    @common.setup_timeline
+    def test_snap_single_layer_multiple_clips_adjacent(self):
+        """Tests whether a single clip can snap to multiple adjacent clips."""
+        ges_clip1 = self.add_clip(self.layer, 5 * Gst.SECOND, duration=2 * Gst.SECOND)
+        ges_clip2 = self.add_clip(self.layer, 7 * Gst.SECOND, duration=2 * Gst.SECOND)
+        ges_clip3 = self.add_clip(self.layer, 11 * Gst.SECOND, duration=2 * Gst.SECOND)
+
+        event = mock.Mock()
+        event.keyval = Gdk.KEY_Control_L
+        self.timeline_container.do_key_press_event(event)
+        self.toggle_clip_selection(ges_clip1, expect_selected=True)
+        self.toggle_clip_selection(ges_clip2, expect_selected=True)
+
+        self.timeline_container.snap_clips_forward_action.emit("activate", None)
+        self.assertEqual(ges_clip2.start, ges_clip3.start - ges_clip2.duration)
+        self.assertEqual(ges_clip1.start, ges_clip2.start - ges_clip1.duration)
+
+        self.timeline_container.snap_clips_backward_action.emit("activate", None)
+        self.assertEqual(ges_clip1.start, 0)
+        self.assertEqual(ges_clip2.start, ges_clip1.start + ges_clip1.duration)
+
+    @common.setup_timeline
+    def test_snap_multiple_layers_not_affected_by_other_layer(self):
+        """Tests whether a clip snap is affected by a clip in another layer."""
+        layer2 = self.timeline.append_layer()
+        self.add_clip(self.layer, 5 * Gst.SECOND, duration=2 * Gst.SECOND)
+        self.add_clip(self.layer, 7 * Gst.SECOND, duration=2 * Gst.SECOND)
+        self.add_clip(self.layer, 11 * Gst.SECOND, duration=2 * Gst.SECOND)
+        clip = self.add_clip(layer2, 5 * Gst.SECOND, duration=2 * Gst.SECOND)
+        end_clip = self.add_clip(layer2, 30 * Gst.SECOND, duration=2 * Gst.SECOND)
+
+        self.toggle_clip_selection(clip, expect_selected=True)
+
+        self.timeline_container.snap_clips_forward_action.emit("activate", None)
+        self.assertEqual(clip.start, end_clip.start - clip.duration)
+
+        self.timeline_container.snap_clips_backward_action.emit("activate", None)
+        self.assertEqual(clip.start, 0)
+
+    @common.setup_timeline
+    def test_single_clip_snap_right_does_nothing(self):
+        """Tests whether the last clip snapped forward remains in place."""
+        ges_clip = self.add_clip(self.layer, 5 * Gst.SECOND, duration=2 * Gst.SECOND)
+        clip_start = ges_clip.start
+
+        self.toggle_clip_selection(ges_clip, expect_selected=True)
+
+        self.timeline_container.snap_clips_forward_action.emit("activate", None)
+        self.assertEqual(ges_clip.start, clip_start)
+
+    @common.setup_timeline
+    def test_clip_snaps_to_timeline_duration(self):
+        """Tests whether a clip snaps to the timeline duration."""
+        layer2 = self.timeline.append_layer()
+        self.add_clip(self.layer, 30 * Gst.SECOND, duration=2 * Gst.SECOND)
+        ges_clip = self.add_clip(layer2, 5 * Gst.SECOND, duration=2 * Gst.SECOND)
+
+        self.toggle_clip_selection(ges_clip, expect_selected=True)
+
+        self.timeline_container.snap_clips_forward_action.emit("activate", None)
+        self.assertEqual(ges_clip.start, self.timeline.get_duration() - ges_clip.duration)