git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Seth House <seth@eseth.com>, Johannes Sixt <j6t@kdbg.org>
Cc: Felipe Contreras <felipe.contreras@gmail.com>,
	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 04:35:29 -0600	[thread overview]
Message-ID: <5fdb3471c6bb7_d6d032087@natae.notmuch> (raw)
In-Reply-To: <20201217094424.GA75257@ellen>

Seth House wrote:
> On Thu, Dec 17, 2020 at 08:35:21AM +0100, Johannes Sixt wrote:
> > Where's WinMerge in your list?
> 
> Added. Thanks for the suggestion; I wasn't familiar with it. Let me know
> if I missed anything in my (quick) assessment. The "auto-merge" function
> does indeed produce similar results to Felipe's patch.
> 
> > I would say that this is a hint that post-processing should
> > be moved to the tool drivers, and should not be done at the proposed level.
> 
> That was my thought as well and it didn't occur to me to raise
> a discussion here. However Felipe made the good point that adding this
> functionality in upstream Git would have immediate downstream effects
> for most tools. Almost every mergetool I've surveyed so far simply
> blindly shows a diff betweeen LOCAL and REMOTE (and sometimes BASE and
> sometimes MERGED) and Felipe's patch would have an immediate benefit for
> all those tools.
> 
> There are a few notable exceptions that have their own, internal
> conflict resolution logic -- tkdiff, WinMerge, and (I think) IntelliJ.
> And only two tools make direct use of the conflict resolution that Git
> already performed to produce MERGED -- Emacs+Magit, and diffconflicts.
> Those tools would have to make a decision whether to opt-in to the
> autoMerge flag or not.

Each tool would have to be evaluated individually, but at least from
what I can see your tool--diffconflicts--shows exactly the same diff
with or without my patch, and the `mergetool.automerge` configuration.
It's only the DiffConflictsShowHistory command (which is secondary
functionality, that you didn't even mention in your blog) that gets
affected.

So, yes; *some* behavior is affected, but in my opinion not in a
negative way.

In my view the whole point of "git mergetool" is to resolve conflicts,
so if git can resolve conflicts *before* launching the mergetool, it
should (whether it's rerere or automerge).

> I did not initially like Felipe's patch because I personally want my
> mergetool to use all five of LOCAL, REMOTE, BASE, _and_ the two split
> halves of MERGED. However the pragmatism of it is growing on me. Because
> so many mergetools do simply diff LOCAL and REMOTE this one, simple
> opt-in would positively benefit all of them.

Yes. But other people may want to see the secondary functionality of
your DiffConflictsShowHistory command with simplified conflicts, and
those people can enable the option.

If you want to see the original LOCAL, REMOTE, and BASE, you can turn
off that option (or simply never turn it on).

With the option everyone can have what they want.

I for example can keep using `gvimdiff3`, except with simpler diff
chunks, thanks to your idea.

And for the record; "git mergetool" is one of of those things that you
setup once, and forget about it. My fingers have been trained for more
than a decade (maybe two) on exactly the same configuration, and I
initially didn't think you were onto something.

It's really good that you did setup a test repository with a merge
conflict that showcased the different situations one might encounter. It
immediately became clear to me that you were indeed onto something.

So I give you full credit for the idea, and explaining it succinctly,
that's in the commit message. I just found a way to shove it directly
into "git mergetool".

Cheers.

-- 
Felipe Contreras

  reply	other threads:[~2020-12-17 10:37 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 [this message]
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
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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=5fdb3471c6bb7_d6d032087@natae.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=seth@eseth.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).