From: Linus Torvalds <firstname.lastname@example.org> To: Junio C Hamano <email@example.com> Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>, firstname.lastname@example.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.email@example.com> (raw) In-Reply-To: <firstname.lastname@example.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
next prev parent 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.email@example.com \ --firstname.lastname@example.org \ --cc=Johannes.Schindelin@gmx.de \ --email@example.com \ --firstname.lastname@example.org \ --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).