[meld: 5/7] filediff: Handle three-way edge case in viewport-relative scrolling

[Kai Willadsen <kaiw src gnome org>]

commit 78db00c2d56a339f945b8115365f7f476b5546ce
Author: Kai Willadsen <kai willadsen gmail com>
Date:   Sun Nov 3 10:39:45 2019 +1000

    filediff: Handle three-way edge case in viewport-relative scrolling
    
    The scenario here is slightly odd, but in some cases in three-way
    comparisons, using the viewport-relative scroll gets stuck going
    between two chunks in the middle. What actually happens is that the
    scroll down doesn't bring the target to above 50% of the screen
    vertically, and so when looking for the "next" chunk from the middle of
    the viewport, we either get the current chunk, or in some weirder cases
    the previous chunk.
    
    The fix in this patch is to only do viewport-relative scrolling if the
    target non-viewport-relative chunk isn't already in the viewport,
    falling back to just using the existing logic. This seems to work well
    in practice, although it's possible that a future enhancement would be
    to do the is-the-chunk-in-the-area check with a restricted area (say
    the middle 50% of the viewport) rather than the whole viewport.

 meld/filediff.py | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)
---
diff --git a/meld/filediff.py b/meld/filediff.py
index 75f1c7d2..4ac84fc9 100644
--- a/meld/filediff.py
+++ b/meld/filediff.py
@@ -17,6 +17,7 @@
 import copy
 import functools
 import math
+from typing import Optional
 
 from gi.repository import Gdk
 from gi.repository import Gio
@@ -601,6 +602,17 @@ class FileDiff(Gtk.VBox, MeldDoc):
     def on_linkmap_scroll_event(self, linkmap, event):
         self.next_diff(event.direction, use_viewport=True)
 
+    def _is_chunk_in_area(
+            self, chunk_id: Optional[int], pane: int, area: Gdk.Rectangle):
+
+        if chunk_id is None:
+            return False
+
+        chunk = self.linediffer.get_chunk(chunk_id, pane)
+        target_iter = self.textbuffer[pane].get_iter_at_line(chunk.start_a)
+        target_y, _height = self.textview[pane].get_line_yrange(target_iter)
+        return area.y <= target_y <= area.y + area.height
+
     def next_diff(self, direction, centered=False, use_viewport=False):
         # use_viewport: seek next and previous diffes based on where
         # the user is currently scrolling at.
@@ -608,23 +620,20 @@ class FileDiff(Gtk.VBox, MeldDoc):
         target = self.cursor.next if scroll_down else self.cursor.prev
 
         if use_viewport:
-
-            if target is None:
-                return
-
             pane = self.cursor.pane
             text_area = self.textview[pane].get_visible_rect()
 
-            chunk = self.linediffer.get_chunk(target, pane)
-            if not chunk:
-                return
-
-            halfscreen = text_area.y + text_area.height / 2
-            halfline = self.textview[pane].get_line_at_y(
-                halfscreen).target_iter.get_line()
-
-            (current, prev, next_) = self.linediffer.locate_chunk(1, halfline)
-            target = next_ if scroll_down else prev
+            # Only do viewport-relative calculations if the chunk we'd
+            # otherwise scroll to is *not* on screen. This avoids 3-way
+            # comparison cases where scrolling won't go past a chunk
+            # because the scroll doesn't go past 50% of the screen.
+            if not self._is_chunk_in_area(target, pane, text_area):
+                halfscreen = text_area.y + text_area.height / 2
+                halfline = self.textview[pane].get_line_at_y(
+                    halfscreen).target_iter.get_line()
+
+                _, prev, next_ = self.linediffer.locate_chunk(1, halfline)
+                target = next_ if scroll_down else prev
 
         self.go_to_chunk(target, centered=centered)