Re: [PATCH 7/7] xdiff: make diff3 the default conflictStyle

From: Felipe Contreras <felipe.contreras@gmail.com>
To: Elijah Newren <newren@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>
Cc: "Johannes Sixt" <j6t@kdbg.org>,
	"Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"David Aguilar" <davvid@gmail.com>,
	"Sergey Organov" <sorganov@gmail.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Denton Liu" <liu.denton@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH 7/7] xdiff: make diff3 the default conflictStyle
Date: Fri, 11 Jun 2021 13:28:46 -0500	[thread overview]
Message-ID: <60c3ab5e62dbf_8d0f20887@natae.notmuch> (raw)
In-Reply-To: <CABPp-BGtYnVPijg2OWoDBM915-PPFXk1O3H=BMf_itc4dNjAxQ@mail.gmail.com>

Elijah Newren wrote:
> On Fri, Jun 11, 2021 at 10:32 AM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > Elijah Newren wrote:
> > > On Fri, Jun 11, 2021 at 7:25 AM Felipe Contreras <felipe.contreras@gmail.com>
> > > wrote:
> >
> > > > Personally I have never experienced what you posted, so maybe there's
> > > > something else happening behind the scenes.
> > > >
> > > > Maybe merge-ort changed something.
> > >
> > > merge-ort made no changes relative to content merges or choice of merge
> > > bases.  (In fact, merge ort doesn't even handle content merges; that's the
> > > xdiff layer.)  Even if merge-ort had made changes in this area, merge-ort
> > > is not the default and I didn't see the necessary config tweaks in your
> > > list of config options.  (I would have recommended against people using
> > > merge ort until 7bec8e7fa6 ("Merge branch 'en/ort-readiness'", 2021-04-16),
> > > which only made it into a release last week with 2.32.  I probably won't be
> > > recommending it as the default at least until the optimization work is
> > > merged and it's hard to predict how many more months that will take.)
> >
> > Indeed, I tested on v2.25 and found the same output.
> >
> > I thought of merge-ort because 1) I've never seen such kind of output
> > before, and 2) grepping the code I thought I saw merge-ort being the
> > default of something, but now I seem to be unable to find where.
> 
> We have briefly discussed multiple times what things might be needed
> to eventually make merge-ort the default (though it's not even
> complete yet; I'm five months into upstreaming the optimization
> patches with an unknown number of months left).  There were also a
> couple patches to make the _tests_ default to using merge-ort on most
> platforms, while still keeping one suite that tests merge-recursive to
> ensure we don't add breakage.  Perhaps one of those is what you're
> thinking of?

Nah, I think I just read something wrong.

> > > It's more likely that the codebases you work with just don't have
> > > criss-cross merges.
> >
> > Yes, that's it.
> >
> > I don't see why people in these kinds of codebases would like diff3
> > doing that by default.
> 
> I suspect they don't[1].  What's the alternative, though?  Not using
> diff3?  Picking a different base to avoid the occasional nested
> conflict in the inner merge region, but which overall has much worse
> other side effects?

Picking a different conflict style for the inner merge.

> I think Junio was addressing this when he
> recently said elsewhere in this thread that "Rejecting diff3 style
> output because of the way a conflicted part in the inner merge appears
> as a common ancestor version may be throwing the baby out with the
> bathwater"[2].  Sure, it's an annoyance, but diff3 is still a good
> option and there is no current solution to the annoying nested merge
> display.

Yes, but he proceeded to say we could not recommend diff3 to new users,
so I see the baby out on the floor.

I don't think it's likely new users will experience a problem with diff3
inner conflict markers (in 15 years of using git I've never encountered
them myself [or I blocked them out my mind]).

Even with this problem, experienced developers seem to be unable to live
without diff3, so I don't know if there's a valid argument against
making it the default.

Yes, improving inner conflict markers would make life better for new
users, but also everyone else. Software can always be better.

Let's not make perfect be the enemy of the good; diff3 is more than good
enough already.

Cheers.

-- 
Felipe Contreras