After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 699033 - in conflict merge view, display BASE in central pane where changes conflict
in conflict merge view, display BASE in central pane where changes conflict
Status: RESOLVED FIXED
Product: meld
Classification: Other
Component: version
git master
Other Linux
: Normal normal
: ---
Assigned To: meld-maint
meld-maint
Depends on:
Blocks:
 
 
Reported: 2013-04-27 12:48 UTC by Adam Dingle
Modified: 2015-10-04 15:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch reconstructing and filtering diff3 from Git (3.19 KB, patch)
2013-05-25 22:13 UTC, Kai Willadsen
none Details | Review

Description Adam Dingle 2013-04-27 12:48:23 UTC
Currently Meld's conflict merge view displays MERGED in the central pane.  This is the result of the version control system's attempt to merge the conflicting files.  If I use git mergetool to invoke Meld, I see BASE in the central pane instead.  That's the base revision that the merging branches split from.

On the Meld mailing list there have been discussions about whether BASE or MERGED is better to display here.  Kai prefers MERGED.  I thought I preferred MERGED too, and I said so on the mailing list a couple of days ago.  But I was resolving a conflict in Meld this morning and realized I really wanted to see BASE instead.  The problem with displaying MERGED is that there's no way to know what the file looked like before it was changed in either branch.  And sometimes that's highly relevant.

So now I've flipped my opinion to prefer BASE.  If others might still prefer MERGED, then this could be worthy of a preference.  (If we decide to display BASE all the time, then bug 699030 to put LOCAL on the left is no longer relevant, by the way.)
Comment 1 Adam Dingle 2013-04-27 19:03:20 UTC
I take back my last parenthetical comment: even if we show BASE, bug 699030 is still relevant since it would be nice to match the order in which the panes appear in git mergetool.
Comment 2 Kai Willadsen 2013-04-28 00:01:37 UTC
I understand why you want to see BASE, but I feel that the only situation in which BASE is relevant is when you have a conflict; this is when you actually want the additional information.

As far as I'm concerned, the right thing to do is to display MERGED everywhere that isn't a conflict, and display BASE where there is a conflict. This is approximately equivalent to making MERGED use diff3-style conflict markers and then removing the LOCAL and REMOTE parts of the conflict (since we're already showing those).

The other option here is to load LOCAL/BASE/REMOTE and then use our own auto-merge to create what I describe above. I don't like this, as I think that modern VCs tend to do a better job than we ever can in merging, even if only because they have more information to hand.
Comment 3 Adam Dingle 2013-04-28 01:21:53 UTC
Your suggestion sounds great to me if you can make it work.  :)
Comment 4 Adam Dingle 2013-05-01 14:57:37 UTC
(In reply to comment #2)
> The other option here is to load LOCAL/BASE/REMOTE and then use our own
> auto-merge to create what I describe above. I don't like this, as I think that
> modern VCs tend to do a better job than we ever can in merging, even if only
> because they have more information to hand.

We have BASE, LOCAL and REMOTE to work with.  Out of curiosity, what additional information might version control systems have and take advantage of when merging?
Comment 5 Kai Willadsen 2013-05-25 22:13:03 UTC
Created attachment 245312 [details] [review]
Patch reconstructing and filtering diff3 from Git

This is a bit of a proof-of-concept patch that implements what I described above. If you find a conflict and invoke the conflict comparison *from Meld* (i.e., this won't work when using git mergetool) then we show BASE in the middle pane for any conflicting sections.

There's no intrinsic reason why this can't be used for any diff3 format file, but we have to know that that's a thing we should try to do, which kind of means knowing that we're being invoked to resolve a conflict, and not just to do a three-way comparison.

The answer to your question about why I'd rather not use our merge algorithm is simply that VCs have invested a lot more time getting good merge algorithms working than we have, and I'd like to trust them where possible. Also, octopus merges, not that we handle those anyway.
Comment 6 Adam Dingle 2013-05-26 15:48:07 UTC
Exciting!  You said this is a proof of concept - is this not yet ready for prime time?  Should I run with this patch so I can see how this holds up the next time I have to resolve a bunch of merge conflicts, or should I wait?
Comment 7 Kai Willadsen 2013-06-14 22:53:57 UTC
Running with this patch would be great!

It's proof-of-concept in so far as there are probably edge cases that I haven't thought about yet, but it seems to work as expected for me. The other big problem is our lack of support for this in git mergetool. I haven't even started to think about how to make that work yet.
Comment 8 Adam Dingle 2013-06-14 23:00:47 UTC
OK - I'll run Meld with this patch applied if I can.  It's been a while since I've had to resolve a bunch of merge conflicts, but the day will come.  :)

Once we commit this patch and also fix bug 698645 (conflict merge tool should mark conflict as resolved) I won't care so much about git mergetool, since I should generally be able to resolve conflicts via Meld.
Comment 9 Kai Willadsen 2015-10-04 03:08:42 UTC
I have no idea why I've procrastinated on merging this for so long, but this is finally in current master. Thanks for the suggestion.

It's worth noting that the current default `git mergetool` behaviour for Meld (which ships with git, so nothing I can do about it) will still show local/base/remote for merges. This new behaviour is basically just an enhancement to Meld's merging display when you view a conflict from inside Meld. Git *could* update to use this for its mergetool, but we'll see how that goes.
Comment 10 Adam Dingle 2015-10-04 15:02:59 UTC
Great to see this - thanks!