Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views

On Thu, Dec 17, 2020 at 08:49:13PM -0600, Felipe Contreras wrote:

I'm not worried about Junio but I am wondering if anyone has managed to change your mind. You and I have been going back and forth on this list and on Reddit for two solid days and, to be frank, I'm running out of steam.

No, I said I would provide a list of before/after screenshots so people in this thread that haven't been following the entire discussion can more easily see the differences and benefits of this change.

I can't speak for the preferences of all the mergetool authors out there but I will try to convince you of the merit of adding a configuration flag in two places instead of just one. This took me a long time to write; I would appreciate it if you read it slowly and carefully.

There are three broad classifications of mergetools that I've seen so far. I've only collected notes for seventeen of them but there are many, many others. I mentioned those classifications earlier in this thread but to repeat them here:

1.  Tools that ignore conflict resolution and simply diff LOCAL and
    REMOTE. This accounts for most of them.
2.  Tools that perform their own, internal conflict resolution. Examples
    are tkdiff, WinMerge, IntelliJ.
3.  Tools that reuse Git's conflict resolution by splitting MERGED.
    Examples are Emacs + Magit, vim-mergetool, diffconflicts.

This patch *will* (probably) always benefit the first group. Because this patch *overwrites* LOCAL and REMOTE those tools will see immediate benefit without having to make any changes at all.

This patch *may* benefit the second group, or it may not affect them at all, or they may want it off. If they keep it on they will run conflict resolution against files that have already undergone that process in Git and likely turn up no additional resolutions since Git is quite good at doing that. It is also possible that some of them may do a better job than Git, or that some of them may want Git to tackle some of the conflict resolution (things that require access to the file history such as renames or recursive parents), but also want to perform even more sophisticated conflict resolution on top of that. Some of those tools are very clever and innovation in this space is still happening.

This patch *may* benefit the third group or it may make them no longer necessary. Speaking as the author of one of them: I see that as a good thing.

This patch *may* prevent tools in the second and third groups from having all the information they desire to show users without also running additional Git commands. Because this patch *overwrites* LOCAL and REMOTE any tool that wishes to show the state of the file before the merge was started will need to use Git to generate a new copy of those files. Although this is a negative I think benefiting the large number of tools in the first group outweighs this negative. Because the second and third groups of tools are more sophisticated, I think the authors of those tools are better suited to navigating these pros and cons.

In closing, below are several use-cases for having both a tool-level flag and a global flag:

I, as the author of diffconflicts, *will* want to disable this flag for diffconflicts because that tool already does what this flag does *and* because that tool makes use of the original versions of LOCAL and REMOTE which this patch destroys.

I, as a user of diffconflicts, *may* want to disable this flag because in practice I don't actually reference LOCAL and REMOTE very often. Other users may reference LOCAL and REMOTE every time. You yourself said, "Even if I were using diffconflict, I would want this option on." Other users may choose differently than you because you are also not the "sole arbiter of truth" and because this is entirely about personal preference and is decidedly not about objective truth. If this patch always benefited everyone then we would make it default and not put it behind an opt-in flag.

I, as a user of diffconflicts, may want to both disable this flag for diffconflicts but enable this flag for VS Code and kdiff3. It is not unusual for people to use more than one mergetool. Some of them are better or worse at visualizing different kinds of conflicts. Sometimes a conflict is small and straightforward; othertimes a conflict is complicated and requires deep knowledge of the history of both branches. If we force this to be a global flag only then users will not be able to make different choices for different tools.

Someone who does use multiple mergetools but only uses tools from group one may appreciate a single global flag so s/he doesn't need to set it for each tool.

Someone who is actively developing a new mergetool may want to enable or disable this flag for that tool while still keeping the flag enabled or disabled for tools that s/he uses for work or school. Someone who is comparing and contrasting multiple mergetools for work or school may want to selectively enable or disable the flag for one tool or another in order to do the comparison. Someone may want the same mergetool configured twice, once with this flag and once without, in order to contrast the difference.

An end-user may want to use a mergetool differently than the author intended. You said, "This should be a setting in the tool driver, not a user-visible setting," and then you said, "Even if I were using diffconflict, I would want this option on." But I said, "I, as the author of diffconflicts, *will* want to disable this flag." and there's no reason we can't have both. I can't presume what preference other mergetool authors will have. We can make some educated guesses and even default the option to true for some mergetool wrappers that ship with Git but there's every possibility that a user will prefer it a different way or that a mergetool author will. And there's every possibility that there will be differing opinions between users and authors like there is between you and me. But that's ok! Because it's just a configuration option.