Re: "static/fixed" mode on file diff view

[Pedro Pedruzzi <pedro pedruzzi gmail com>]

On Fri, Mar 1, 2013 at 5:49 PM, Kai Willadsen <kai willadsen gmail com> wrote:
> On 26 February 2013 06:58, Pedro Pedruzzi <pedro pedruzzi gmail com> wrote:
> > Hello, everyone!
> >
> > I'm a happy meld user and I've just joined this list because I have
> > some improvement ideas I'd like to share, discuss and help implement.
> >
> > The current visual highlighting of matching chunks is awesome. But the
> > scrolling can get a little confusing to use because it lacks
> > one-to-one alignment between lines in the panes.
> >
> > I'd like to experiment adding an option to make the diff view
> > scrolling fixed between panes. I'd have to add vertical spacing in the
> > panes to fill the gaps (can be done with "null" lines), so that
> > identical lines are always aligned (regardless of the scrolling
> > position) and so are matching chunks.
> >
> > What do you guys think of this idea?
>
> Plenty of people have requested this before, and I'm pretty sure
> there's a bug somewhere. It would be nice to have, but it's not all
> that easy to add.
>
> > I'd appreciate some guidance on how this could be hacked in the code base.
>
> It will be difficult to know what the best approach is until you're
> half way through. There are two basic options that come to mind: add
> extra linebreaks into the buffer, but make sure to account for them
> whenever handling text; or try to get the textview to do the extra
> padding for you. Both are probably broken in different ways (e.g.,
> line wrapping).
>
> For the first option, Meld has a strong assumption that the textbuffer
> isn't going to lie to us about line numbers. We assume that line 7 in
> our comparison will be line 7 in the buffer and vice versa. Fixing
> this would be a big undertaking, and would have all sorts of edge
> cases, but if done properly would be very worthwhile.
>
> The second option is to get the gtk.TextView to artificially inflate
> the last line of each misaligned chunk. This *should* be significantly
> easier, but not trivial to do properly. Basically, you'd create a tag
> per chunk and per pane, and iterate over chunks, checking to see
> whether their ending text iters y-locations match. If not, adjust the
> relevant tag's pixels-below-lines & pixels-below-lines-set properties
> to make them match. The hard part comes when the buffer changes and
> the tags need to be cleaned, adjusted, revalidated, etc. As a
> proof-of-concept, the first part could be done in
> FileDiff.on_textview_expose() I think, but in the long term I think
> doing the tag manipulation in there would be too slow. I haven't tried
> this, but I think it should work.
>
> Anyway, it would be cool to have this option, so if you have any
> questions, feel free to ask.

Thanks for the pointers.

I've came up with this proof-of-concept based on the second option.
The tags isn't getting updated on buffer change yet. This is just to
demonstrate how would it look and feel.

Please, tell me what you think.

I have some questions:

1. Where is a good place to do the tag creation? I tried somewhere in
the constructor but it seems that the linediffer was not ready to
iterate over changes yet.

2. How about 3 way filediffs? I would need to iterate over chunks on
the 3 panes.

The patch is also available on github if you prefer:

https://github.com/pedrox/meld/commit/9bc4113b8b128c9053b7cb3509a6b4437049a47b

Regards,
Pedro Pedruzzi

-- >8 --
Subject: [PATCH] Proof-of-concept: Make filediff panes aligned and with the
 same height by adding extra padding

---
 meld/filediff.py | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/meld/filediff.py b/meld/filediff.py
index 36220c1..587d016 100644
--- a/meld/filediff.py
+++ b/meld/filediff.py
@@ -212,9 +212,14 @@ class FileDiff(melddoc.MeldDoc, gnomeglade.Component):
         self._cached_match = CachedSequenceMatcher()
         self.anim_source_id = [None for buf in self.textbuffer]
         self.animating_chunks = [[] for buf in self.textbuffer]
+        self._nopad = 0
         for buf in self.textbuffer:
             buf.create_tag("inline")

+        # self.linediffer.single_changes(0) is still empty at this point.
+        for change in self.linediffer.single_changes(0):
+            print(change)
+
         actions = (
             ("MakePatch", None, _("Format as patch..."), None,
_("Create a patch using differences between files"), self.make_patch),
             ("PrevConflict", None, _("Previous conflict"), "<Ctrl>I",
_("Go to the previous conflict"), lambda x:
self.on_next_conflict(gtk.gdk.SCROLL_UP)),
@@ -1271,6 +1276,28 @@ class FileDiff(melddoc.MeldDoc, gnomeglade.Component):
             self.text_filters = []
             self.refresh_comparison()

+    def xxx_init_paddings(self):
+        if self._nopad:
+            return
+
+        sumdy = 0
+
+        for change in self.linediffer.single_changes(0):
+            self._nopad = 1
+            dy = self.textview[0].get_y_for_line_num(change[2] - 1) -
self.textview[1].get_y_for_line_num(change[4] - 1) - sumdy
+            sumdy = sumdy + dy
+
+            if dy >= 0:
+                panepad = 1
+                padline = change[4] - 1
+            else:
+                panepad = 0
+                padline = change[2] - 1
+                dy = -dy
+
+            tag = self.textbuffer[panepad].create_tag(**{
"pixels-below-lines": dy, "pixels-below-lines-set": True })
+            self.textbuffer[panepad].apply_tag(tag,
self.textbuffer[panepad].get_iter_at_line(padline),
self.textbuffer[panepad].get_iter_at_line(padline + 1))
+
     def on_textview_expose_event(self, textview, event):
         if self.num_panes == 1:
             return
@@ -1299,6 +1326,8 @@ class FileDiff(melddoc.MeldDoc, gnomeglade.Component):
         context.clip()
         context.set_line_width(1.0)

+        self.xxx_init_paddings()
+
         for change in self.linediffer.single_changes(pane, bounds):
             ypos0 = textview.get_y_for_line_num(change[1]) - visible.y
             ypos1 = textview.get_y_for_line_num(change[2]) - visible.y
@@ -1551,7 +1580,14 @@ class FileDiff(melddoc.MeldDoc, gnomeglade.Component):
         if self._sync_vscroll_lock:
             return

-        if not self._scroll_lock and (self.keymask & MASK_SHIFT) == 0:
+        # scrollbar influence 0->1->2 or 0<-1->2 or 0<-1<-2
+        scrollbar_influence = ((1, 2), (0, 2), (1, 0))
+
+        if 1: # FIXME: aligned mode
+            # all the scrollbars get the same raw adjustment
+            for i in scrollbar_influence[master][:self.num_panes - 1]:
+
self.scrolledwindow[i].get_vadjustment().set_value(adjustment.value)
+        elif not self._scroll_lock and (self.keymask & MASK_SHIFT) == 0:
             self._sync_vscroll_lock = True
             syncpoint = 0.5

@@ -1561,9 +1597,6 @@ class FileDiff(melddoc.MeldDoc, gnomeglade.Component):
             line_y, height = self.textview[master].get_line_yrange(it)
             line = it.get_line() + ((master_y-line_y)/height)

-            # scrollbar influence 0->1->2 or 0<-1->2 or 0<-1<-2
-            scrollbar_influence = ((1, 2), (0, 2), (1, 0))
-
             for i in scrollbar_influence[master][:self.num_panes - 1]:
                 adj = self.scrolledwindow[i].get_vadjustment()
                 mbegin, mend = 0, self.textbuffer[master].get_line_count()
--
1.8.2.rc1