All of lore.kernel.org
 help / color / mirror / Atom feed
From: Seth House <seth@eseth.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: Johannes Sixt <j6t@kdbg.org>, Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, David Aguilar <davvid@gmail.com>
Subject: Re: [RFC/PATCH] mergetool: use resolved conflicts in all the views
Date: Thu, 17 Dec 2020 22:49:47 -0700	[thread overview]
Message-ID: <20201218054947.GA123376@ellen> (raw)
In-Reply-To: <5fdc18a91c402_f2faf20837@natae.notmuch>

On Thu, Dec 17, 2020 at 08:49:13PM -0600, Felipe Contreras wrote:
> And no person is the sole arbiter of truth, in this list, or anywhere.
> People have managed to change Junio's mind.

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.

> You said you would provide a list of tools that would be affected.

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.


  reply	other threads:[~2020-12-18  5:50 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-16 17:43 [RFC/PATCH] mergetool: use resolved conflicts in all the views Felipe Contreras
2020-12-16 22:24 ` Junio C Hamano
2020-12-16 22:53   ` Seth House
2020-12-17  5:18     ` Junio C Hamano
2020-12-17  5:41     ` Felipe Contreras
2020-12-17  7:35       ` Johannes Sixt
2020-12-17  8:27         ` Felipe Contreras
2020-12-17 19:23           ` Johannes Sixt
2020-12-18  2:30             ` Felipe Contreras
2020-12-17  9:44         ` Seth House
2020-12-17 10:35           ` Felipe Contreras
2020-12-17 17:50             ` Seth House
2020-12-17 19:28               ` Junio C Hamano
2020-12-18  2:34                 ` Felipe Contreras
     [not found]                   ` <CANiSa6jMXTyfo43bUdC8601BvYKiF67HXo+QaiTh_-8KWyBsLg@mail.gmail.com>
2020-12-21  0:31                     ` Felipe Contreras
2020-12-18  2:05               ` Felipe Contreras
2020-12-18  2:35                 ` Seth House
2020-12-18  2:49                   ` Felipe Contreras
2020-12-18  5:49                     ` Seth House [this message]
2020-12-18  9:46                       ` Felipe Contreras
2020-12-19  0:13                         ` Seth House
2020-12-19  0:53                           ` Felipe Contreras
2020-12-19 11:14                           ` Junio C Hamano
2020-12-19 12:08                             ` Felipe Contreras
2020-12-19 18:26                               ` Junio C Hamano
2020-12-19 20:18                                 ` Felipe Contreras
2020-12-21  4:25                             ` Seth House
2020-12-21  5:34                               ` Felipe Contreras
2020-12-21  7:36                                 ` Seth House
2020-12-21 11:17                                   ` Felipe Contreras
2020-12-21 22:15                                   ` David Aguilar
2020-12-21 23:51                                     ` Code of conduct violation? Felipe Contreras
2020-12-22  7:13                                       ` Junio C Hamano
2020-12-22  9:58                                         ` Felipe Contreras
2020-12-22 15:01                                       ` Pratyush Yadav
2020-12-23  4:23                                         ` Felipe Contreras
2020-12-23  5:02                                           ` Junio C Hamano
2020-12-23  5:41                                             ` Felipe Contreras
2020-12-23 15:04                                               ` Nobody is THE one making contribution Junio C Hamano
2020-12-23 15:51                                                 ` Felipe Contreras
2020-12-23 20:56                                                   ` Junio C Hamano
2020-12-24  1:09                                                     ` Felipe Contreras
2020-12-24  2:01                                                   ` Ævar Arnfjörð Bjarmason
2020-12-24  5:19                                                     ` Felipe Contreras
2020-12-24 12:30                                                       ` Ævar Arnfjörð Bjarmason
2020-12-24 15:26                                                         ` Felipe Contreras
2020-12-24 22:57                                                           ` Junio C Hamano
2020-12-27 17:29                                                             ` Felipe Contreras
2020-12-27 18:30                                                               ` Junio C Hamano
2020-12-27 18:47                                                                 ` Felipe Contreras
2020-12-28 10:39                                                                   ` Junio C Hamano
2020-12-28 14:27                                                                     ` Felipe Contreras
2020-12-24 15:09                                                       ` Randall S. Becker
2020-12-24 15:37                                                         ` Felipe Contreras
2020-12-24 22:40                                                           ` Junio C Hamano
2020-12-24 21:00                                       ` Code of conduct violation? David Aguilar
2020-12-24 22:32                                         ` Felipe Contreras
2020-12-18 10:04                       ` [RFC/PATCH] mergetool: use resolved conflicts in all the views Junio C Hamano
2020-12-18 11:58                         ` Felipe Contreras
2020-12-19 18:52                           ` Junio C Hamano
2020-12-19 20:59                             ` Felipe Contreras
2020-12-20  6:44                               ` David Aguilar
2020-12-20  7:53                                 ` Felipe Contreras
2020-12-20 22:22                                   ` David Aguilar
2020-12-21  1:46                                     ` Felipe Contreras
2020-12-19  0:18                         ` Seth House
2020-12-16 23:41   ` Felipe Contreras
2020-12-17  5:19     ` Junio C Hamano
2020-12-17  5:43       ` Felipe Contreras
2020-12-17  2:35 ` [RFC/PATCH v2] " Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201218054947.GA123376@ellen \
    --to=seth@eseth.com \
    --cc=davvid@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.