{"thread":{"id":"33088","subject":"feature suggestion: optimize common parts for checkout --conflict=diff3","startedAt":"2013-03-06T15:05:48Z","lastAt":"2013-04-04T21:19:19Z","messageCount":24,"participants":["Uwe Kleine-König","Antoine Pelisse","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"210700","messageId":"20130306150548.GC15375@pengutronix.de","threadId":"33088","inReplyTo":null,"subject":"feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2013-03-06T15:05:48Z","receivedAt":"2013-03-06T15:05:48Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello,\n\nHere comes another recipe for a different suggestion:\n\n\tgit init\n\techo 1 > file\n\tgit add file\n\tgit commit -m 'base'\n\tgit branch branch\n\tseq 1 30 | grep -v 15 > file\n\tgit commit -m 'add 2-30 without 15' file\n\tgit checkout branch\n\tseq 1 30 | grep -v 16 > file\n\tgit commit -m 'add 2-30 without 16' file\n\tgit merge master\n\tgit diff\n\nThis yields:\n\n\tdiff --cc file\n\tindex a07e697,5080129..0000000\n\t--- a/file\n\t+++ b/file\n\t@@@ -12,7 -12,7 +12,11 @@@\n\t 12\n\t 13\n\t 14\n\t++<<<<<<< HEAD\n\t +15\n\t++=======\n\t+ 16\n\t++>>>>>>> master\n\t 17\n\t 18\n\t 19\n\nas expected; nice and sweet. After\n\n\tgit checkout --conflict=diff3 file\n\nhowever the difference isn't that easy to spot any more. I expected\n\n\tdiff --cc file\n\tindex a07e697,5080129..0000000\n\t--- a/file\n\t+++ b/file\n\t@@@ -12,7 -12,7 +12,12 @@@\n\t 12\n\t 13\n\t 14\n\t++<<<<<<< ours\n\t +15\n\t++||||||| base\n\t++=======\n\t+ 16\n\t++>>>>>>> theirs\n\t 17\n\t 18\n\t 19\n\nBut instead I get\n\n\tdiff --cc file\n\tindex a07e697,5080129..0000000\n\t--- a/file\n\t+++ b/file\n\t@@@ -1,29 -1,29 +1,61 @@@\n\t 1\n\t++<<<<<<< ours\n\t +2\n\t +3\n\t +4\n\t +5\n\t +6\n\t +7\n\t +8\n\t +9\n\t +10\n\t +11\n\t +12\n\t +13\n\t +14\n\t +15\n\t +17\n\t +18\n\t +19\n\t +20\n\t +21\n\t +22\n\t +23\n\t +24\n\t +25\n\t +26\n\t +27\n\t +28\n\t +29\n\t +30\n\t++||||||| base\n\t++=======\n\t+ 2\n\t+ 3\n\t+ 4\n\t+ 5\n\t+ 6\n\t+ 7\n\t+ 8\n\t+ 9\n\t+ 10\n\t+ 11\n\t+ 12\n\t+ 13\n\t+ 14\n\t+ 16\n\t+ 17\n\t+ 18\n\t+ 19\n\t+ 20\n\t+ 21\n\t+ 22\n\t+ 23\n\t+ 24\n\t+ 25\n\t+ 26\n\t+ 27\n\t+ 28\n\t+ 29\n\t+ 30\n\t++>>>>>>> theirs\n\nOf course this is technically correct, just not maximally helpful.\n\nIs this a missing optimisation for the diff3 case or did I miss a detail\nthat makes my expectation wrong?\n\nBest regards\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | http://www.pengutronix.de/ |\n"},{"id":"210706","messageId":"CALWbr2zjrKN-op+deOvjT5ZC+6X=we7eoXTPv9W4AkcNst4yMw@mail.gmail.com","threadId":"33088","inReplyTo":"20130306150548.GC15375@pengutronix.de","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-06T18:27:59Z","receivedAt":"2013-03-06T18:27:59Z","isPatch":false,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"> git checkout --conflict=diff3 file\n\nThat's somehow unrelated, but shouldn't we have a \"conflict\" option to\ngit-merge as we have for git-checkout ?\n\nWith something like this (pasted into gmail):\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 7c8922c..edad742 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -65,6 +65,7 @@ static int abort_current_merge;\n static int show_progress = -1;\n static int default_to_upstream;\n static const char *sign_commit;\n+static char *conflict_style;\n\n static struct strategy all_strategy[] = {\n { \"recursive\", DEFAULT_TWOHEAD | NO_TRIVIAL },\n@@ -213,6 +214,7 @@ static struct option builtin_merge_options[] = {\n { OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key id\"),\n N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n OPT_BOOLEAN(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update\nignored files (default)\")),\n+ OPT_STRING(0, \"conflict\", &conflict_style, N_(\"style\"), N_(\"conflict\nstyle (merge or diff3)\")),\n OPT_END()\n };\n\n@@ -1102,6 +1104,9 @@ int cmd_merge(int argc, const char **argv, const\nchar *prefix)\n if (verbosity < 0 && show_progress == -1)\n show_progress = 0;\n\n+ if (conflict_style)\n+ git_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n+\n if (abort_current_merge) {\n int nargc = 2;\n const char *nargv[] = {\"reset\", \"--merge\", NULL};\n"},{"id":"210710","messageId":"CALWbr2xDYuCN4nd-UNxkAY8-EguYjHBYgfu1fLtOGhYZyRQg_A@mail.gmail.com","threadId":"33088","inReplyTo":"20130306150548.GC15375@pengutronix.de","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-03-06T19:26:57Z","receivedAt":"2013-03-06T19:26:57Z","isPatch":false,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"> however the difference isn't that easy to spot any more. I expected\n>\n> diff --cc file\n> index a07e697,5080129..0000000\n> --- a/file\n> +++ b/file\n> @@@ -12,7 -12,7 +12,12 @@@\n> 12\n> 13\n> 14\n> ++<<<<<<< ours\n> +15\n> ++||||||| base\n> ++=======\n> + 16\n> ++>>>>>>> theirs\n> 17\n> 18\n> 19\n\nThis is not correct, it would mean that 12, 13, 14, 17, 18, 19 are in\nbase, while they are not.\n\nCheers,\nAntoine\n"},{"id":"210711","messageId":"20130306200347.GA20312@sigill.intra.peff.net","threadId":"33088","inReplyTo":"CALWbr2xDYuCN4nd-UNxkAY8-EguYjHBYgfu1fLtOGhYZyRQg_A@mail.gmail.com","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-06T20:03:47Z","receivedAt":"2013-03-06T20:03:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 06, 2013 at 08:26:57PM +0100, Antoine Pelisse wrote:\n\n> > however the difference isn't that easy to spot any more. I expected\n> >\n> > diff --cc file\n> > index a07e697,5080129..0000000\n> > --- a/file\n> > +++ b/file\n> > @@@ -12,7 -12,7 +12,12 @@@\n> > 12\n> > 13\n> > 14\n> > ++<<<<<<< ours\n> > +15\n> > ++||||||| base\n> > ++=======\n> > + 16\n> > ++>>>>>>> theirs\n> > 17\n> > 18\n> > 19\n> \n> This is not correct, it would mean that 12, 13, 14, 17, 18, 19 are in\n> base, while they are not.\n\nYeah, I agree it is a bit of a lie, as you are not seeing the full\npicture of what was in the base. That is why we intentionally dial down\nthe conflict simplification level when using diff3. If you apply this\npatch to git:\n\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 9e13b25..f381e0c 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -420,15 +420,6 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,\n \tint style = xmp->style;\n \tint favor = xmp->favor;\n \n-\tif (style == XDL_MERGE_DIFF3) {\n-\t\t/*\n-\t\t * \"diff3 -m\" output does not make sense for anything\n-\t\t * more aggressive than XDL_MERGE_EAGER.\n-\t\t */\n-\t\tif (XDL_MERGE_EAGER < level)\n-\t\t\tlevel = XDL_MERGE_EAGER;\n-\t}\n-\n \tc = changes = NULL;\n \n \twhile (xscr1 && xscr2) {\n\nthen it will produce the output that Uwe expects. While it can be\nmisleading, I also think it can make some conflicts (like this one) much\neasier to understand. I don't see any reason we can't have a \"zealous\ndiff3\" mode to let people view this output, as long as it is not the\ndefault.\n\n-Peff\n"},{"id":"210712","messageId":"1362602202-29749-1-git-send-email-u.kleine-koenig@pengutronix.de","threadId":"33088","inReplyTo":"20130306200347.GA20312@sigill.intra.peff.net","subject":"[PATCH] xdiff: implement a zealous diff3","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2013-03-06T20:36:42Z","receivedAt":"2013-03-06T20:36:42Z","isPatch":true,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"\"zdiff3\" is identical to ordinary diff3, only it allows more aggressive\ncompaction than diff3. This way the displayed base isn't necessary\ntechnically correct, but still this mode might help resolving merge\nconflicts between two near identical additions.\n\nSigned-off-by: Uwe Kleine-König \n---\nHello,\n\nthis patch implements what I want. Thanks to Jeff for pointing me to the\nright code to modify.\n\nBest regards\nUwe\n\n builtin/merge-file.c | 2 ++\n contrib/completion/git-completion.bash | 2 +-\n xdiff-interface.c | 2 ++\n xdiff/xdiff.h | 1 +\n xdiff/xmerge.c | 8 +++++++-\n 5 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex c0570f2..4ef86aa 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -32,6 +32,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tstruct option options[] = {\n \t\tOPT_BOOLEAN('p', \"stdout\", &to_stdout, N_(\"send results to standard output\")),\n \t\tOPT_SET_INT(0, \"diff3\", &xmp.style, N_(\"use a diff3 based merge\"), XDL_MERGE_DIFF3),\n+\t\tOPT_SET_INT(0, \"zdiff3\", &xmp.style, N_(\"use a zealous diff3 based merge\"),\n+\t\t\t\tXDL_MERGE_ZEALOUS_DIFF3),\n \t\tOPT_SET_INT(0, \"ours\", &xmp.favor, N_(\"for conflicts, use our version\"),\n \t\t\t XDL_MERGE_FAVOR_OURS),\n \t\tOPT_SET_INT(0, \"theirs\", &xmp.favor, N_(\"for conflicts, use their version\"),\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex b62bec0..a0d887e 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1091,7 +1091,7 @@ _git_checkout ()\n \n \tcase \"$cur\" in\n \t--conflict=*)\n-\t\t__gitcomp \"diff3 merge\" \"\" \"${cur##--conflict=}\"\n+\t\t__gitcomp \"diff3 merge zdiff3\" \"\" \"${cur##--conflict=}\"\n \t\t;;\n \t--*)\n \t\t__gitcomp \"\ndiff --git a/xdiff-interface.c b/xdiff-interface.c\nindex ecfa05f..a911c25 100644\n--- a/xdiff-interface.c\n+++ b/xdiff-interface.c\n@@ -308,6 +308,8 @@ int git_xmerge_config(const char *var, const char *value, void *cb)\n \t\t\tdie(\"'%s' is not a boolean\", var);\n \t\tif (!strcmp(value, \"diff3\"))\n \t\t\tgit_xmerge_style = XDL_MERGE_DIFF3;\n+\t\telse if (!strcmp(value, \"zdiff3\"))\n+\t\t\tgit_xmerge_style = XDL_MERGE_ZEALOUS_DIFF3;\n \t\telse if (!strcmp(value, \"merge\"))\n \t\t\tgit_xmerge_style = 0;\n \t\telse\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 219a3bb..9730c63 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -64,6 +64,7 @@ extern \"C\" {\n \n /* merge output styles */\n #define XDL_MERGE_DIFF3 1\n+#define XDL_MERGE_ZEALOUS_DIFF3 2\n \n typedef struct s_mmfile {\n \tchar *ptr;\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 9e13b25..4772707 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -177,7 +177,7 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \tsize += xdl_recs_copy(xe1, m->i1, m->chg1, 1,\n \t\t\t dest ? dest + size : NULL);\n \n-\tif (style == XDL_MERGE_DIFF3) {\n+\tif (style == XDL_MERGE_DIFF3 || style == XDL_MERGE_ZEALOUS_DIFF3) {\n \t\t/* Shared preimage */\n \t\tif (!dest) {\n \t\t\tsize += marker_size + 1 + marker3_size;\n@@ -420,6 +420,12 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,\n \tint style = xmp->style;\n \tint favor = xmp->favor;\n \n+\t/*\n+\t * This is the only change between XDL_MERGE_DIFF3 and\n+\t * XDL_MERGE_ZEALOUS_DIFF3. \"zdiff3\" isn't 100% technically correct (as\n+\t * the base might be considerably simplified), but still it might help\n+\t * interpreting conflicts between two big and near identical additions.\n+\t */\n \tif (style == XDL_MERGE_DIFF3) {\n \t\t/*\n \t\t * \"diff3 -m\" output does not make sense for anything\n-- \n1.8.2.rc2\n"},{"id":"210713","messageId":"7vvc94p8hb.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"20130306200347.GA20312@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-06T20:40:48Z","receivedAt":"2013-03-06T20:40:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> then it will produce the output that Uwe expects. While it can be\n> misleading,...\n\nMisleading is one thing but in this case isn't it outright wrong?\n\nIf you remove <<< ours ||| portion from the combined diff output,\nI would expect that the hunk will apply to the base, but that is no\nlonger true, no?\n"},{"id":"210714","messageId":"20130306204612.GA24535@sigill.intra.peff.net","threadId":"33088","inReplyTo":"1362602202-29749-1-git-send-email-u.kleine-koenig@pengutronix.de","subject":"Re: [PATCH] xdiff: implement a zealous diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-06T20:46:12Z","receivedAt":"2013-03-06T20:46:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 06, 2013 at 09:36:42PM +0100, Uwe Kleine-König wrote:\n\n> \"zdiff3\" is identical to ordinary diff3, only it allows more aggressive\n> compaction than diff3. This way the displayed base isn't necessary\n> technically correct, but still this mode might help resolving merge\n> conflicts between two near identical additions.\n> \n> Signed-off-by: Uwe Kleine-König \n\nI think the patch is correct, assuming this is the interface we want.\n\nIt would be more flexible instead to have:\n\n 1. user can configure zealous-level of merge via command-line or\n config (they cannot control it at all right now)\n\n 2. when diff3 is used and no level is explicitly given, do not go\n above EAGER\n\n 3. otherwise, respect the level given by the user, even if it is\n ZEALOUS\n\nBut that would involve a lot of refactoring. I don't know if it is worth\nthe effort (the bonus is that people can then set the level\nindependently, but I do not know that anyone ever wants to do that).\n\n> builtin/merge-file.c | 2 ++\n> contrib/completion/git-completion.bash | 2 +-\n> xdiff-interface.c | 2 ++\n> xdiff/xdiff.h | 1 +\n> xdiff/xmerge.c | 8 +++++++-\n> 5 files changed, 13 insertions(+), 2 deletions(-)\n\nI think this would need documentation not only to let users know about\nthe feature, but also to warn them of the caveats.\n\n-Peff\n"},{"id":"210715","messageId":"20130306205400.GA29604@sigill.intra.peff.net","threadId":"33088","inReplyTo":"7vvc94p8hb.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-06T20:54:00Z","receivedAt":"2013-03-06T20:54:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 06, 2013 at 12:40:48PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > then it will produce the output that Uwe expects. While it can be\n> > misleading,...\n> \n> Misleading is one thing but in this case isn't it outright wrong?\n> \n> If you remove <<< ours ||| portion from the combined diff output,\n> I would expect that the hunk will apply to the base, but that is no\n> longer true, no?\n\nIt shifts the concept of what is the \"base\" and what is the \"conflict\".\nIn Uwe's example, no, it would not apply to the single-line file that is\nthe true 3-way base. But it would apply to the content that is outside\nof the hunk marker; we have changed the concept of what is in the base\nand what is in the conflict by shrinking the conflict to its smallest\nsize. The same is true of the conflict markers produced in the non-diff3\ncase. It is a property of XDL_MERGE_ZEALOUS, not of the conflict style.\n\nIf your argument is \"diff3 means something different than regular\nconflict markers; it should have the property of being\nmachine-convertible into a patch, but regular markers do not\", I'm not\nsure I agree. It may be used that way, but I think it is mostly used in\ngit to give the reader more context when making a resolution. And\nanyway, I think the proposed change would not be to change diff3, but to\nintroduce a new diff3-like format that also shrinks the hunk size, so it\nwould not hurt existing users of diff3.\n\n-Peff\n"},{"id":"210717","messageId":"7vr4jsp756.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"20130306205400.GA29604@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-06T21:09:41Z","receivedAt":"2013-03-06T21:09:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But it would apply to the content that is outside\n> of the hunk marker; we have changed the concept of what is in the base\n> and what is in the conflict by shrinking the conflict to its smallest\n> size.\n\nHmm, unless you mean by \"base\" something entirely different from\n\"what was in the common ancestor version\", I do not think I can\nagree. The point of diff3 mode is to show how it looked line in the\ncommon ancestor and what the conflicting sides want to change that\ncommon version into; letting the user view three versions to help\nhim decide what to do by only looking at the part inside conflict\nmarkers.\n\nWe show \"both sides added, either identically or differently\" as\nnoteworthy events, but the patched code pushes \"both sides added\nidentically\" case outside the conflicting hunk, as if what was added\nrelative to the common ancestor version (in Uwe's case, is it 1-14\nthat is common, or just 10-14?) is not worth looking at when\nconsidering what the right resolution is. If it is not worth\nlooking at what was in the original for the conflicting part, why\nwould we be even using diff3 mode in the first place?\n"},{"id":"210718","messageId":"20130306212140.GA30202@sigill.intra.peff.net","threadId":"33088","inReplyTo":"7vr4jsp756.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-06T21:21:40Z","receivedAt":"2013-03-06T21:21:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 06, 2013 at 01:09:41PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > But it would apply to the content that is outside\n> > of the hunk marker; we have changed the concept of what is in the base\n> > and what is in the conflict by shrinking the conflict to its smallest\n> > size.\n> \n> Hmm, unless you mean by \"base\" something entirely different from\n> \"what was in the common ancestor version\", I do not think I can\n> agree.\n\nI don't know. I didn't use the word \"base\" in the first place. I was\ntrying to figure out what you meant. :)\n\nMy point is that the hunk (everything from \"<<<\" to \">>>\") is\nself-consistent. It's just misleading in that the hunk has been shrunk\nnot to include identical bits from each side. IMHO, this is not much\ndifferent than a nearby change being auto-resolved. The conflict hunks\nthe user sees do not represent the original files, but rather the\nremains after a first pass at resolving.\n\n> The point of diff3 mode is to show how it looked line in the\n> common ancestor and what the conflicting sides want to change that\n> common version into; letting the user view three versions to help\n> him decide what to do by only looking at the part inside conflict\n> markers.\n\nRight, I agree.\n\n> We show \"both sides added, either identically or differently\" as\n> noteworthy events, but the patched code pushes \"both sides added\n> identically\" case outside the conflicting hunk, as if what was added\n> relative to the common ancestor version (in Uwe's case, is it 1-14\n> that is common, or just 10-14?) is not worth looking at when\n> considering what the right resolution is. If it is not worth\n> looking at what was in the original for the conflicting part, why\n> would we be even using diff3 mode in the first place?\n\nI think Uwe's example shows that it _is_ useful. Yes, you no longer have\nthe information about what happened through 1-14 (whether it was really\nthere in the ancestor file, or whether it was simply added identically).\nBut that information might or might not be relevant. In Uwe's example,\nit is just noise that detracts from the interesting part of the change\n(or does it? I think the answer is in the eye of the reader). I think\nit can be helpful to have both types available, and they can pick which\none they want; it's just another tool.\n\nAnother argument is that some people (including me) set\nmerge.conflictstyle to diff3, because they like seeing the extra context\nwhen resolving (I find it helps a lot with rebasing, when it is\nsometimes hard to remember which side is which in the merge). I'd\nconsider setting it to zdiff3 to get the benefits of XDL_MERGE_ZEALOUS,\nand using \"git checkout --conflict-style=diff3\" if I need to get more\ninformation about a specific case.\n\n-Peff\n"},{"id":"210720","messageId":"20130306213129.GE15375@pengutronix.de","threadId":"33088","inReplyTo":"7vr4jsp756.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2013-03-06T21:31:29Z","receivedAt":"2013-03-06T21:31:29Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hello Junio,\n\nOn Wed, Mar 06, 2013 at 01:09:41PM -0800, Junio C Hamano wrote:\n> Jeff King writes:\n> \n> > But it would apply to the content that is outside\n> > of the hunk marker; we have changed the concept of what is in the base\n> > and what is in the conflict by shrinking the conflict to its smallest\n> > size.\n> \n> Hmm, unless you mean by \"base\" something entirely different from\n> \"what was in the common ancestor version\", I do not think I can\n> agree. The point of diff3 mode is to show how it looked line in the\n> common ancestor and what the conflicting sides want to change that\n> common version into; letting the user view three versions to help\n> him decide what to do by only looking at the part inside conflict\n> markers.\n> \n> We show \"both sides added, either identically or differently\" as\n> noteworthy events, but the patched code pushes \"both sides added\n> identically\" case outside the conflicting hunk, as if what was added\nI didn't test, but \"both sides removed identically\" should be moved out,\ntoo, shouldn't it?\n\n> relative to the common ancestor version (in Uwe's case, is it 1-14\n> that is common, or just 10-14?) is not worth looking at when\n> considering what the right resolution is. If it is not worth\n> looking at what was in the original for the conflicting part, why\n> would we be even using diff3 mode in the first place?\nbecause even zdiff3 contains more information than merge. And compared\nto diff3 it's smaller sometimes and so easier to understand.\n\nOther than that I agree fully to the things Jeff said so far.\n\nBest regards\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | http://www.pengutronix.de/ |\n"},{"id":"210721","messageId":"7vmwugp637.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"7vr4jsp756.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-06T21:32:28Z","receivedAt":"2013-03-06T21:32:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> But it would apply to the content that is outside\n>> of the hunk marker; we have changed the concept of what is in the base\n>> and what is in the conflict by shrinking the conflict to its smallest\n>> size.\n>\n> Hmm, unless you mean by \"base\" something entirely different from\n> \"what was in the common ancestor version\", I do not think I can\n> agree. The point of diff3 mode is to show how it looked line in the\n> common ancestor and what the conflicting sides want to change that\n> common version into; letting the user view three versions to help\n> him decide what to do by only looking at the part inside conflict\n> markers.\n>\n> We show \"both sides added, either identically or differently\" as\n> noteworthy events, but the patched code pushes \"both sides added\n> identically\" case outside the conflicting hunk, as if what was added\n> relative to the common ancestor version (in Uwe's case, is it 1-14\n> that is common, or just 10-14?) is not worth looking at when\n> considering what the right resolution is. If it is not worth\n> looking at what was in the original for the conflicting part, why\n> would we be even using diff3 mode in the first place?\n\nI vaguely recall we did this \"clip to eager\" as an explicit bugfix\nat 83133740d9c8 (xmerge.c: \"diff3 -m\" style clips merge reduction\nlevel to EAGER or less, 2008-08-29). The list archive around that\ntime may give us more contexts.\n"},{"id":"210725","messageId":"7vip54p58p.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"20130306212140.GA30202@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-06T21:50:46Z","receivedAt":"2013-03-06T21:50:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think Uwe's example shows that it _is_ useful. Yes, you no longer have\n> the information about what happened through 1-14 (whether it was really\n> there in the ancestor file, or whether it was simply added identically).\n> But that information might or might not be relevant.\n\nI think it is more like \"I added bread and my wife added bread to\nour common shopping list\" and our two-way \"RCS merge\" default is to\ncollapse that case to \"one loaf of bread on the shopping list\". My\nimpression has always been that people who use \"diff3\" mode care\nabout this case and want to know that the original did not have\n\"bread\" on the list in order to decide if one or two loaves of bread\nshould remain in the result.\n\n> In Uwe's example,\n> it is just noise that detracts from the interesting part of the change\n> (or does it? I think the answer is in the eye of the reader).\n\nIn other words, you would use the \"RCS merge\" style because most of\nthe time you would resolve to \"one loaf of bread\" and the fact that\nit was missing in the original is not needed to decide that. So, it\nfeels strange to use \"diff3\" and still want to discard that\ninformation---if it is not relevant, why are you using diff3 mode in\nthe first place? That is the question that is still not answered.\n"},{"id":"210737","messageId":"20130307010254.GA850@sigill.intra.peff.net","threadId":"33088","inReplyTo":"7vip54p58p.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-07T01:02:54Z","receivedAt":"2013-03-07T01:02:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 06, 2013 at 01:50:46PM -0800, Junio C Hamano wrote:\n\n> I think it is more like \"I added bread and my wife added bread to\n> our common shopping list\" and our two-way \"RCS merge\" default is to\n> collapse that case to \"one loaf of bread on the shopping list\". My\n> impression has always been that people who use \"diff3\" mode care\n> about this case and want to know that the original did not have\n> \"bread\" on the list in order to decide if one or two loaves of bread\n> should remain in the result.\n\nI think that is only the case sometimes. It depends on what is in the\nconflict, and what your data is. I think you are conflating two things,\nthough: zealousness of merge, and having the original content handy when\nresolving. To me, diff3 is about the latter. It can also be a hint that\nthe user cares about the former, but not necessarily.\n\n> > In Uwe's example,\n> > it is just noise that detracts from the interesting part of the change\n> > (or does it? I think the answer is in the eye of the reader).\n> \n> In other words, you would use the \"RCS merge\" style because most of\n> the time you would resolve to \"one loaf of bread\" and the fact that\n> it was missing in the original is not needed to decide that. So, it\n> feels strange to use \"diff3\" and still want to discard that\n> information---if it is not relevant, why are you using diff3 mode in\n> the first place? That is the question that is still not answered.\n\nBecause for the lines that _are_ changed, you may want to see what the\noriginal looked like. Here's a more realistic example:\n\n\tgit init repo\n\tcd repo\n\n\t# Some baseline C code.\n\tcat >foo.c <<\\EOF\n\tint foo(int bar)\n\t{\n\t return bar + 5;\n\t}\n\tEOF\n\tgit add foo.c\n\tgit commit -m base\n\tgit tag base\n\n\t# Simulate a modification to the function.\n\tsed -i '2a\\\n\t if (bar < 3)\\\n\t bar *= 2;\n\t' foo.c\n\tgit commit -am multiply\n\tgit tag multiply\n\n\t# And another modification.\n\tsed -i 's/bar + 5/bar + 7/' foo.c\n\tgit commit -am plus7\n\n\t# Now on a side branch...\n\tgit checkout -b side base\n\n\t# let's cherry pick the first change. Obviously\n\t# we could just fast-forward in this toy example,\n\t# but let's try to simulate a real history.\n\t#\n\t# We insert a sleep so that the cherry-pick does not\n\t# accidentally end up with the exact same commit-id (again,\n\t# because this is a toy example).\n\tsleep 1\n\tgit cherry-pick multiply\n\n\t# and now let's make a change that conflicts with later\n\t# changes on master\n\tsed -i 's/bar + 5/bar + 8/' foo.c\n\tgit commit -am plus8\n\n\t# and now merge, getting a conflict\n\tgit merge master\n\n\t# show the result with various marker styles\n\tfor i in merge diff3 zdiff3; do\n\t echo\n\t echo \"==> $i\"\n\t git.compile checkout --conflict=$i foo.c\n\t cat foo.c\n\tdone\n\nwhich produces:\n\n\t==> merge\n\tint foo(int bar)\n\t{\n\t if (bar < 3)\n\t bar *= 2;\n\t<<<<<<< ours\n\t return bar + 8;\n\t=======\n\t return bar + 7;\n\t>>>>>>> theirs\n\t}\n\nThe ZEALOUS level has helpfully cut out the shared cherry-picked bits,\nand let us focus on the real change.\n\n\t==> diff3\n\tint foo(int bar)\n\t{\n\t<<<<<<< ours\n\t if (bar < 3)\n\t bar *= 2;\n\t return bar + 8;\n\t||||||| base\n\t return bar + 5;\n\t=======\n\t if (bar < 3)\n\t bar *= 2;\n\t return bar + 7;\n\t>>>>>>> theirs\n\t}\n\nHere we get to see all of the change, but the interesting difference is\noverwhelmed by the shared cherry-picked bits. It's only 2 lines here,\nbut of course it could be much larger in a real example, and the reader\nis forced to manually verify that the early parts are byte-for-byte\nidentical.\n\n\t==> zdiff3\n\tint foo(int bar)\n\t{\n\t if (bar < 3)\n\t bar *= 2;\n\t<<<<<<< ours\n\t return bar + 8;\n\t||||||| base\n\t return bar + 5;\n\t=======\n\t return bar + 7;\n\t>>>>>>> theirs\n\t}\n\nHere we see the hunk cut-down again, removing the cherry-picked parts.\nBut the presence of the base is still interesting, because we see\nsomething that was not in the \"merge\" marker: that we were originally\nat \"5\", and moved to \"7\" on one side and \"8\" on the other.\n\nI see conflicts like this when I rebase my topics forward; you may pick\nup part of my series, or even make a tweak to a patch in the middle. I\nprefer diff3 markers because they carry more information (and use them\nautomatically via merge.conflictstyle). But in some cases, the lack of\nzealous reduction means that I end having to figure out whether and if\nanything changed in the seemingly identical bits. Sometimes it is\nnothing, and sometimes you tweaked whitespace or fixed a typo, and it\ntakes a lot of manual looking to figure it out. I hadn't realized it was\nrelated to the use of diff3 until the discussion today.\n\n-Peff\n"},{"id":"210749","messageId":"20130307080411.GA25506@sigill.intra.peff.net","threadId":"33088","inReplyTo":"7vmwugp637.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-07T08:04:11Z","receivedAt":"2013-03-07T08:04:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 06, 2013 at 01:32:28PM -0800, Junio C Hamano wrote:\n\n> > We show \"both sides added, either identically or differently\" as\n> > noteworthy events, but the patched code pushes \"both sides added\n> > identically\" case outside the conflicting hunk, as if what was added\n> > relative to the common ancestor version (in Uwe's case, is it 1-14\n> > that is common, or just 10-14?) is not worth looking at when\n> > considering what the right resolution is. If it is not worth\n> > looking at what was in the original for the conflicting part, why\n> > would we be even using diff3 mode in the first place?\n> \n> I vaguely recall we did this \"clip to eager\" as an explicit bugfix\n> at 83133740d9c8 (xmerge.c: \"diff3 -m\" style clips merge reduction\n> level to EAGER or less, 2008-08-29). The list archive around that\n> time may give us more contexts.\n\nThanks for the pointer. The relevant threads are:\n\n http://article.gmane.org/gmane.comp.version-control.git/94228\n\nand\n\n http://thread.gmane.org/gmane.comp.version-control.git/94339\n\nThere is not much discussion beyond what ended up in 8313374; both Linus\nand Dscho question whether level and output format are orthogonal, but\nseem to accept the explanation you give in the commit message.\n\nHaving read that commit and the surrounding thread, I think I stand by\nmy argument that \"zdiff3\" is a useful tool to have, as long as the user\nunderstands what the hunks mean. It should never replace diff3, but I\nthink it makes sense as a separate format.\n\nI was also curious whether it would the diff3/zealous combination would\ntrigger any weird corner cases. In particular, I wanted to know how the\nexample you gave in that commit of:\n\n postimage#1: 1234ABCDE789\n | /\n | /\n preimage: 123456789\n | \\\n | \\\n postimage#2: 1234AXCYE789\n\nwould react with diff3 (this is not the original example, but one with\nan extra \"C\" in the middle of postimage#2, which could in theory be\npresented as split hunks). However, it seems that we do not do such hunk\nsplitting at all, neither for diff3 nor for the \"merge\" representation.\n\n-Peff\n"},{"id":"210772","messageId":"7v1ubrnmtu.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"20130307080411.GA25506@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-07T17:26:05Z","receivedAt":"2013-03-07T17:26:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I was also curious whether it would the diff3/zealous combination would\n> trigger any weird corner cases. In particular, I wanted to know how the\n> example you gave in that commit of:\n>\n> postimage#1: 1234ABCDE789\n> | /\n> | /\n> preimage: 123456789\n> | \\\n> | \\\n> postimage#2: 1234AXCYE789\n>\n> would react with diff3 (this is not the original example, but one with\n> an extra \"C\" in the middle of postimage#2, which could in theory be\n> presented as split hunks). However, it seems that we do not do such hunk\n> splitting at all, neither for diff3 nor for the \"merge\" representation.\n\nWithout thinking about it too deeply,...\n\nI think the \"RCS merge\" _could_ show it as \"1234ACE789\"\nwithout losing any information (as it is already discarding what was\nin the original in the part that is affected by the conflict,\ni.e. \"56 was there\").\n\nLet's think aloud how \"diff3 -m\" _should_ split this. The most\nstraight-forward representation would be \"1234789\",\nthat is, where \"56\" was originally there, one side made it to\n\"ABCDE\" and the other \"AXCYE\".\n\nYou could make it \"1234789\", and that is\ntechnically correct (what there were in the shared original for the\nconflicted part is 5 and then 6), but the representation pretends\nthat it knows more than there actually is information, which may be\nsomewhat misleading. All these three are equally plausible split of\nthe original \"56\":\n\n\t1234789\n\t1234789\n\t1234789\n\nand picking one over others would be a mere heuristic. All three\nare technically correct representations and it is just the matter of\nwhich one is the easiest to understand. So, this is the kind of\n\"misleading but not incorrect\".\n\nIn all these cases, the middle part would look like this:\n\n\t<<<<<<< ours\n C\n ||||||| base\n =======\n\tC\n >>>>>>> theirs\n\nin order to honor the explicit \"I want to view all three versions to\nexamine the situation\" aka \"--conflict=diff3\" option. We cannot\nreduce it to just \"C\". That will make it \"not just misleading but\nis actively wrong\".\n"},{"id":"210775","messageId":"20130307180157.GA6604@sigill.intra.peff.net","threadId":"33088","inReplyTo":"7v1ubrnmtu.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-07T18:01:57Z","receivedAt":"2013-03-07T18:01:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 07, 2013 at 09:26:05AM -0800, Junio C Hamano wrote:\n\n> Without thinking about it too deeply,...\n> \n> I think the \"RCS merge\" _could_ show it as \"1234ACE789\"\n> without losing any information (as it is already discarding what was\n> in the original in the part that is affected by the conflict,\n> i.e. \"56 was there\").\n\nRight, I think that is sane, though we do not do that at this point.\n\n> Let's think aloud how \"diff3 -m\" _should_ split this. The most\n> straight-forward representation would be \"1234789\",\n> that is, where \"56\" was originally there, one side made it to\n> \"ABCDE\" and the other \"AXCYE\".\n\nYes, that is what diff3 would do now (because it does not do any hunk\nrefinement at all), and should continue doing.\n\n> You could make it \"1234789\", and that is\n> technically correct (what there were in the shared original for the\n> conflicted part is 5 and then 6), but the representation pretends\n> that it knows more than there actually is information, which may be\n> somewhat misleading. All these three are equally plausible split of\n> the original \"56\":\n> \n> \t1234789\n> \t1234789\n> \t1234789\n> \n> and picking one over others would be a mere heuristic. All three\n> are technically correct representations and it is just the matter of\n> which one is the easiest to understand. So, this is the kind of\n> \"misleading but not incorrect\".\n\nYes, I agree it is a heuristic about which part of a split hunk to place\ndeleted preimage lines in. Conceptually, I'm OK with that; the point of\nzdiff3 is to try to make the conflict easier to read by eliminating\npossibly uninteresting parts. It doesn't have to be right all the time;\nit just has to be useful most of the time. But it's not clear how true\nthat would be in real life.\n\nI think this is somewhat a moot point, though. We do not do this\nsplitting now. If we later learn to do it, there is nothing to say that\nzdiff3 would have to adopt it also; it could stop at a lower\nzealous-level than the regular merge markers. I think I'd want to\nexperiment with it and see some real-world examples before making a\ndecision on that.\n\n> In all these cases, the middle part would look like this:\n> \n> \t<<<<<<< ours\n> C\n> ||||||| base\n> =======\n> \tC\n> >>>>>>> theirs\n> \n> in order to honor the explicit \"I want to view all three versions to\n> examine the situation\" aka \"--conflict=diff3\" option. We cannot\n> reduce it to just \"C\". That will make it \"not just misleading but\n> is actively wrong\".\n\nI'm not sure I agree. In this output (which does the zealous\nsimplification, the splitting, and arbitrarily assigns deleted preimage\nto the first of the split hunks):\n\n 1234ACE789\n\nI do not see the promotion of C to \"already resolved, you cannot tell if\nit was really in the preimage or not\" as any more or less misleading or\nwrong than that of A or E. It is no more misleading than what the\nmerge-marker case would do, which would be:\n\n 1234ACE789\n\nThe wrong thing to me is the arbitrary choice about how to distribute\nthe preimage lines. In this example, it is not a big deal for the\nheuristic to be wrong; you can see both of the hunks. But if C is long,\nand you do not even see D=Y while resolving B=X, seeing the preimage\nthere may become nonsensical.\n\nBut again, we don't do this splitting now. So I don't think it's\nsomething that should make or break a decision to have zdiff3. Without\nthe splitting, I can see it being quite useful. I'm going to carry the\npatch in my tree for a while and try using it in practice for a while.\n\n-Peff\n"},{"id":"210776","messageId":"7vk3pjm5of.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"7v1ubrnmtu.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-07T18:21:52Z","receivedAt":"2013-03-07T18:21:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> You could make it \"1234789\", and that is\n> technically correct (what there were in the shared original for the\n> conflicted part is 5 and then 6), but the representation pretends\n> that it knows more than there actually is information, which may be\n> somewhat misleading. All these three are equally plausible split of\n> the original \"56\":\n>\n> \t1234789\n> \t1234789\n> \t1234789\n>\n> and picking one over others would be a mere heuristic. All three\n> are technically correct representations and it is just the matter of\n> which one is the easiest to understand. So, this is the kind of\n> \"misleading but not incorrect\".\n\nI forgot to say that youu could even do something silly like:\n\n\t1234789\n\n;-)\n\n> In all these cases, the middle part would look like this:\n>\n> <<<<<<< ours\n> C\n> ||||||| base\n> =======\n> C\n> >>>>>>> theirs\n>\n> in order to honor the explicit \"I want to view all three versions to\n> examine the situation\" aka \"--conflict=diff3\" option. We cannot\n> reduce it to just \"C\". That will make it \"not just misleading but\n> is actively wrong\".\n\nI also forgot to say that the issue is the same to reduce\n\n \t1234789\n\nto\n\n \t1234789\n\nwhich is unconditionally correct and then for all x reduce to\nx, yielding\n\n \t1234ACE789\n\nwhich your zealous-diff3 would do. So squashing that in the\nmiddle would be consistent if you take the zealous-diff3 route.\n\nBut again, that is discarding the information of the original, which\nthe user explicitly asked from \"diff3 -m\", i.e. show all three to\nexamine the situation. If the user wants to operate _without_ the\noriginal, the user would have asked for \"RCS merge\" style output, so\nI am still not sure if that is a sensible mode of operation for diff3\nto begin with.\n\n\n\n\t\n"},{"id":"210779","messageId":"7vfw07m4sx.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"20130307180157.GA6604@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-07T18:40:46Z","receivedAt":"2013-03-07T18:40:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I'm not sure I agree. In this output (which does the zealous\n> simplification, the splitting, and arbitrarily assigns deleted preimage\n> to the first of the split hunks):\n>\n> 1234ACE789\n>\n> I do not see the promotion of C to \"already resolved, you cannot tell if\n> it was really in the preimage or not\" as any more or less misleading or\n> wrong than that of A or E. It is no more misleading than what the\n> merge-marker case would do, which would be:\n>\n> 1234ACE789\n\nThat is exactly my point and I think we are in complete agreement.\nWhile the intended difference between RCS merge and diff3 -m is for\nthe latter not to lose information on the original, zealous-diff3\nchooses to lose information in \"both sides added, identically\" case.\n\nWhere we differ is if such information loss is a good thing to have.\n\nWe could say \"both sides added, identically\" is auto-resolved when\nyou use the zealous option, and do so regardless of how the merge\nconflicts are presented. Then it becomes perfectly fine to eject\n\"A\" and \"E\" out of the conflicted block and merge them to be part of\npre/post contexts. The same goes for reducing \"\" to \"C\". As\nlong as we clearly present the users what the option does and what\nits implications are, it is not bad to have such an option, I think.\n\n> The wrong thing to me is the arbitrary choice about how to distribute\n> the preimage lines.\n\nYeah, but that is not \"diff3 -m\" vs \"zealous-diff3\" issue, is it?\nIf you value the original and want to show it somewhere, you cannot\navoid making the choice whether you are zealous or not if you split\nsuch a hunk.\n"},{"id":"210782","messageId":"20130307185046.GA11622@sigill.intra.peff.net","threadId":"33088","inReplyTo":"7vfw07m4sx.fsf@alter.siamese.dyndns.org","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-07T18:50:46Z","receivedAt":"2013-03-07T18:50:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 07, 2013 at 10:40:46AM -0800, Junio C Hamano wrote:\n\n> Where we differ is if such information loss is a good thing to have.\n>\n> We could say \"both sides added, identically\" is auto-resolved when\n> you use the zealous option, and do so regardless of how the merge\n> conflicts are presented. Then it becomes perfectly fine to eject\n> \"A\" and \"E\" out of the conflicted block and merge them to be part of\n> pre/post contexts. The same goes for reducing \"\" to \"C\". As\n> long as we clearly present the users what the option does and what\n> its implications are, it is not bad to have such an option, I think.\n\nExactly. I do think it has real-world uses (see the example script I\nposted yesterday), but it would never replace diff3. I'm going to try it\nout for a bit. As I mentioned yesterday, I see those sorts of\ncherry-pick-with-something-on-top conflicts when I am rebasing onto or\nmerging my topics into what you have picked up from the same topic on\nthe list.\n\nI think the code in Uwe's patch looked fine, but it definitely needs a\ndocumentation change to explain the new mode and its caveats. I'd also\nbe happy with a different name, if you think it implies that it is too\nrelated to zdiff3, but I cannot think of anything better at the moment.\n\n> > The wrong thing to me is the arbitrary choice about how to distribute\n> > the preimage lines.\n> \n> Yeah, but that is not \"diff3 -m\" vs \"zealous-diff3\" issue, is it?\n> If you value the original and want to show it somewhere, you cannot\n> avoid making the choice whether you are zealous or not if you split\n> such a hunk.\n\nRight, but I meant that we would never split a hunk like that with\ndiff3, because we would not do any hunk refinement at all. Splitting a\nhunk with \"merge\" is OK, because the \"where does the preimage go\"\nproblem does not exist there. zdiff3 is the only problematic case,\nbecause it would be the only one that (potentially) splits and cares\nabout how the preimage maps to each hunk. But we can deal with that if\nand when we ever do such splitting.\n\n-Peff\n"},{"id":"213189","messageId":"20130404203344.GA25330@sigill.intra.peff.net","threadId":"33088","inReplyTo":"20130307185046.GA11622@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-04T20:33:44Z","receivedAt":"2013-04-04T20:33:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 07, 2013 at 01:50:46PM -0500, Jeff King wrote:\n\n> On Thu, Mar 07, 2013 at 10:40:46AM -0800, Junio C Hamano wrote:\n> \n> > Where we differ is if such information loss is a good thing to have.\n> >\n> > We could say \"both sides added, identically\" is auto-resolved when\n> > you use the zealous option, and do so regardless of how the merge\n> > conflicts are presented. Then it becomes perfectly fine to eject\n> > \"A\" and \"E\" out of the conflicted block and merge them to be part of\n> > pre/post contexts. The same goes for reducing \"\" to \"C\". As\n> > long as we clearly present the users what the option does and what\n> > its implications are, it is not bad to have such an option, I think.\n> \n> Exactly. I do think it has real-world uses (see the example script I\n> posted yesterday), but it would never replace diff3. I'm going to try it\n> out for a bit. As I mentioned yesterday, I see those sorts of\n> cherry-pick-with-something-on-top conflicts when I am rebasing onto or\n> merging my topics into what you have picked up from the same topic on\n> the list.\n\nI wanted to give an update on how this has been going. I've been running\nwith zdiff3 for almost a month. I keep my merge.conflictstyle set to\ndiff3, and when I see something that I think might benefit from the\n\"both sides added\" zealousness, I do a \"git checkout --conflict=zdiff3\"\nand examine the result.\n\nI have seen it help, and always when rebasing patches that were accepted\nupstream. For example, imagine I added a big block of text in one patch\n(e.g., an entire test script). Then I added more tests in a follow-on\npatch. Or I change some of the lines from expect_failure to\nexpect_success. You can see this in t1060 of the\njk/check-corrupt-objects-carefully topic (I didn't try, but you could\nprobably reproduce by just rebasing it on top of the current master).\n\nWhen I rebase my version of the patches on your master with the new\ncontent, the conflict for the first patch is useless in diff3. I see\nthat the base had nothing, upstream added a hundred lines, and my patch\nadded ninety lines. But it's hard to see which lines are missing or\nmodified because of the size of the conflict. It looks like:\n\n <<<<<<< ours\n #!/bin/sh\n test_description=whatever\n ...\n end of some test\n '\n test_done\n ||||||| base\n =======\n #!/bin/sh\n test_description=whatever\n ...\n end of another test\n '\n test_done\n >>>>>>> theirs\n\nThe interesting part is in the \"...\", which contains different lines in\neach version, but it may be hundreds of lines long. Using zdiff3, I get:\n\n #!/bin/sh\n test_description=whatever\n ...\n <<<<<<< ours\n test_expect_success 'some_new_test' '\n ...\n ||||||| base\n =======\n >>>>>>> theirs\n '\n test_done\n\nI can see that nothing was tweaked; I just didn't add any content there,\nand upstream did. Contrast this with zealous \"merge\" conflicts, which\nwould look like:\n\n #!/bin/sh\n test_description=whatever\n ...\n <<<<<<< ours\n test_expect_success 'some_new_test' '\n ...\n =======\n >>>>>>> theirs\n '\n test_done\n\nwhich similarly condenses, but is missing a piece of information: that\nthere was nothing in the base. I don't know whether the conflict is\nthere because my patch removed some content that got changed upstream,\nor whether upstream added some content that I did not have in my patch.\n\nSo I think it is useful when rebasing on top of what upstream took,\nspecifically when:\n\n 1. You have a series that updates the same hunk repeatedly (because\n from your perspective, you see only the tip of what upstream took).\n\n 2. Upstream takes your patch but tweaks it (either as a fixup, to deal\n with a merge conflict, or whatever). You get to see the minimal\n tweak, not the fact that you have a giant hunk which differs from\n the upstream only by a few characters or a few lines.\n\nSo I do think zdiff3 is useful, and I plan to continue using it.\n\n-Peff\n"},{"id":"213196","messageId":"20130404204944.GB4913@pengutronix.de","threadId":"33088","inReplyTo":"20130404203344.GA25330@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Uwe Kleine-König","fromEmail":"u.kleine-koenig@pengutronix.de","sentAt":"2013-04-04T20:49:44Z","receivedAt":"2013-04-04T20:49:44Z","isPatch":false,"sender":{"key":"u.kleine-koenig@pengutronix.de","avatar":"https://gravatar.com/avatar/354b5e3ceb2806a2f1e1e382ac29ddbdad18288654da62b61eb13583a857eee7?d=mp&s=160"},"body":"Hi Jeff,\n\nOn Thu, Apr 04, 2013 at 04:33:44PM -0400, Jeff King wrote:\n> [...]\n> So I do think zdiff3 is useful, and I plan to continue using it.\nThanks for your description. I'm using and liking zdiff3, too. So I'd\nreally like seeing it in vanilla git.\n\nThanks\nUwe\n\n-- \nPengutronix e.K. | Uwe Kleine-König |\nIndustrial Linux Solutions | http://www.pengutronix.de/ |\n"},{"id":"213197","messageId":"20130404205423.GA25986@sigill.intra.peff.net","threadId":"33088","inReplyTo":"20130404204944.GB4913@pengutronix.de","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-04-04T20:54:23Z","receivedAt":"2013-04-04T20:54:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 04, 2013 at 10:49:44PM +0200, Uwe Kleine-König wrote:\n\n> On Thu, Apr 04, 2013 at 04:33:44PM -0400, Jeff King wrote:\n> > [...]\n> > So I do think zdiff3 is useful, and I plan to continue using it.\n> Thanks for your description. I'm using and liking zdiff3, too. So I'd\n> really like seeing it in vanilla git.\n\nI don't know if Junio is interested in taking a patch with the concept\nor not, but as I recall, your patch needed at least a documentation\nupdate before it could be picked up. Unless Junio wants to say \"no, I am\nnot interested at all\", I think the next step would be to repost an\nupdated version.\n\n-Peff\n"},{"id":"213204","messageId":"7vip42gfjc.fsf@alter.siamese.dyndns.org","threadId":"33088","inReplyTo":"20130404205423.GA25986@sigill.intra.peff.net","subject":"Re: feature suggestion: optimize common parts for checkout --conflict=diff3","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-04T21:19:19Z","receivedAt":"2013-04-04T21:19:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Apr 04, 2013 at 10:49:44PM +0200, Uwe Kleine-König wrote:\n>\n>> On Thu, Apr 04, 2013 at 04:33:44PM -0400, Jeff King wrote:\n>> > [...]\n>> > So I do think zdiff3 is useful, and I plan to continue using it.\n>> Thanks for your description. I'm using and liking zdiff3, too. So I'd\n>> really like seeing it in vanilla git.\n>\n> I don't know if Junio is interested in taking a patch with the concept\n> or not,...\n\nIn two messages upthread before you restarted this discussion, I\nsaid:\n\n As long as we clearly present the users what the option does and\n what its implications are, it is not bad to have such an option,\n I think.\n\n> but as I recall, your patch needed at least a documentation update\n> before it could be picked up. Unless Junio wants to say \"no, I am\n> not interested at all\", I think the next step would be to repost\n> an updated version.\n\nYup.\n"}]}