Merge branch 'syncpoints' into 'master'

[FIX] Improve synchronization points

Closes #381

See merge request GNOME/meld!80

Author: kaiw

help/C/syncpoints.page

@@ -0,0 +1,52 @@
+<page xmlns="http://projectmallard.org/1.0/"
+      type="topic"
+      id="syncpoints">
+<info>
+  <title type="sort">2</title>
+  <link type="guide" xref="index#file-mode"/>
+  <revision docversion="1.6" status="draft"/>
+  <include href="legal.xml" xmlns="http://www.w3.org/2001/XInclude"/>
+  <credit type="author copyright">
+    <name>Roberto Vidal</name>
+    <email>vidal.roberto.j@gmail.com</email>
+    <years>2022</years>
+  </credit>
+</info>
+
+<title>Synchronization Points</title>
+
+<p>
+Synchronization points help <app>Meld</app> perform a more fine-grained comparison between your files. When a synchronization point is added to each file, <app>Meld</app> effectively performs two comparisons: one for the chunk above the synchronization point and another for the one below.
+</p>
+
+<p>
+To add a synchronization point, click on a line and then right-click and select <gui style="menu">Add Synchronization Point</gui>. Repeat this with each file in your comparison: click on a line and then right-click and select <gui style="menu">Match Synchronization Point</gui>. Once every file has a synchronization point, they will match each other and the comparison will be updated to take them into account.
+</p>
+
+<p>
+You can add successive synchronization points and subdivide your files even further by repeating the steps above. Note that synchronization points are matched to each other in the order they appear in the text, which might not correspond to the order they were created.
+</p>
+
+<p>
+To remove all synchronization points from a comparison, right-click anywhere in the file and select <gui style="menu">Clear Synchronization Points</gui>.
+</p>
+
+
+<section id="dangling-syncpoints">
+<title>Dangling and matched syncpoints</title>
+
+<p>
+A synchronization point is "dangling" if it has not been matched yet to other files. For instance, if you have 1 synchronization point in each file, they are all matched together and affect the comparison. If you now add a synchronization point to the first file, it is dangling until we add a second synchronization point to the rest of the files.
+</p>
+
+<p>
+Dangling synchronization points can be moved around: click on a different line, and then right-click and select <gui style="menu">Move Synchronization Point</gui>. Dangling synchronization points can be removed: click on the line where it was set, right-click and select <gui style="menu">Remove Synchronization Point</gui>.
+</p>
+
+<p>
+Matched synchronization points can be removed as well. Note that removing a matched synchronization point also removes its siblings in the other files. To remove a matched synchronization point, click on the line where it is set, then right-click and select <gui style="menu">Remove Synchronization Point</gui>.
+</p>
+
+</section>
+
+</page>

meld/filediff.py

@@ -18,6 +18,7 @@
 import functools
 import logging
 import math
+from enum import Enum
 from typing import Optional, Tuple, Type
 
 from gi.repository import Gdk, Gio, GLib, GObject, Gtk, GtkSource
@@ -43,6 +44,7 @@
     BufferLines,
 )
 from meld.melddoc import ComparisonState, MeldDoc, open_files_external
+from meld.menuhelpers import replace_menu_section
 from meld.misc import user_critical, with_focused_pane
 from meld.patchdialog import PatchDialog
 from meld.recent import RecentType
@@ -281,7 +283,11 @@ def __init__(
         self._sync_hscroll_lock = False
         self.linediffer = self.differ()
         self.force_highlight = False
-        self.syncpoints = []
+
+        def get_mark_line(pane, mark):
+            return self.textbuffer[pane].get_iter_at_mark(mark).get_line()
+
+        self.syncpoints = Syncpoints(num_panes, get_mark_line)
         self.in_nested_textview_gutter_expose = False
         self._cached_match = CachedSequenceMatcher(self.scheduler)
 
@@ -313,6 +319,7 @@ def __init__(
         # Manually handle GAction additions
         actions = (
             ('add-sync-point', self.add_sync_point),
+            ('remove-sync-point', self.remove_sync_point),
             ('clear-sync-point', self.clear_sync_points),
             ('copy', self.action_copy),
             ('copy-full-path', self.action_copy_full_path),
@@ -359,8 +366,8 @@ def __init__(
 
         builder = Gtk.Builder.new_from_resource(
             '/org/gnome/meld/ui/filediff-menus.ui')
-        context_menu = builder.get_object('filediff-context-menu')
-        self.popup_menu = Gtk.Menu.new_from_model(context_menu)
+        self.popup_menu_model = builder.get_object('filediff-context-menu')
+        self.popup_menu = Gtk.Menu.new_from_model(self.popup_menu_model)
         self.popup_menu.attach_to_widget(self)
 
         builder = Gtk.Builder.new_from_resource(
@@ -1392,6 +1399,9 @@ def on_textview_popup_menu(self, textview):
         rect.x, rect.y = textview.buffer_to_window_coords(
             Gtk.TextWindowType.WIDGET, location.x, location.y)
 
+        pane = self.textview.index(textview)
+        self.set_syncpoint_menuitem(pane)
+
         self.popup_menu.popup_at_rect(
             Gtk.Widget.get_window(textview),
             rect,
@@ -1405,10 +1415,61 @@ def on_textview_popup_menu(self, textview):
     def on_textview_button_press_event(self, textview, event):
         if event.button == 3:
             textview.grab_focus()
+            pane = self.textview.index(textview)
+            self.set_syncpoint_menuitem(pane)
             self.popup_menu.popup_at_pointer(event)
             return True
         return False
 
+    def set_syncpoint_menuitem(self, pane):
+        menu_actions = {
+            SyncpointAction.ADD: [
+                _("Add Synchronization Point"),
+                "view.add-sync-point"
+            ],
+            SyncpointAction.DELETE: [
+                _("Remove Synchronization Point"),
+                "view.remove-sync-point"
+            ],
+            SyncpointAction.MOVE: [
+                _("Move Synchronization Point"),
+                "view.add-sync-point"
+            ],
+            SyncpointAction.MATCH: [
+                _("Match Synchronization Point"),
+                "view.add-sync-point"
+            ],
+            SyncpointAction.DISABLED: [
+                _("Add Synchronization Point"),
+                "view.add-sync-point"
+            ],
+        }
+
+        def get_mark():
+            return self.textbuffer[pane].get_insert()
+
+        action = self.syncpoints.action(pane, get_mark)
+
+        self.set_action_enabled(
+            "add-sync-point",
+            action != SyncpointAction.DISABLED
+        )
+
+        label, action_id = menu_actions[action]
+
+        syncpoint_menu = Gio.Menu()
+        syncpoint_menu.append(label=label, detailed_action=action_id)
+        syncpoint_menu.append(
+            label=_("Clear Synchronization Points"),
+            detailed_action='view.clear-sync-point',
+        )
+        section = Gio.MenuItem.new_section(None, syncpoint_menu)
+        section.set_attribute([("id", "s", "syncpoint-section")])
+        replace_menu_section(self.popup_menu_model, section)
+
+        self.popup_menu = Gtk.Menu.new_from_model(self.popup_menu_model)
+        self.popup_menu.attach_to_widget(self)
+
     def set_labels(self, labels):
         labels = labels[:self.num_panes]
         for label, buf in zip(labels, self.textbuffer):
@@ -2411,18 +2472,24 @@ def delete_chunk(self, src, chunk):
 
     @with_focused_pane
     def add_sync_point(self, pane, *args):
-        # Find a non-complete syncpoint, or create a new one
-        if self.syncpoints and None in self.syncpoints[-1]:
-            syncpoint = self.syncpoints.pop()
-        else:
-            syncpoint = [None] * self.num_panes
         cursor_it = self.textbuffer[pane].get_iter_at_mark(
             self.textbuffer[pane].get_insert())
-        syncpoint[pane] = self.textbuffer[pane].create_mark(None, cursor_it)
-        self.syncpoints.append(syncpoint)
 
+        self.syncpoints.add(
+            pane,
+            self.textbuffer[pane].create_mark(None, cursor_it)
+        )
+
+        self.refresh_sync_points()
+
+    @with_focused_pane
+    def remove_sync_point(self, pane, *args):
+        self.syncpoints.remove(pane, self.textbuffer[pane].get_insert())
+        self.refresh_sync_points()
+
+    def refresh_sync_points(self):
         for i, t in enumerate(self.textview[:self.num_panes]):
-            t.syncpoints = [p[i] for p in self.syncpoints if p[i] is not None]
+            t.syncpoints = self.syncpoints.points(i)
 
         def make_line_retriever(pane, marks):
             buf = self.textbuffer[pane]
@@ -2432,7 +2499,8 @@ def get_line_for_mark():
                 return buf.get_iter_at_mark(mark).get_line()
             return get_line_for_mark
 
-        valid_points = [p for p in self.syncpoints if all(p)]
+        valid_points = self.syncpoints.valid_points()
+
         if valid_points and self.num_panes == 2:
             self.linediffer.syncpoints = [
                 ((make_line_retriever(1, p), make_line_retriever(0, p)), )
@@ -2444,6 +2512,8 @@ def get_line_for_mark():
                  (make_line_retriever(1, p), make_line_retriever(2, p)))
                 for p in valid_points
             ]
+        elif not valid_points:
+            self.linediffer.syncpoints = []
 
         if valid_points:
             for mgr in self.msgarea_mgr:
@@ -2460,7 +2530,7 @@ def get_line_for_mark():
         self.refresh_comparison()
 
     def clear_sync_points(self, *args):
-        self.syncpoints = []
+        self.syncpoints.clear()
         self.linediffer.syncpoints = []
         for t in self.textview:
             t.syncpoints = []
@@ -2471,3 +2541,135 @@ def clear_sync_points(self, *args):
 
 
 FileDiff.set_css_name('meld-file-diff')
+
+
+class SyncpointAction(Enum):
+    # A dangling syncpoint can be moved to the line
+    MOVE = "move"
+    # A dangling syncpoint sits can be remove from this line
+    DELETE = "delete"
+    # A syncpoint can be added to this line to match existing ones
+    # in other panes
+    MATCH = "match"
+    # A new, dangling syncpoint can be added to this line
+    ADD = "add"
+    # No syncpoint-related action can be taken on this line
+    DISABLED = "disabled"
+
+
+class Syncpoints:
+    def __init__(self, num_panes: int, comparator):
+        self._num_panes = num_panes
+        self._points = [[] for _i in range(0, num_panes)]
+        self._comparator = comparator
+
+    def add(self, pane_idx: int, point):
+        pane_state = self._pane_state(pane_idx)
+
+        if pane_state == self.PaneState.DANGLING:
+            self._points[pane_idx].pop()
+
+        self._points[pane_idx].append(point)
+
+        lengths = set(len(p) for p in self._points)
+
+        if len(lengths) == 1:
+            for (i, p) in enumerate(self._points):
+                p.sort(key=lambda point: self._comparator(i, point))
+
+    def remove(self, pane_idx: int, cursor_point):
+        cursor_key = self._comparator(pane_idx, cursor_point)
+
+        index = -1
+
+        for (i, point) in enumerate(self._points[pane_idx]):
+            if self._comparator(pane_idx, point) == cursor_key:
+                index = i
+                break
+
+        assert index is not None
+
+        pane_state = self._pane_state(pane_idx)
+
+        assert pane_state != self.PaneState.SHORT
+
+        if pane_state == self.PaneState.MATCHED:
+            for pane in self._points:
+                pane.pop(index)
+        elif pane_state == self.PaneState.DANGLING:
+            self._points[pane_idx].pop()
+
+    def clear(self):
+        self._points = [[] for _i in range(0, self._num_panes)]
+
+    def points(self, pane_idx: int):
+        return self._points[pane_idx].copy()
+
+    def valid_points(self):
+        num_matched = min(len(p) for p in self._points)
+
+        if not num_matched:
+            return []
+
+        matched = [p[:num_matched] for p in self._points]
+
+        return [
+            tuple(matched_point[i] for matched_point in matched)
+            for i in range(0, num_matched)
+        ]
+
+    def _pane_state(self, pane_idx: int):
+        lengths = set(len(points) for points in self._points)
+
+        if len(lengths) == 1:
+            return self.PaneState.MATCHED
+
+        if len(self._points[pane_idx]) == min(lengths):
+            return self.PaneState.SHORT
+        else:
+            return self.PaneState.DANGLING
+
+    def action(self, pane_idx: int, get_mark):
+        state = self._pane_state(pane_idx)
+
+        if state == self.PaneState.SHORT:
+            return SyncpointAction.MATCH
+
+        target = self._comparator(pane_idx, get_mark())
+
+        points = self._points[pane_idx]
+
+        if state == self.PaneState.MATCHED:
+            is_syncpoint = any(
+                self._comparator(pane_idx, point) == target
+                for point in points
+            )
+
+            if is_syncpoint:
+                return SyncpointAction.DELETE
+            else:
+                return SyncpointAction.ADD
+
+        # state == DANGLING
+        if target == self._comparator(pane_idx, points[-1]):
+            return SyncpointAction.DELETE
+
+        is_syncpoint = any(
+            self._comparator(pane_idx, point) == target
+            for point in points
+        )
+
+        if is_syncpoint:
+            return SyncpointAction.DISABLED
+        else:
+            return SyncpointAction.MOVE
+
+    class PaneState(Enum):
+        # The state of a pane with all its syncpoints matched
+        MATCHED = "matched"
+        # The state of a pane waiting to be matched to existing syncpoints
+        # in other panes
+        SHORT = "short"
+        # The state of a pane with a dangling syncpoint, not yet matched
+        # across all panes
+        DANGLING = "DANGLING"

meld/resources/gtk/menus.ui

@@ -86,17 +86,6 @@
             <attribute name="action">view.merge-all</attribute>
           </item>
         </section>
-        <section>
-          <attribute name="id">synchronisation-section</attribute>
-          <item>
-            <attribute name="label" translatable="yes">Add Synchronization Point</attribute>
-            <attribute name="action">view.add-sync-point</attribute>
-          </item>
-          <item>
-            <attribute name="label" translatable="yes">Clear Synchronization Points</attribute>
-            <attribute name="action">view.clear-sync-point</attribute>
-          </item>
-        </section>
         <section>
           <attribute name="id">tool-section</attribute>
           <item>