git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>, git@vger.kernel.org
Subject: Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in "diff3 -m" style
Date: Sun, 31 Aug 2008 10:38:18 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0808311021120.12958@nehalem.linux-foundation.org> (raw)
In-Reply-To: <7v1w05d5hm.fsf@gitster.siamese.dyndns.org>



On Sun, 31 Aug 2008, Junio C Hamano wrote:
> 
> On average, I am finding that "diff3 -m" format more irritating than
> useful.  However, on occasions like this, I am finding it quite useful.

I have to say that the best feature of the standard merge is the zealous 
version, which apparently doesn't work well with the "diff3 -m" format.

I do agree that what is often really nasty with the normal merge conflict 
info is when you actually need the original thing to understand why some 
hunk was left, and yes, a common case of that is that one side simply 
removed a function, and the other side had modified it. At that point the 
resulting conflict is often hard to understand without seeing the 
original, exactly because you don't see any actual "conflict", you only 
see one side (the other side being empty).

But what really saves that situation is the combination of

 - 'gitk --merge <file>'
 - 'git diff' showing the multi-way merge

and I find myself really _hating_ doing rebases because the merge helpers 
are so totally useless (ie "gitk --merge" at least didn't use to work 
across a rebase conflict because MERGE_HEAD isn't set)

But the biggest problem, and the reason I _really_ detest the diff3 
format, is that small merges are fairly often pretty easy to see anyway. 
If the conflict markers all fit in one screenful, it's generally fairly 
easy to see why something conflicted, because you can visually compare 
things.

But the complicated cases are when there are bigger changes, and the 
conflict is over many many lines of code, and it's really hard to visually 
see what changed. And the diff3 format makes this worse - it not only 
makes the conflict 50% bigger to begin with, it moves the two conflicting 
versions away from each other, making that visual comparison much harder.

Now, there are tools to help with that. I think various of the graphical 
merge tools understand the diff3 format, and then ti can really help. But 
I think it hurts for a lot of the _common_ cases.

> My observation so far suggests that it would be best for me to leave the
> configuration "merge.conflictstyle" to the default "merge", and instead
> give an option to allow me to tell "git checkout -m -- $path" (which is
> also a new feature; it overwrites the $path by the result of a fresh merge
> to reproduce the conflicted state in the working tree, using the three
> stages recorded in the index) to use "diff3 -m" style, when I want to.

Now *this* I think is a great idea! 

The reason I think it's a great idea is that it solves so many _different_ 
issues (which is the mark of a really good solution):

 - it fixes my problem with diff3 output: the fact that it's more annoying 
   by _default_ than it is occasionally useful.

   If the default isn't to do it - since by default it often hurts - but 
   you have the option to do it when there is something confusing going on 
   (like the "one side disappeared, why did it conflict?" case), then you 
   have the best of both worlds - a good default with a way to dig deeper 
   when you need to.

 - it fixes another totally unrelated problem: incorrect merge 
   resolutions.

   Again, I find this to be fairly rare, but what git is good at is to 
   make incremental resolutions for merge problems - you can resolve the 
   merge in the work tree, then compile and test the result before 
   actually committing it, and "git diff" always gives relevant and 
   interesting output for the merge.

   And _occasionally_ the resolve looks obvious, but then when you compile 
   things you notice that it doesn't work because (for example) you 
   resolved it by removing one side (exactly because the other side was a 
   removal), and it turned out that the conflict was adding a function 
   that you hadn't realized was new, and was needed.

   And while "git diff" is fine, and you can cut-and-paste things and try 
   to re-resolve it that way, I have occasionally decided to just do a 
   "git reset" and re-do the whole merge.

   But your idea allows us to just re-do the merge for a single file.

So I think we do quite well already, but your solution really does sound 
like a good and useful addition to the toolbox.

		Linus

  reply	other threads:[~2008-08-31 17:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-29  0:09 [PATCH 1/2] xdl_fill_merge_buffer(): separate out a too deeply nested function Junio C Hamano
2008-08-29  0:18 ` [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in "diff3 -m" style Junio C Hamano
2008-08-29  0:34   ` Linus Torvalds
2008-08-29  1:07     ` Junio C Hamano
2008-08-31  7:10       ` Junio C Hamano
2008-08-31 17:38         ` Linus Torvalds [this message]
2008-08-31 18:42           ` Junio C Hamano
2008-08-29  6:27     ` Junio C Hamano
2008-08-29  9:01     ` 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=alpine.LFD.1.10.0808311021120.12958@nehalem.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --subject='Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in "diff3 -m" style' \
    /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

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).