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

Seth House wrote:
> On Sun, Dec 20, 2020 at 11:34:24PM -0600, Felipe Contreras wrote:
> > I disagree. It's fine if you don't want to participate, but the fact
> > remains that the position that some tools would want to turn this off
> > hasn't been properly defended.
> 
> If you are _genuinely_ interested in the answer to this question,

It's not about an answer to any specific question, defending a position
requires for you to answer any subsequent questions, like the one I made
here [1], which you never answered.

> please read the section in my post titled "Conflict Resolution"
> followed by the sub-section "Custom Merge Algorithm", and finally
> "Merge algorithms" [1] on Wikipedia.
> [1] https://en.wikipedia.org/wiki/Merge_(version_control)#Merge_algorithms

I had already read your blog post, and OK; I read that section in
Wikipedia.

> Then pretend you want to write your own conflict resolution algorithm
> for a new mergetool you've been dreaming up and ask yourself what
> versions of the conflicted file your tool will need.

I don't dream of writing my own conflict resolution algorithm, but if I
did it, would be to be used *before* the mergetool, before git makes the
decision to bother the user to resolve the conflicts manually, before it
decides there are actual conflicts, and before it writes the conflict
markers in the file.

In fact, it would work without any mergetool, that way the people that
like to edit the file directly (which you mentioned in your blog post
are actually the majority of your coworkers) can benefit from such
an algorithm.


I answered yours, now you answer mine [1]. What happens if you remove
the first paragraph of your make-conflicts.sh script?

Would a user have the chance to run "git mergetool" in those situations?

  echo "The frumious bandersnatch!" > BASE
  echo "The frumious bandersnatch!" > LOCAL
  echo "The frumious Bandersnatch!" > REMOTE
  git merge-file -p LOCAL BASE REMOTE

> Right now the algorithm Git uses is pretty best-in-class so it might
> seem unlikely that someone would want to write one of those.

And yet that's what Elijah Newren is doing with his merge-ort proposal
which is flying right now.

You want these algorithms to run in "git merge", not "git mergetool".

> However a whopping *seven* of the tools surveyed do just that.

Because they are used in other situations, like working with subversion,
not just in git merge conflicts.

> Some of them even do a pretty good job (I've tried to point those out
> in the reviews).

Better than git?

> You're preoccupied with identifying a specific "adverse effect" but this
> debate isn't about that -- it's about giving individual tools the option
> to choose how they are used.

Yes, individual *real* tools that either exist, or could actually exist.

> If people out there want to try and write a better algorithm than Git,
> I want to see them try.

Yes, and we want that algorithm to be used *before* the mergetool.

Moreover, if any merge algorithm hopes to do better than git, it would
require more than just 3 versions of the file; it would require to look
at the commit graph.

If you somehow managed to create such an algorithm that is better than
git, you certainly wouldn't give it out for free, and it wouldn't be on
a puny mergetool to be used only when git thinks there are conflicts; it
would be a solution to be used *at every merge*.

And that's what a *real* tool, Plastic SCM [2], does; it's a complete
solution that claims to have a better merge algorithm than Git.

> That's the point I've been trying to drive home and that's the point
> that David also made in his last reply to you.
> 
> On that note: you replied to David and said:
> 
> > [Y]ou spend your time implementing this on top of my patch. That way
> > it's clear who made the mistake.
> 
> I plan to start work on exactly that tomorrow. You made the initial
> patch so if you'd prefer to take it over the finish line yourself I'll
> defer. But if you're not interested then I would be happy to credit you
> and finish it.

It is already at the finish line (I just need to update the commit
message).

If you take my patch and add a bunch of unnecessary code, then remove my
'Signed-off-by' line, because I don't approve of those changes.

Better yet; add *your* changes on a separate commit on top of my patch.
That way it's clear who did the meat of the changes, and who did the
unnecessary ones.

> > Thanks for doing this, but unfortunately one of the most popular isn't
> > listed there: vimdiff2.
> 
> It's under the Vim section. Use the page search in your browser for
> "vimdiff2".

It says "The vimdiff, vimdiff3, and vimdiff2 mergetools are not
individually detailed because they're all variations on the same theme".
So yeah; no screenshots.

And I disagree; vimdiff2 looks completely different from vimdiff. If you
have to choose one, you should choose vimdiff2, which is what everyone
in the reddit thread you posted on r/git told you they use, not vimdiff.

You shouldn't deliberately chose the worst option that nobody uses.

Moreover, the main view of your mergetool can be considered a variation
also, I can replicate the behavior with the following vimdiff4:

  "$merge_tool_path" -f -d -c 'wincmd l | %d | read #1 | 1d | bd 1' \
    "$LOCAL" "$MERGED" "$REMOTE"

Does that mean diffconflicts shouldn't be listed either?

Of course not; just add vimdiff2.

Cheers.

[1] https://lore.kernel.org/git/5fdd4f22c473b_1d952220844@natae.notmuch/
[2] http://blog.plasticscm.com/2011/09/merge-recursive-strategy.html

-- 
Felipe Contreras

  reply	other threads:[~2020-12-21 11:18 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
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 [this message]
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=5fe08449f3bd1_9b3120865@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.
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.