git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Antoine Pelisse" <apelisse@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	git <git@vger.kernel.org>,
	kernel@pengutronix.de
Subject: Re: feature suggestion: optimize common parts for checkout --conflict=diff3
Date: Wed, 6 Mar 2013 20:02:54 -0500	[thread overview]
Message-ID: <20130307010254.GA850@sigill.intra.peff.net> (raw)
In-Reply-To: <7vip54p58p.fsf@alter.siamese.dyndns.org>

On Wed, Mar 06, 2013 at 01:50:46PM -0800, Junio C Hamano wrote:

> I think it is more like "I added bread and my wife added bread to
> our common shopping list" and our two-way "RCS merge" default is to
> collapse that case to "one loaf of bread on the shopping list".  My
> impression has always been that people who use "diff3" mode care
> about this case and want to know that the original did not have
> "bread" on the list in order to decide if one or two loaves of bread
> should remain in the result.

I think that is only the case sometimes. It depends on what is in the
conflict, and what your data is. I think you are conflating two things,
though: zealousness of merge, and having the original content handy when
resolving. To me, diff3 is about the latter. It can also be a hint that
the user cares about the former, but not necessarily.

> > In Uwe's example,
> > it is just noise that detracts from the interesting part of the change
> > (or does it? I think the answer is in the eye of the reader).
> 
> In other words, you would use the "RCS merge" style because most of
> the time you would resolve to "one loaf of bread" and the fact that
> it was missing in the original is not needed to decide that.  So, it
> feels strange to use "diff3" and still want to discard that
> information---if it is not relevant, why are you using diff3 mode in
> the first place?  That is the question that is still not answered.

Because for the lines that _are_ changed, you may want to see what the
original looked like. Here's a more realistic example:

	git init repo
	cd repo

	# Some baseline C code.
	cat >foo.c <<\EOF
	int foo(int bar)
	{
	  return bar + 5;
	}
	EOF
	git add foo.c
	git commit -m base
	git tag base

	# Simulate a modification to the function.
	sed -i '2a\
	  if (bar < 3)\
	    bar *= 2;
	' foo.c
	git commit -am multiply
	git tag multiply

	# And another modification.
	sed -i 's/bar + 5/bar + 7/' foo.c
	git commit -am plus7

	# Now on a side branch...
	git checkout -b side base

	# let's cherry pick the first change. Obviously
	# we could just fast-forward in this toy example,
	# but let's try to simulate a real history.
	#
	# We insert a sleep so that the cherry-pick does not
	# accidentally end up with the exact same commit-id (again,
	# because this is a toy example).
	sleep 1
	git cherry-pick multiply

	# and now let's make a change that conflicts with later
	# changes on master
	sed -i 's/bar + 5/bar + 8/' foo.c
	git commit -am plus8

	# and now merge, getting a conflict
	git merge master

	# show the result with various marker styles
	for i in merge diff3 zdiff3; do
	  echo
	  echo "==> $i"
	  git.compile checkout --conflict=$i foo.c
	  cat foo.c
	done

which produces:

	==> merge
	int foo(int bar)
	{
	  if (bar < 3)
	    bar *= 2;
	<<<<<<< ours
	  return bar + 8;
	=======
	  return bar + 7;
	>>>>>>> theirs
	}

The ZEALOUS level has helpfully cut out the shared cherry-picked bits,
and let us focus on the real change.

	==> diff3
	int foo(int bar)
	{
	<<<<<<< ours
	  if (bar < 3)
	    bar *= 2;
	  return bar + 8;
	||||||| base
	  return bar + 5;
	=======
	  if (bar < 3)
	    bar *= 2;
	  return bar + 7;
	>>>>>>> theirs
	}

Here we get to see all of the change, but the interesting difference is
overwhelmed by the shared cherry-picked bits. It's only 2 lines here,
but of course it could be much larger in a real example, and the reader
is forced to manually verify that the early parts are byte-for-byte
identical.

	==> zdiff3
	int foo(int bar)
	{
	  if (bar < 3)
	    bar *= 2;
	<<<<<<< ours
	  return bar + 8;
	||||||| base
	  return bar + 5;
	=======
	  return bar + 7;
	>>>>>>> theirs
	}

Here we see the hunk cut-down again, removing the cherry-picked parts.
But the presence of the base is still interesting, because we see
something that was not in the "merge" marker: that we were originally
at "5", and moved to "7" on one side and "8" on the other.

I see conflicts like this when I rebase my topics forward; you may pick
up part of my series, or even make a tweak to a patch in the middle. I
prefer diff3 markers because they carry more information (and use them
automatically via merge.conflictstyle). But in some cases, the lack of
zealous reduction means that I end having to figure out whether and if
anything changed in the seemingly identical bits.  Sometimes it is
nothing, and sometimes you tweaked whitespace or fixed a typo, and it
takes a lot of manual looking to figure it out. I hadn't realized it was
related to the use of diff3 until the discussion today.

-Peff

  reply	other threads:[~2013-03-07  1:03 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-06 15:05 feature suggestion: optimize common parts for checkout --conflict=diff3 Uwe Kleine-König
2013-03-06 18:27 ` Antoine Pelisse
2013-03-06 19:26 ` Antoine Pelisse
2013-03-06 20:03   ` Jeff King
2013-03-06 20:36     ` [PATCH] xdiff: implement a zealous diff3 Uwe Kleine-König
2013-03-06 20:46       ` Jeff King
2013-03-06 20:40     ` feature suggestion: optimize common parts for checkout --conflict=diff3 Junio C Hamano
2013-03-06 20:54       ` Jeff King
2013-03-06 21:09         ` Junio C Hamano
2013-03-06 21:21           ` Jeff King
2013-03-06 21:50             ` Junio C Hamano
2013-03-07  1:02               ` Jeff King [this message]
2013-03-06 21:31           ` Uwe Kleine-König
2013-03-06 21:32           ` Junio C Hamano
2013-03-07  8:04             ` Jeff King
2013-03-07 17:26               ` Junio C Hamano
2013-03-07 18:01                 ` Jeff King
2013-03-07 18:40                   ` Junio C Hamano
2013-03-07 18:50                     ` Jeff King
2013-04-04 20:33                       ` Jeff King
2013-04-04 20:49                         ` Uwe Kleine-König
2013-04-04 20:54                           ` Jeff King
2013-04-04 21:19                             ` Junio C Hamano
2013-03-07 18:21                 ` Junio C Hamano

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=20130307010254.GA850@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=apelisse@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kernel@pengutronix.de \
    --cc=u.kleine-koenig@pengutronix.de \
    /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 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 NNTP newsgroup(s).