git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
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

  reply	other threads:[~2021-06-11 18:30 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-09 19:28 [PATCH 0/7] Make diff3 the default conflict style Felipe Contreras
2021-06-09 19:28 ` [PATCH 1/7] test: add merge style config test Felipe Contreras
2021-06-09 19:42   ` Eric Sunshine
2021-06-09 20:29     ` Felipe Contreras
2021-06-10  9:18   ` Phillip Wood
2021-06-10 13:26     ` Felipe Contreras
2021-06-10 14:54       ` Phillip Wood
2021-06-10 16:34         ` Felipe Contreras
2021-06-10 14:58       ` Phillip Wood
2021-06-10 16:47         ` Felipe Contreras
2021-06-11  9:19           ` Phillip Wood
2021-06-11 14:39             ` Felipe Contreras
2021-06-09 19:28 ` [PATCH 2/7] merge-tree: fix merge.conflictstyle handling Felipe Contreras
2021-06-09 19:28 ` [PATCH 3/7] notes: " Felipe Contreras
2021-06-09 19:28 ` [PATCH 4/7] checkout: " Felipe Contreras
2021-06-10  9:32   ` Phillip Wood
2021-06-10 14:11     ` Felipe Contreras
2021-06-10 14:50       ` Phillip Wood
2021-06-10 16:32         ` Felipe Contreras
2021-06-11  9:18           ` Phillip Wood
2021-06-11 14:34             ` Felipe Contreras
2021-06-11  9:18   ` Phillip Wood
2021-06-09 19:28 ` [PATCH 5/7] xdiff: rename XDL_MERGE_STYLE_DIFF3 Felipe Contreras
2021-06-10  9:21   ` Phillip Wood
2021-06-10 13:33     ` Felipe Contreras
2021-06-11  3:17     ` Junio C Hamano
2021-06-11 13:42       ` Felipe Contreras
2021-06-09 19:28 ` [PATCH 6/7] xdiff: simplify style assignments Felipe Contreras
2021-06-10  9:26   ` Phillip Wood
2021-06-10 13:50     ` Felipe Contreras
2021-06-09 19:28 ` [PATCH 7/7] xdiff: make diff3 the default conflictStyle Felipe Contreras
2021-06-10  6:41   ` Johannes Sixt
2021-06-10  7:53     ` Đoàn Trần Công Danh
2021-06-10 13:18       ` Felipe Contreras
2021-06-10 13:18     ` Felipe Contreras
2021-06-10 13:49     ` Jeff King
2021-06-10 16:00       ` Felipe Contreras
2021-06-10 16:31         ` Jeff King
2021-06-11  1:20       ` Junio C Hamano
2021-06-11  6:23         ` Johannes Sixt
2021-06-11  6:43           ` Junio C Hamano
2021-06-11  7:02             ` Johannes Sixt
2021-06-11  7:14               ` Junio C Hamano
2021-06-11 11:51                 ` Sergey Organov
2021-06-11 15:32                   ` Felipe Contreras
2021-06-11 15:52                     ` Sergey Organov
2021-06-11 16:36                       ` Felipe Contreras
     [not found]                     ` <CABPp-BHRQSF2_aYTBfpfnW4Bh3Hz7vLFj_QNGj8R4WeCS6_utw@mail.gmail.com>
2021-06-11 17:57                       ` Felipe Contreras
2021-06-11 19:02                         ` Elijah Newren
2021-06-11 21:05                           ` Felipe Contreras
2021-06-11 21:40                             ` Elijah Newren
2021-06-13 14:34                               ` Felipe Contreras
2021-06-11 16:41                   ` Johannes Sixt
2021-06-11 17:21                     ` Felipe Contreras
2021-06-11 17:40                       ` Sergey Organov
2021-06-11 18:10                         ` Felipe Contreras
2021-06-11 18:22                           ` Sergey Organov
2021-06-11 14:28                 ` Felipe Contreras
2021-06-11 14:25               ` Felipe Contreras
2021-06-11 16:53                 ` Johannes Sixt
     [not found]                 ` <CABPp-BH0aRiSUw03nSK6jHRNQ+zcpUzr6WjeJ5GpdUCqCKxbag@mail.gmail.com>
2021-06-11 17:32                   ` Felipe Contreras
2021-06-11 17:57                     ` Elijah Newren
2021-06-11 18:28                       ` Felipe Contreras [this message]
2021-06-11 14:20           ` Felipe Contreras
2021-06-11 14:09         ` Felipe Contreras
2021-06-10  9:40   ` Phillip Wood
2021-06-10 14:19     ` Felipe Contreras
2021-06-17 17:40 ` [PATCH 0/7] Make diff3 the default conflict style Phillip Wood
2021-06-17 18:24   ` 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=60c3ab5e62dbf_8d0f20887@natae.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=avarab@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=davvid@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=liu.denton@gmail.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=sorganov@gmail.com \
    --subject='Re: [PATCH 7/7] xdiff: make diff3 the default conflictStyle' \
    /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

Code repositories for project(s) associated with this 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).