{"thread":{"id":"15268","subject":"[PATCH 1/2] xdl_fill_merge_buffer(): separate out a too deeply nested function","startedAt":"2008-08-29T00:09:33Z","lastAt":"2008-08-31T18:42:10Z","messageCount":9,"participants":["Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"89060","messageId":"7vzlmwbs1u.fsf@gitster.siamese.dyndns.org","threadId":"15268","inReplyTo":null,"subject":"[PATCH 1/2] xdl_fill_merge_buffer(): separate out a too deeply nested function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T00:09:33Z","receivedAt":"2008-08-29T00:09:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This simply moves code around to make a separate function that prepares\na single conflicted hunk with markers into the buffer.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I am sending this out because I need an extra set of eyeballs and a bit\n   of insight into this code from you for the second part, which is still\n   an early RFC/WIP, and it depends on this refactoring.\n\n xdiff/xmerge.c |  121 ++++++++++++++++++++++++++++++++-----------------------\n 1 files changed, 70 insertions(+), 51 deletions(-)\n\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 82b3573..6ffaa4f 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -113,65 +113,84 @@ static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n \treturn size;\n }\n \n-static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,\n-\t\txdfenv_t *xe2, const char *name2, xdmerge_t *m, char *dest)\n+static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n+\t\t\t      xdfenv_t *xe2, const char *name2,\n+\t\t\t      int size, int i,\n+\t\t\t      xdmerge_t *m, char *dest)\n {\n \tconst int marker_size = 7;\n \tint marker1_size = (name1 ? strlen(name1) + 1 : 0);\n \tint marker2_size = (name2 ? strlen(name2) + 1 : 0);\n-\tint conflict_marker_size = 3 * (marker_size + 1)\n-\t\t+ marker1_size + marker2_size;\n-\tint size, i1, j;\n-\n-\tfor (size = i1 = 0; m; m = m->next) {\n-\t\tif (m->mode == 0) {\n-\t\t\tsize += xdl_recs_copy(xe1, i1, m->i1 - i1, 0,\n-\t\t\t\t\tdest ? dest + size : NULL);\n-\t\t\tif (dest) {\n-\t\t\t\tfor (j = 0; j < marker_size; j++)\n-\t\t\t\t\tdest[size++] = '<';\n-\t\t\t\tif (marker1_size) {\n-\t\t\t\t\tdest[size] = ' ';\n-\t\t\t\t\tmemcpy(dest + size + 1, name1,\n-\t\t\t\t\t\t\tmarker1_size - 1);\n-\t\t\t\t\tsize += marker1_size;\n-\t\t\t\t}\n-\t\t\t\tdest[size++] = '\\n';\n-\t\t\t} else\n-\t\t\t\tsize += conflict_marker_size;\n-\t\t\tsize += xdl_recs_copy(xe1, m->i1, m->chg1, 1,\n-\t\t\t\t\tdest ? dest + size : NULL);\n-\t\t\tif (dest) {\n-\t\t\t\tfor (j = 0; j < marker_size; j++)\n-\t\t\t\t\tdest[size++] = '=';\n-\t\t\t\tdest[size++] = '\\n';\n-\t\t\t}\n-\t\t\tsize += xdl_recs_copy(xe2, m->i2, m->chg2, 1,\n-\t\t\t\t\tdest ? dest + size : NULL);\n-\t\t\tif (dest) {\n-\t\t\t\tfor (j = 0; j < marker_size; j++)\n-\t\t\t\t\tdest[size++] = '>';\n-\t\t\t\tif (marker2_size) {\n-\t\t\t\t\tdest[size] = ' ';\n-\t\t\t\t\tmemcpy(dest + size + 1, name2,\n-\t\t\t\t\t\t\tmarker2_size - 1);\n-\t\t\t\t\tsize += marker2_size;\n-\t\t\t\t}\n-\t\t\t\tdest[size++] = '\\n';\n-\t\t\t}\n-\t\t} else if (m->mode == 1)\n-\t\t\tsize += xdl_recs_copy(xe1, i1, m->i1 + m->chg1 - i1, 0,\n-\t\t\t\t\tdest ? dest + size : NULL);\n+\tint j;\n+\n+\t/* Before conflicting part */\n+\tsize += xdl_recs_copy(xe1, i, m->i1 - i, 0,\n+\t\t\t      dest ? dest + size : NULL);\n+\n+\tif (!dest) {\n+\t\tsize += marker_size + 1 + marker1_size;\n+\t} else {\n+\t\tfor (j = 0; j < marker_size; j++)\n+\t\t\tdest[size++] = '<';\n+\t\tif (marker1_size) {\n+\t\t\tdest[size] = ' ';\n+\t\t\tmemcpy(dest + size + 1, name1, marker1_size - 1);\n+\t\t\tsize += marker1_size;\n+\t\t}\n+\t\tdest[size++] = '\\n';\n+\t}\n+\n+\t/* Postimage from side #1 */\n+\tsize += xdl_recs_copy(xe1, m->i1, m->chg1, 1,\n+\t\t\t      dest ? dest + size : NULL);\n+\tif (!dest) {\n+\t\tsize += marker_size + 1;\n+\t} else {\n+\t\tfor (j = 0; j < marker_size; j++)\n+\t\t\tdest[size++] = '=';\n+\t\tdest[size++] = '\\n';\n+\t}\n+\n+\t/* Postimage from side #2 */\n+\tsize += xdl_recs_copy(xe2, m->i2, m->chg2, 1,\n+\t\t\t      dest ? dest + size : NULL);\n+\tif (!dest) {\n+\t\tsize += marker_size + 1 + marker2_size;\n+\t} else {\n+\t\tfor (j = 0; j < marker_size; j++)\n+\t\t\tdest[size++] = '>';\n+\t\tif (marker2_size) {\n+\t\t\tdest[size] = ' ';\n+\t\t\tmemcpy(dest + size + 1, name2, marker2_size - 1);\n+\t\t\tsize += marker2_size;\n+\t\t}\n+\t\tdest[size++] = '\\n';\n+\t}\n+\treturn size;\n+}\n+\n+static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,\n+\t\txdfenv_t *xe2, const char *name2, xdmerge_t *m, char *dest)\n+{\n+\tint size, i;\n+\n+\tfor (size = i = 0; m; m = m->next) {\n+\t\tif (m->mode == 0)\n+\t\t\tsize = fill_conflict_hunk(xe1, name1, xe2, name2,\n+\t\t\t\t\t\t  size, i, m, dest);\n+\t\telse if (m->mode == 1)\n+\t\t\tsize += xdl_recs_copy(xe1, i, m->i1 + m->chg1 - i, 0,\n+\t\t\t\t\t      dest ? dest + size : NULL);\n \t\telse if (m->mode == 2)\n-\t\t\tsize += xdl_recs_copy(xe2, m->i2 - m->i1 + i1,\n-\t\t\t\t\tm->i1 + m->chg2 - i1, 0,\n-\t\t\t\t\tdest ? dest + size : NULL);\n+\t\t\tsize += xdl_recs_copy(xe2, m->i2 - m->i1 + i,\n+\t\t\t\t\t      m->i1 + m->chg2 - i, 0,\n+\t\t\t\t\t      dest ? dest + size : NULL);\n \t\telse\n \t\t\tcontinue;\n-\t\ti1 = m->i1 + m->chg1;\n+\t\ti = m->i1 + m->chg1;\n \t}\n-\tsize += xdl_recs_copy(xe1, i1, xe1->xdf2.nrec - i1, 0,\n-\t\t\tdest ? dest + size : NULL);\n+\tsize += xdl_recs_copy(xe1, i, xe1->xdf2.nrec - i, 0,\n+\t\t\t      dest ? dest + size : NULL);\n \treturn size;\n }\n \n-- \n1.6.0.1.200.ge682f\n"},{"id":"89065","messageId":"7vsksobrn9.fsf@gitster.siamese.dyndns.org","threadId":"15268","inReplyTo":"7vzlmwbs1u.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T00:18:18Z","receivedAt":"2008-08-29T00:18:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When showing conflicting merges, we traditionally followed RCS's merge\noutput format.  The output shows:\n\n <<<<<<<\n postimage from one side;\n =======\n postimage of the other side; and\n >>>>>>>\n\nSome poeple find it easier to be able to understand what is going on when\nthey can view the common ancestor's version, which is used by \"diff3 -m\",\nwhich shows:\n\n <<<<<<<\n postimage from one side;\n |||||||\n shared preimage;\n =======\n postimage of the other side; and\n >>>>>>>\n\nThis is an initial step to bring that as an optional feature to git.\nOnly \"git merge-file\" has been converted, with \"--diff3\" option.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This is RFH/WIP partly because I tested it only on simplest cases, but\n   more importantly I suspect this should be orthogonal to the refinement\n   level parameter.\n\n   The idea is to keep (just like <i1,chg1>, <i2,chg2> you use to point at\n   the postimage of both sides) <i0,chg0> in xdmerge_t that point at the\n   corresponding part of the shared preimage.  I think I got the initial\n   computation of <i0,chg0>, coalescing done in xdl_append_merge(), and\n   use of that in xdl_fill_merge_buffer() right, but I couldn't figure out\n   how to adjust them inside the merge refinement logic.\n\n   Of course the eventual goal is to add this to merge-file.c, give a new\n   configuration variable to allow people to use this, while teaching\n   rerere to ignore the new '|||||||' + shared preimage section, so that\n   existing rerere information can be reused for new conflicts in\n   diff3 style output.\n\n builtin-merge-file.c |    6 ++-\n xdiff/xdiff.h        |    5 +-\n xdiff/xmerge.c       |  145 ++++++++++++++++++++++++++++++++++++++------------\n 3 files changed, 119 insertions(+), 37 deletions(-)\n\ndiff --git a/builtin-merge-file.c b/builtin-merge-file.c\nindex 3605960..2f69aa1 100644\n--- a/builtin-merge-file.c\n+++ b/builtin-merge-file.c\n@@ -13,6 +13,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tmmbuffer_t result = {NULL, 0};\n \txpparam_t xpp = {XDF_NEED_MINIMAL};\n \tint ret = 0, i = 0, to_stdout = 0;\n+\tint level = XDL_MERGE_ZEALOUS_ALNUM;\n \n \twhile (argc > 4) {\n \t\tif (!strcmp(argv[1], \"-L\") && i < 3) {\n@@ -25,6 +26,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(argv[1], \"-q\") ||\n \t\t\t\t!strcmp(argv[1], \"--quiet\"))\n \t\t\tfreopen(\"/dev/null\", \"w\", stderr);\n+\t\telse if (!strcmp(argv[1], \"-3\") ||\n+\t\t\t !strcmp(argv[1], \"--diff3\"))\n+\t\t\tlevel = XDL_MERGE_DIFF3;\n \t\telse\n \t\t\tusage(merge_file_usage);\n \t\targc--;\n@@ -46,7 +50,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t}\n \n \tret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],\n-\t\t\t&xpp, XDL_MERGE_ZEALOUS_ALNUM, &result);\n+\t\t\t&xpp, level, &result);\n \n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfs[i].ptr);\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 413082e..a7b6e08 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -52,8 +52,9 @@ extern \"C\" {\n \n #define XDL_MERGE_MINIMAL 0\n #define XDL_MERGE_EAGER 1\n-#define XDL_MERGE_ZEALOUS 2\n-#define XDL_MERGE_ZEALOUS_ALNUM 3\n+#define XDL_MERGE_DIFF3 2\n+#define XDL_MERGE_ZEALOUS 10\n+#define XDL_MERGE_ZEALOUS_ALNUM 11\n \n typedef struct s_mmfile {\n \tchar *ptr;\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 6ffaa4f..ac6cc40 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -30,17 +30,32 @@ typedef struct s_xdmerge {\n \t * 2 = no conflict, take second.\n \t */\n \tint mode;\n+\t/*\n+\t * These point at the respective postimages.  E.g. <i1,chg1> is\n+\t * how side #1 wants to change the common ancestor; if there is no\n+\t * overlap, lines before i1 in the postimage of side #1 appear\n+\t * in the merge result as a region touched by neither side.\n+\t */\n \tlong i1, i2;\n \tlong chg1, chg2;\n+\t/*\n+\t * These point at the preimage; of course there is just one\n+\t * preimage, that is from the shared common ancestor.\n+\t */\n+\tlong i0;\n+\tlong chg0;\n } xdmerge_t;\n \n static int xdl_append_merge(xdmerge_t **merge, int mode,\n-\t\tlong i1, long chg1, long i2, long chg2)\n+\t\t\t    long i0, long chg0,\n+\t\t\t    long i1, long chg1,\n+\t\t\t    long i2, long chg2)\n {\n \txdmerge_t *m = *merge;\n \tif (m && (i1 <= m->i1 + m->chg1 || i2 <= m->i2 + m->chg2)) {\n \t\tif (mode != m->mode)\n \t\t\tm->mode = 0;\n+\t\tm->chg0 = i0 + chg0 - m->i0;\n \t\tm->chg1 = i1 + chg1 - m->i1;\n \t\tm->chg2 = i2 + chg2 - m->i2;\n \t} else {\n@@ -49,6 +64,8 @@ static int xdl_append_merge(xdmerge_t **merge, int mode,\n \t\t\treturn -1;\n \t\tm->next = NULL;\n \t\tm->mode = mode;\n+\t\tm->i0 = i0;\n+\t\tm->chg0 = chg0;\n \t\tm->i1 = i1;\n \t\tm->chg1 = chg1;\n \t\tm->i2 = i2;\n@@ -91,11 +108,13 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,\n \treturn 0;\n }\n \n-static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n+static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n {\n-\txrecord_t **recs = xe->xdf2.recs + i;\n+\txrecord_t **recs;\n \tint size = 0;\n \n+\trecs = (use_orig ? xe->xdf1.recs : xe->xdf2.recs) + i;\n+\n \tif (count < 1)\n \t\treturn 0;\n \n@@ -113,9 +132,19 @@ static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n \treturn size;\n }\n \n+static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n+{\n+\treturn xdl_recs_copy_0(0, xe, i, count, add_nl, dest);\n+}\n+\n+static int xdl_orig_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n+{\n+\treturn xdl_recs_copy_0(1, xe, i, count, add_nl, dest);\n+}\n+\n static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \t\t\t      xdfenv_t *xe2, const char *name2,\n-\t\t\t      int size, int i,\n+\t\t\t      int size, int i, int level,\n \t\t\t      xdmerge_t *m, char *dest)\n {\n \tconst int marker_size = 7;\n@@ -143,6 +172,20 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \t/* Postimage from side #1 */\n \tsize += xdl_recs_copy(xe1, m->i1, m->chg1, 1,\n \t\t\t      dest ? dest + size : NULL);\n+\n+\tif (level == XDL_MERGE_DIFF3) {\n+\t\t/* Shared preimage */\n+\t\tif (!dest) {\n+\t\t\tsize += marker_size + 1;\n+\t\t} else {\n+\t\t\tfor (j = 0; j < marker_size; j++)\n+\t\t\t\tdest[size++] = '|';\n+\t\t\tdest[size++] = '\\n';\n+\t\t}\n+\t\tsize += xdl_orig_copy(xe1, m->i0, m->chg0, 1,\n+\t\t\t\t      dest ? dest + size : NULL);\n+\t}\n+\n \tif (!dest) {\n \t\tsize += marker_size + 1;\n \t} else {\n@@ -170,14 +213,15 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n }\n \n static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,\n-\t\txdfenv_t *xe2, const char *name2, xdmerge_t *m, char *dest)\n+\t\t\t\t xdfenv_t *xe2, const char *name2,\n+\t\t\t\t xdmerge_t *m, char *dest, int level)\n {\n \tint size, i;\n \n \tfor (size = i = 0; m; m = m->next) {\n \t\tif (m->mode == 0)\n \t\t\tsize = fill_conflict_hunk(xe1, name1, xe2, name2,\n-\t\t\t\t\t\t  size, i, m, dest);\n+\t\t\t\t\t\t  size, i, level, m, dest);\n \t\telse if (m->mode == 1)\n \t\t\tsize += xdl_recs_copy(xe1, i, m->i1 + m->chg1 - i, 0,\n \t\t\t\t\t      dest ? dest + size : NULL);\n@@ -332,19 +376,31 @@ static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m,\n }\n \n /*\n- * level == 0: mark all overlapping changes as conflict\n- * level == 1: mark overlapping changes as conflict only if not identical\n- * level == 2: analyze non-identical changes for minimal conflict set\n- * level == 3: analyze non-identical changes for minimal conflict set, but\n- *             treat hunks not containing any letter or number as conflicting\n+ * \"Level\" parameter can be:\n+ *\n+ * (MINIMAL):\n+ * Mark all overlapping changes as conflict\n+ *\n+ * (EAGER):\n+ * Mark overlapping changes as conflict only if not identical\n+ *\n+ * (DIFF3):\n+ * Same as EAGER but show the shared preimage in the output as well\n+ *\n+ * (ZEALOUS):\n+ * Analyze non-identical changes for minimal conflict set\n+ *\n+ * (ZEALOUS_ALNUM):\n+ * Analyze non-identical changes for minimal conflict set, but\n+ * treat hunks not containing any letter or number as conflicting\n  *\n  * returns < 0 on error, == 0 for no conflicts, else number of conflicts\n  */\n static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n-\t\txdfenv_t *xe2, xdchange_t *xscr2, const char *name2,\n-\t\tint level, xpparam_t const *xpp, mmbuffer_t *result) {\n+\t\t\txdfenv_t *xe2, xdchange_t *xscr2, const char *name2,\n+\t\t\tint level, xpparam_t const *xpp, mmbuffer_t *result) {\n \txdmerge_t *changes, *c;\n-\tint i1, i2, chg1, chg2;\n+\tint i0, i1, i2, chg0, chg1, chg2;\n \n \tc = changes = NULL;\n \n@@ -352,11 +408,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\tif (!changes)\n \t\t\tchanges = c;\n \t\tif (xscr1->i1 + xscr1->chg1 < xscr2->i1) {\n+\t\t\ti0 = xscr1->i1;\n \t\t\ti1 = xscr1->i2;\n \t\t\ti2 = xscr2->i2 - xscr2->i1 + xscr1->i1;\n+\t\t\tchg0 = xscr1->chg1;\n \t\t\tchg1 = xscr1->chg2;\n \t\t\tchg2 = xscr1->chg1;\n-\t\t\tif (xdl_append_merge(&c, 1, i1, chg1, i2, chg2)) {\n+\t\t\tif (xdl_append_merge(&c, 1,\n+\t\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\t\txdl_cleanup_merge(changes);\n \t\t\t\treturn -1;\n \t\t\t}\n@@ -364,40 +423,50 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\t\tcontinue;\n \t\t}\n \t\tif (xscr2->i1 + xscr2->chg1 < xscr1->i1) {\n+\t\t\ti0 = xscr2->i1;\n \t\t\ti1 = xscr1->i2 - xscr1->i1 + xscr2->i1;\n \t\t\ti2 = xscr2->i2;\n+\t\t\tchg0 = xscr2->chg1;\n \t\t\tchg1 = xscr2->chg1;\n \t\t\tchg2 = xscr2->chg2;\n-\t\t\tif (xdl_append_merge(&c, 2, i1, chg1, i2, chg2)) {\n+\t\t\tif (xdl_append_merge(&c, 2,\n+\t\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\t\txdl_cleanup_merge(changes);\n \t\t\t\treturn -1;\n \t\t\t}\n \t\t\txscr2 = xscr2->next;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (level < 1 || xscr1->i1 != xscr2->i1 ||\n-\t\t\t\txscr1->chg1 != xscr2->chg1 ||\n-\t\t\t\txscr1->chg2 != xscr2->chg2 ||\n-\t\t\t\txdl_merge_cmp_lines(xe1, xscr1->i2,\n+\t\tif (level == XDL_MERGE_MINIMAL ||\n+\t\t    (xscr1->i1 != xscr2->i1 ||\n+\t\t     xscr1->chg1 != xscr2->chg1 ||\n+\t\t     xscr1->chg2 != xscr2->chg2) ||\n+\t\t    xdl_merge_cmp_lines(xe1, xscr1->i2,\n \t\t\t\t\txe2, xscr2->i2,\n \t\t\t\t\txscr1->chg2, xpp->flags)) {\n \t\t\t/* conflict */\n \t\t\tint off = xscr1->i1 - xscr2->i1;\n \t\t\tint ffo = off + xscr1->chg1 - xscr2->chg1;\n \n+\t\t\ti0 = xscr1->i1;\n \t\t\ti1 = xscr1->i2;\n \t\t\ti2 = xscr2->i2;\n-\t\t\tif (off > 0)\n+\t\t\tif (off > 0) {\n+\t\t\t\ti0 -= off;\n \t\t\t\ti1 -= off;\n+\t\t\t}\n \t\t\telse\n \t\t\t\ti2 += off;\n+\t\t\tchg0 = xscr1->i1 + xscr1->chg1 - i0;\n \t\t\tchg1 = xscr1->i2 + xscr1->chg2 - i1;\n \t\t\tchg2 = xscr2->i2 + xscr2->chg2 - i2;\n-\t\t\tif (ffo > 0)\n-\t\t\t\tchg2 += ffo;\n-\t\t\telse\n+\t\t\tif (ffo < 0) {\n+\t\t\t\tchg0 -= ffo;\n \t\t\t\tchg1 -= ffo;\n-\t\t\tif (xdl_append_merge(&c, 0, i1, chg1, i2, chg2)) {\n+\t\t\t} else\n+\t\t\t\tchg2 += ffo;\n+\t\t\tif (xdl_append_merge(&c, 0,\n+\t\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\t\txdl_cleanup_merge(changes);\n \t\t\t\treturn -1;\n \t\t\t}\n@@ -414,11 +483,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \twhile (xscr1) {\n \t\tif (!changes)\n \t\t\tchanges = c;\n+\t\ti0 = xscr1->i1;\n \t\ti1 = xscr1->i2;\n \t\ti2 = xscr1->i1 + xe2->xdf2.nrec - xe2->xdf1.nrec;\n+\t\tchg0 = xscr1->chg1;\n \t\tchg1 = xscr1->chg2;\n \t\tchg2 = xscr1->chg1;\n-\t\tif (xdl_append_merge(&c, 1, i1, chg1, i2, chg2)) {\n+\t\tif (xdl_append_merge(&c, 1,\n+\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\txdl_cleanup_merge(changes);\n \t\t\treturn -1;\n \t\t}\n@@ -427,11 +499,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \twhile (xscr2) {\n \t\tif (!changes)\n \t\t\tchanges = c;\n+\t\ti0 = xscr2->i1;\n \t\ti1 = xscr2->i1 + xe1->xdf2.nrec - xe1->xdf1.nrec;\n \t\ti2 = xscr2->i2;\n+\t\tchg0 = xscr2->chg1;\n \t\tchg1 = xscr2->chg1;\n \t\tchg2 = xscr2->chg2;\n-\t\tif (xdl_append_merge(&c, 2, i1, chg1, i2, chg2)) {\n+\t\tif (xdl_append_merge(&c, 2,\n+\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\txdl_cleanup_merge(changes);\n \t\t\treturn -1;\n \t\t}\n@@ -440,16 +515,17 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \tif (!changes)\n \t\tchanges = c;\n \t/* refine conflicts */\n-\tif (level > 1 &&\n+\tif (XDL_MERGE_ZEALOUS <= level &&\n \t    (xdl_refine_conflicts(xe1, xe2, changes, xpp) < 0 ||\n-\t     xdl_simplify_non_conflicts(xe1, changes, level > 2) < 0)) {\n+\t     xdl_simplify_non_conflicts(xe1, changes,\n+\t\t\t\t\tlevel > XDL_MERGE_ZEALOUS) < 0)) {\n \t\txdl_cleanup_merge(changes);\n \t\treturn -1;\n \t}\n \t/* output */\n \tif (result) {\n \t\tint size = xdl_fill_merge_buffer(xe1, name1, xe2, name2,\n-\t\t\tchanges, NULL);\n+\t\t\t\t\t\t changes, NULL, level);\n \t\tresult->ptr = xdl_malloc(size);\n \t\tif (!result->ptr) {\n \t\t\txdl_cleanup_merge(changes);\n@@ -457,14 +533,15 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\t}\n \t\tresult->size = size;\n \t\txdl_fill_merge_buffer(xe1, name1, xe2, name2, changes,\n-\t\t\t\tresult->ptr);\n+\t\t\t\t      result->ptr, level);\n \t}\n \treturn xdl_cleanup_merge(changes);\n }\n \n-int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,\n-\t\tmmfile_t *mf2, const char *name2,\n-\t\txpparam_t const *xpp, int level, mmbuffer_t *result) {\n+int xdl_merge(mmfile_t *orig,\n+\t      mmfile_t *mf1, const char *name1,\n+\t      mmfile_t *mf2, const char *name2,\n+\t      xpparam_t const *xpp, int level, mmbuffer_t *result) {\n \txdchange_t *xscr1, *xscr2;\n \txdfenv_t xe1, xe2;\n \tint status;\n-- \n1.6.0.1.200.ge682f\n"},{"id":"89068","messageId":"alpine.LFD.1.10.0808281727490.3300@nehalem.linux-foundation.org","threadId":"15268","inReplyTo":"7vsksobrn9.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-08-29T00:34:36Z","receivedAt":"2008-08-29T00:34:36Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 28 Aug 2008, Junio C Hamano wrote:\n> \n> Some poeple find it easier to be able to understand what is going on when\n> they can view the common ancestor's version, which is used by \"diff3 -m\",\n> which shows:\n> \n>  <<<<<<<\n>  postimage from one side;\n>  |||||||\n>  shared preimage;\n>  =======\n>  postimage of the other side; and\n>  >>>>>>>\n> \n> This is an initial step to bring that as an optional feature to git.\n> Only \"git merge-file\" has been converted, with \"--diff3\" option.\n\nIf you have the common ancestor, why would you ever want this format, and \nnot a nice conflict entry in the index?\n\nAnyway, that's irrelevant for my real question, which is:\n\n>  /*\n> - * level == 0: mark all overlapping changes as conflict\n> - * level == 1: mark overlapping changes as conflict only if not identical\n> - * level == 2: analyze non-identical changes for minimal conflict set\n> - * level == 3: analyze non-identical changes for minimal conflict set, but\n> - *             treat hunks not containing any letter or number as conflicting\n> + * \"Level\" parameter can be:\n> + *\n> + * (MINIMAL):\n> + * Mark all overlapping changes as conflict\n> + *\n> + * (EAGER):\n> + * Mark overlapping changes as conflict only if not identical\n> + *\n> + * (DIFF3):\n> + * Same as EAGER but show the shared preimage in the output as well\n> + *\n> + * (ZEALOUS):\n> + * Analyze non-identical changes for minimal conflict set\n> + *\n> + * (ZEALOUS_ALNUM):\n> + * Analyze non-identical changes for minimal conflict set, but\n> + * treat hunks not containing any letter or number as conflicting\n>   *\n\nWouldn't it be much nicer to make this a bitflag, than an enumeration of \ndifferent models?\n\nIn particular, why would it be wrong to want to be ZEALOUS (possibly \n_ALNUM) and still want DIFF3 output?\n\nIOW, I don't think these are at all disjoint sets. In fact, I think DIFF3 \nsounds not at all like a \"level\" to me, but like an output format thing, \nso it's in a totally different \"address space\".\n\nHmm?\n\n\t\tLinus\n"},{"id":"89071","messageId":"7vmyiwbpe2.fsf@gitster.siamese.dyndns.org","threadId":"15268","inReplyTo":"alpine.LFD.1.10.0808281727490.3300@nehalem.linux-foundation.org","subject":"Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T01:07:01Z","receivedAt":"2008-08-29T01:07:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Thu, 28 Aug 2008, Junio C Hamano wrote:\n>> \n>> Some poeple find it easier to be able to understand what is going on when\n>> they can view the common ancestor's version, which is used by \"diff3 -m\",\n>> which shows:\n>> \n>>  <<<<<<<\n>>  postimage from one side;\n>>  |||||||\n>>  shared preimage;\n>>  =======\n>>  postimage of the other side; and\n>>  >>>>>>>\n>> \n>> This is an initial step to bring that as an optional feature to git.\n>> Only \"git merge-file\" has been converted, with \"--diff3\" option.\n>\n> If you have the common ancestor, why would you ever want this format, and \n> not a nice conflict entry in the index?\n\nWe already have that, don't we?  You can think of it as how to present\nthat information without resorting to \"diff :1:path :2:path\".\n\n> Anyway, that's irrelevant for my real question, which is:\n\n... the same thing as I already said after three-dashes, hence RFH/WIP.\n"},{"id":"89089","messageId":"7viqtk8hfo.fsf@gitster.siamese.dyndns.org","threadId":"15268","inReplyTo":"alpine.LFD.1.10.0808281727490.3300@nehalem.linux-foundation.org","subject":"Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T06:27:07Z","receivedAt":"2008-08-29T06:27:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> Anyway, that's irrelevant for my real question, which is:\n> ...\n> Wouldn't it be much nicer to make this a bitflag, than an enumeration of \n> different models?\n>\n> In particular, why would it be wrong to want to be ZEALOUS (possibly \n> _ALNUM) and still want DIFF3 output?\n>\n> IOW, I don't think these are at all disjoint sets. In fact, I think DIFF3 \n> sounds not at all like a \"level\" to me, but like an output format thing, \n> so it's in a totally different \"address space\".\n\nOk, here is a replacement for [2/2].\n\nThis still forces XDL_MERGE_EAGER when \"git merge-file --diff3\" is asked\nfor, because higher level \"merge refinement\" routines do not know anything\nabout the necessary adjustment of the preimage pointers.\n\nI threw away some clean-ups to xdiff/xmerge.c to use symbolic constants\nwhen deciding what merge refinement codepaths are taken, to make it \neasier to review (the clean-up and style fixes should be done\nindependently, not as part of this patch).\n\n builtin-merge-file.c  |   10 ++++-\n xdiff/xdiff.h         |    6 +++\n xdiff/xmerge.c        |  103 ++++++++++++++++++++++++++++++++++++++++---------\n t/t6023-merge-file.sh |   37 +++++++++++++++++\n 4 files changed, 135 insertions(+), 21 deletions(-)\n\ndiff --git c/builtin-merge-file.c w/builtin-merge-file.c\nindex 3605960..5b4f020 100644\n--- c/builtin-merge-file.c\n+++ w/builtin-merge-file.c\n@@ -4,7 +4,7 @@\n #include \"xdiff-interface.h\"\n \n static const char merge_file_usage[] =\n-\"git merge-file [-p | --stdout] [-q | --quiet] [-L name1 [-L orig [-L name2]]] file1 orig_file file2\";\n+\"git merge-file [-p | --stdout] [--diff3] [-q | --quiet] [-L name1 [-L orig [-L name2]]] file1 orig_file file2\";\n \n int cmd_merge_file(int argc, const char **argv, const char *prefix)\n {\n@@ -13,6 +13,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \tmmbuffer_t result = {NULL, 0};\n \txpparam_t xpp = {XDF_NEED_MINIMAL};\n \tint ret = 0, i = 0, to_stdout = 0;\n+\tint merge_level = XDL_MERGE_ZEALOUS_ALNUM;\n+\tint merge_style = 0;\n \n \twhile (argc > 4) {\n \t\tif (!strcmp(argv[1], \"-L\") && i < 3) {\n@@ -25,6 +27,10 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(argv[1], \"-q\") ||\n \t\t\t\t!strcmp(argv[1], \"--quiet\"))\n \t\t\tfreopen(\"/dev/null\", \"w\", stderr);\n+\t\telse if (!strcmp(argv[1], \"--diff3\")) {\n+\t\t\tmerge_style = XDL_MERGE_DIFF3;\n+\t\t\tmerge_level = XDL_MERGE_EAGER;\n+\t\t}\n \t\telse\n \t\t\tusage(merge_file_usage);\n \t\targc--;\n@@ -46,7 +52,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t}\n \n \tret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],\n-\t\t\t&xpp, XDL_MERGE_ZEALOUS_ALNUM, &result);\n+\t\t\t&xpp, merge_level | merge_style, &result);\n \n \tfor (i = 0; i < 3; i++)\n \t\tfree(mmfs[i].ptr);\ndiff --git c/xdiff/xdiff.h w/xdiff/xdiff.h\nindex 413082e..deebe02 100644\n--- c/xdiff/xdiff.h\n+++ w/xdiff/xdiff.h\n@@ -50,10 +50,16 @@ extern \"C\" {\n #define XDL_BDOP_CPY 2\n #define XDL_BDOP_INSB 3\n \n+/* merge simplification levels */\n #define XDL_MERGE_MINIMAL 0\n #define XDL_MERGE_EAGER 1\n #define XDL_MERGE_ZEALOUS 2\n #define XDL_MERGE_ZEALOUS_ALNUM 3\n+#define XDL_MERGE_LEVEL_MASK 0x0f\n+\n+/* merge output styles */\n+#define XDL_MERGE_DIFF3 0x8000\n+#define XDL_MERGE_STYLE_MASK 0x8000\n \n typedef struct s_mmfile {\n \tchar *ptr;\ndiff --git c/xdiff/xmerge.c w/xdiff/xmerge.c\nindex 6ffaa4f..29cdbea 100644\n--- c/xdiff/xmerge.c\n+++ w/xdiff/xmerge.c\n@@ -30,17 +30,32 @@ typedef struct s_xdmerge {\n \t * 2 = no conflict, take second.\n \t */\n \tint mode;\n+\t/*\n+\t * These point at the respective postimages.  E.g. <i1,chg1> is\n+\t * how side #1 wants to change the common ancestor; if there is no\n+\t * overlap, lines before i1 in the postimage of side #1 appear\n+\t * in the merge result as a region touched by neither side.\n+\t */\n \tlong i1, i2;\n \tlong chg1, chg2;\n+\t/*\n+\t * These point at the preimage; of course there is just one\n+\t * preimage, that is from the shared common ancestor.\n+\t */\n+\tlong i0;\n+\tlong chg0;\n } xdmerge_t;\n \n static int xdl_append_merge(xdmerge_t **merge, int mode,\n-\t\tlong i1, long chg1, long i2, long chg2)\n+\t\t\t    long i0, long chg0,\n+\t\t\t    long i1, long chg1,\n+\t\t\t    long i2, long chg2)\n {\n \txdmerge_t *m = *merge;\n \tif (m && (i1 <= m->i1 + m->chg1 || i2 <= m->i2 + m->chg2)) {\n \t\tif (mode != m->mode)\n \t\t\tm->mode = 0;\n+\t\tm->chg0 = i0 + chg0 - m->i0;\n \t\tm->chg1 = i1 + chg1 - m->i1;\n \t\tm->chg2 = i2 + chg2 - m->i2;\n \t} else {\n@@ -49,6 +64,8 @@ static int xdl_append_merge(xdmerge_t **merge, int mode,\n \t\t\treturn -1;\n \t\tm->next = NULL;\n \t\tm->mode = mode;\n+\t\tm->i0 = i0;\n+\t\tm->chg0 = chg0;\n \t\tm->i1 = i1;\n \t\tm->chg1 = chg1;\n \t\tm->i2 = i2;\n@@ -91,11 +108,13 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,\n \treturn 0;\n }\n \n-static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n+static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n {\n-\txrecord_t **recs = xe->xdf2.recs + i;\n+\txrecord_t **recs;\n \tint size = 0;\n \n+\trecs = (use_orig ? xe->xdf1.recs : xe->xdf2.recs) + i;\n+\n \tif (count < 1)\n \t\treturn 0;\n \n@@ -113,9 +132,19 @@ static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n \treturn size;\n }\n \n+static int xdl_recs_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n+{\n+\treturn xdl_recs_copy_0(0, xe, i, count, add_nl, dest);\n+}\n+\n+static int xdl_orig_copy(xdfenv_t *xe, int i, int count, int add_nl, char *dest)\n+{\n+\treturn xdl_recs_copy_0(1, xe, i, count, add_nl, dest);\n+}\n+\n static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \t\t\t      xdfenv_t *xe2, const char *name2,\n-\t\t\t      int size, int i,\n+\t\t\t      int size, int i, int style,\n \t\t\t      xdmerge_t *m, char *dest)\n {\n \tconst int marker_size = 7;\n@@ -143,6 +172,20 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \t/* Postimage from side #1 */\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+\t\t/* Shared preimage */\n+\t\tif (!dest) {\n+\t\t\tsize += marker_size + 1;\n+\t\t} else {\n+\t\t\tfor (j = 0; j < marker_size; j++)\n+\t\t\t\tdest[size++] = '|';\n+\t\t\tdest[size++] = '\\n';\n+\t\t}\n+\t\tsize += xdl_orig_copy(xe1, m->i0, m->chg0, 1,\n+\t\t\t\t      dest ? dest + size : NULL);\n+\t}\n+\n \tif (!dest) {\n \t\tsize += marker_size + 1;\n \t} else {\n@@ -170,14 +213,15 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n }\n \n static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,\n-\t\txdfenv_t *xe2, const char *name2, xdmerge_t *m, char *dest)\n+\t\t\t\t xdfenv_t *xe2, const char *name2,\n+\t\t\t\t xdmerge_t *m, char *dest, int style)\n {\n \tint size, i;\n \n \tfor (size = i = 0; m; m = m->next) {\n \t\tif (m->mode == 0)\n \t\t\tsize = fill_conflict_hunk(xe1, name1, xe2, name2,\n-\t\t\t\t\t\t  size, i, m, dest);\n+\t\t\t\t\t\t  size, i, style, m, dest);\n \t\telse if (m->mode == 1)\n \t\t\tsize += xdl_recs_copy(xe1, i, m->i1 + m->chg1 - i, 0,\n \t\t\t\t\t      dest ? dest + size : NULL);\n@@ -342,9 +386,11 @@ static int xdl_simplify_non_conflicts(xdfenv_t *xe1, xdmerge_t *m,\n  */\n static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\txdfenv_t *xe2, xdchange_t *xscr2, const char *name2,\n-\t\tint level, xpparam_t const *xpp, mmbuffer_t *result) {\n+\t\tint flags, xpparam_t const *xpp, mmbuffer_t *result) {\n \txdmerge_t *changes, *c;\n-\tint i1, i2, chg1, chg2;\n+\tint i0, i1, i2, chg0, chg1, chg2;\n+\tint level = flags & XDL_MERGE_LEVEL_MASK;\n+\tint style = flags & XDL_MERGE_STYLE_MASK;\n \n \tc = changes = NULL;\n \n@@ -352,11 +398,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\tif (!changes)\n \t\t\tchanges = c;\n \t\tif (xscr1->i1 + xscr1->chg1 < xscr2->i1) {\n+\t\t\ti0 = xscr1->i1;\n \t\t\ti1 = xscr1->i2;\n \t\t\ti2 = xscr2->i2 - xscr2->i1 + xscr1->i1;\n+\t\t\tchg0 = xscr1->chg1;\n \t\t\tchg1 = xscr1->chg2;\n \t\t\tchg2 = xscr1->chg1;\n-\t\t\tif (xdl_append_merge(&c, 1, i1, chg1, i2, chg2)) {\n+\t\t\tif (xdl_append_merge(&c, 1,\n+\t\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\t\txdl_cleanup_merge(changes);\n \t\t\t\treturn -1;\n \t\t\t}\n@@ -364,11 +413,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\t\tcontinue;\n \t\t}\n \t\tif (xscr2->i1 + xscr2->chg1 < xscr1->i1) {\n+\t\t\ti0 = xscr2->i1;\n \t\t\ti1 = xscr1->i2 - xscr1->i1 + xscr2->i1;\n \t\t\ti2 = xscr2->i2;\n+\t\t\tchg0 = xscr2->chg1;\n \t\t\tchg1 = xscr2->chg1;\n \t\t\tchg2 = xscr2->chg2;\n-\t\t\tif (xdl_append_merge(&c, 2, i1, chg1, i2, chg2)) {\n+\t\t\tif (xdl_append_merge(&c, 2,\n+\t\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\t\txdl_cleanup_merge(changes);\n \t\t\t\treturn -1;\n \t\t\t}\n@@ -385,19 +437,26 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\t\tint off = xscr1->i1 - xscr2->i1;\n \t\t\tint ffo = off + xscr1->chg1 - xscr2->chg1;\n \n+\t\t\ti0 = xscr1->i1;\n \t\t\ti1 = xscr1->i2;\n \t\t\ti2 = xscr2->i2;\n-\t\t\tif (off > 0)\n+\t\t\tif (off > 0) {\n+\t\t\t\ti0 -= off;\n \t\t\t\ti1 -= off;\n+\t\t\t}\n \t\t\telse\n \t\t\t\ti2 += off;\n+\t\t\tchg0 = xscr1->i1 + xscr1->chg1 - i0;\n \t\t\tchg1 = xscr1->i2 + xscr1->chg2 - i1;\n \t\t\tchg2 = xscr2->i2 + xscr2->chg2 - i2;\n \t\t\tif (ffo > 0)\n \t\t\t\tchg2 += ffo;\n-\t\t\telse\n+\t\t\telse {\n+\t\t\t\tchg0 -= ffo;\n \t\t\t\tchg1 -= ffo;\n-\t\t\tif (xdl_append_merge(&c, 0, i1, chg1, i2, chg2)) {\n+\t\t\t}\n+\t\t\tif (xdl_append_merge(&c, 0,\n+\t\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\t\txdl_cleanup_merge(changes);\n \t\t\t\treturn -1;\n \t\t\t}\n@@ -414,11 +473,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \twhile (xscr1) {\n \t\tif (!changes)\n \t\t\tchanges = c;\n+\t\ti0 = xscr1->i1;\n \t\ti1 = xscr1->i2;\n \t\ti2 = xscr1->i1 + xe2->xdf2.nrec - xe2->xdf1.nrec;\n+\t\tchg0 = xscr1->chg1;\n \t\tchg1 = xscr1->chg2;\n \t\tchg2 = xscr1->chg1;\n-\t\tif (xdl_append_merge(&c, 1, i1, chg1, i2, chg2)) {\n+\t\tif (xdl_append_merge(&c, 1,\n+\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\txdl_cleanup_merge(changes);\n \t\t\treturn -1;\n \t\t}\n@@ -427,11 +489,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \twhile (xscr2) {\n \t\tif (!changes)\n \t\t\tchanges = c;\n+\t\ti0 = xscr2->i1;\n \t\ti1 = xscr2->i1 + xe1->xdf2.nrec - xe1->xdf1.nrec;\n \t\ti2 = xscr2->i2;\n+\t\tchg0 = xscr2->chg1;\n \t\tchg1 = xscr2->chg1;\n \t\tchg2 = xscr2->chg2;\n-\t\tif (xdl_append_merge(&c, 2, i1, chg1, i2, chg2)) {\n+\t\tif (xdl_append_merge(&c, 2,\n+\t\t\t\t     i0, chg0, i1, chg1, i2, chg2)) {\n \t\t\txdl_cleanup_merge(changes);\n \t\t\treturn -1;\n \t\t}\n@@ -449,7 +514,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t/* output */\n \tif (result) {\n \t\tint size = xdl_fill_merge_buffer(xe1, name1, xe2, name2,\n-\t\t\tchanges, NULL);\n+\t\t\tchanges, NULL, style);\n \t\tresult->ptr = xdl_malloc(size);\n \t\tif (!result->ptr) {\n \t\t\txdl_cleanup_merge(changes);\n@@ -457,14 +522,14 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, const char *name1,\n \t\t}\n \t\tresult->size = size;\n \t\txdl_fill_merge_buffer(xe1, name1, xe2, name2, changes,\n-\t\t\t\tresult->ptr);\n+\t\t\t\t      result->ptr, style);\n \t}\n \treturn xdl_cleanup_merge(changes);\n }\n \n int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,\n \t\tmmfile_t *mf2, const char *name2,\n-\t\txpparam_t const *xpp, int level, mmbuffer_t *result) {\n+\t\txpparam_t const *xpp, int flags, mmbuffer_t *result) {\n \txdchange_t *xscr1, *xscr2;\n \txdfenv_t xe1, xe2;\n \tint status;\n@@ -501,7 +566,7 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, const char *name1,\n \t\t} else {\n \t\t\tstatus = xdl_do_merge(&xe1, xscr1, name1,\n \t\t\t\t\t      &xe2, xscr2, name2,\n-\t\t\t\t\t      level, xpp, result);\n+\t\t\t\t\t      flags, xpp, result);\n \t\t}\n \t\txdl_free_script(xscr1);\n \t\txdl_free_script(xscr2);\n\n\ndiff --git c/t/t6023-merge-file.sh w/t/t6023-merge-file.sh\nindex 42620e0..f3484e3 100755\n--- c/t/t6023-merge-file.sh\n+++ w/t/t6023-merge-file.sh\n@@ -161,4 +161,41 @@ test_expect_success 'ZEALOUS_ALNUM' '\n \n '\n \n+cat >expect <<\\EOF\n+Dominus regit me,\n+<<<<<<< new8.txt\n+et nihil mihi deerit;\n+\n+\n+\n+\n+In loco pascuae ibi me collocavit;\n+super aquam refectionis educavit me.\n+|||||||\n+et nihil mihi deerit.\n+In loco pascuae ibi me collocavit,\n+super aquam refectionis educavit me;\n+=======\n+et nihil mihi deerit,\n+\n+\n+\n+\n+In loco pascuae ibi me collocavit --\n+super aquam refectionis educavit me,\n+>>>>>>> new9.txt\n+animam meam convertit,\n+deduxit me super semitas jusitiae,\n+propter nomen suum.\n+Nam et si ambulavero in medio umbrae mortis,\n+non timebo mala, quoniam TU mecum es:\n+virga tua et baculus tuus ipsa me consolata sunt.\n+EOF\n+\n+test_expect_success '\"diff3 -m\" style output' '\n+\ttest_must_fail git merge-file -p --diff3 \\\n+\t\tnew8.txt new5.txt new9.txt >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n"},{"id":"89104","messageId":"7vej485h5f.fsf@gitster.siamese.dyndns.org","threadId":"15268","inReplyTo":"alpine.LFD.1.10.0808281727490.3300@nehalem.linux-foundation.org","subject":"Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-29T09:01:32Z","receivedAt":"2008-08-29T09:01:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Thu, 28 Aug 2008, Junio C Hamano wrote:\n>> \n>> Some poeple find it easier to be able to understand what is going on when\n>> they can view the common ancestor's version, which is used by \"diff3 -m\",\n>> which shows:\n>> \n>>  <<<<<<<\n>>  postimage from one side;\n>>  |||||||\n>>  shared preimage;\n>>  =======\n>>  postimage of the other side; and\n>>  >>>>>>>\n> ...\n> Wouldn't it be much nicer to make this a bitflag, than an enumeration of \n> different models?\n>\n> In particular, why would it be wrong to want to be ZEALOUS (possibly \n> _ALNUM) and still want DIFF3 output?\n>\n> IOW, I don't think these are at all disjoint sets. In fact, I think DIFF3 \n> sounds not at all like a \"level\" to me, but like an output format thing, \n> so it's in a totally different \"address space\".\n>\n> Hmm?\n\nI've thought about this a bit more, and I think \"diff3 -m\" style output\nwould not make sense for anything more aggressive than XDL_MERGE_EAGER,\nbecause of the way how the merge reduction works.\n\nSuppose a common ancestor (shared preimage) is modified to postimage #1\nand #2 (each letter represents one line):\n\n                     #####\n    postimage#1: 1234ABCDE789\n                    |    /\n                    |   /\n    preimage:    123456789\n                    |   \\\n    postimage#2: 1234AXYE789\n                     ####\n\nXDL_MERGE_MINIMAL and XDL_MERGE_EAGER would:\n\n (1) find the s/56/ABCDE/ done on one side and s/56/AXYE/ done on the\n     other side,\n\n (2) notice that they touch an overlapping area, and\n\n (3) mark it as a conflict, \"ABCDE vs AXYE\".\n\nThe difference between the two algorithm is that EAGER drops the hunk if\nthe postimages match (i.e. both sides modified the same way), while\nMINIMAL keeps it.  There is no other operation performed to the hunk.  As\nthe result, lines marked with \"#\" in the above picure will be in the RCS\nmerge style output.  We can sanely say that the part from the preimage\nthat corresponds to these conflicting changes is \"567\", which is what\n\"diff3 -m\" style output adds to it.\n\nNow, XDL_MERGE_ZEALOUS postprocesses output XDL_MERGE_EAGER would have\nproduced, by looking at the differences between the changes two postimages\nmade.  It notices that both sides start their new contents with \"A\", and\nexcludes it from the output (it also excludes \"E\" for the same reason).\nThe conflict that used to be \"ABCDE vs AXYE\" is now \"BCD vs XY\".\n\nThere could even be matching part between two postimages in the middle.\nInstead of one side rewriting the shared \"56\" to \"ABCDE\" and the other\nside to \"AXYE\", imagine the case where the postimages are \"ABCDE\" and\n\"AXCYE\", in which case instead of having one conflicted hunk \"BCD vs XY\",\nyou would have two conflicting hunks \"B vs X\" and \"D vs Y\".\n\nIn either case, once you reduce \"ABCDE vs AXYE\" to \"BCD vs XY\" (or \"ABCDE\nvs AXCYE\" to \"B vs X\" and \"D vs Y\"), there is no part from the preimage\nthat corresponds to the conflicting change made in both postimages\nanymore.  In other words, conflict reduced by ZEALOUS algorithm cannot be\nexpressed in \"diff3 -m\" style.\n\nYou can still have \"diff3 -m\" style with MINIMAL or EAGER, but I think MINIMAL\nis pointless and using \"diff3 -m\" style with it is even more so.\n"},{"id":"89291","messageId":"7v1w05d5hm.fsf@gitster.siamese.dyndns.org","threadId":"15268","inReplyTo":"7vmyiwbpe2.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-31T07:10:45Z","receivedAt":"2008-08-31T07:10:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>\n>> On Thu, 28 Aug 2008, Junio C Hamano wrote:\n>>> \n>>> Some poeple find it easier to be able to understand what is going on when\n>>> they can view the common ancestor's version, which is used by \"diff3 -m\",\n>>> which shows:\n>>> \n>>>  <<<<<<<\n>>>  postimage from one side;\n>>>  |||||||\n>>>  shared preimage;\n>>>  =======\n>>>  postimage of the other side; and\n>>>  >>>>>>>\n>>> \n>>> This is an initial step to bring that as an optional feature to git.\n>>> Only \"git merge-file\" has been converted, with \"--diff3\" option.\n>>\n>> If you have the common ancestor, why would you ever want this format, and \n>> not a nice conflict entry in the index?\n>\n> We already have that, don't we?  You can think of it as how to present\n> that information without resorting to \"diff :1:path :2:path\".\n\nI've been using this for my own work, and here is a typical example.  This\nhappened while rebuilding 'pu', merging jc/merge-whitespace topic on top\nof what contains jc/better-conflict-resolution topic.\n\n    In case if anybody is interested in reproducing this to follow the\n    discussion of improving the support of conflicted merge resolution,\n    here is how to reproduce this:\n\n        $ git checkout 7ead6a7^\n        $ git merge ^2\n\nIn builtin-merge-file.c, there appears:\n\n\tret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],\n<<<<<<< HEAD:builtin-merge-file.c\n\t\t\t&xpp, merge_level | merge_style, &result);\n|||||||\n\t\t\t&xpp, XDL_MERGE_ZEALOUS_ALNUM, &result);\n=======\n\t\t\t&xpp, XDL_MERGE_ZEALOUS_ALNUM, ws_rule, &result);\n>>>>>>> jc/merge-whitespace:builtin-merge-file.c\n\n\tfor (i = 0; i < 3; i++)\n\nIn this case, both branches are my code, so I do not need the additional\nhint to see that one branch added ws_rule parameter while the other one\ngeneralized the \"merge simplify level\" parameter and is now passing a\nbitwise OR of two variables.  But if somebody else is doing the merge, it\nmay not be so obvious how the resolution should be.\n\nOn the other hand, this appears in ll-merge.c:\n\n\txpparam_t xpp;\n<<<<<<< HEAD:ll-merge.c\n\tint style = 0;\n|||||||\n=======\n\tunsigned ws_rule = 0;\n>>>>>>> jc/merge-whitespace:ll-merge.c\n\nThis is obviously useless; perhaps we can drop the \"original\" hunk when it\nis empty to save space?\n\nAnother thing that helped me figure out what was going on was when merging\nmv/merge-recursive topic on top of next.\n\n    In case if anybody is interested in reproducing this to follow the\n    discussion of improving the support of conflicted merge resolution,\n    here is how to reproduce this:\n\n        $ git checkout ebe0e3b^\n        $ git merge ebe0e3b^2\n\nThis merge involves removal of quite a lot of lines to a new file\nmerge-recursive.c from existing builtin-merge-recursive.c; the default\nconflict simplification (XDL_MERGE_ZEALOUS) works _extremely_ well for\nthis merge, in that it shows this (I'll refer to this result as *1* in a\nlater paragraph):\n\n    <<<<<<<\n    static struct commit *get_ref(const char *ref)\n    {\n    ...\n    }\n\n    static int merge_config(const char *var, const char *value, void *cb)\n    {\n    ...\n    }\n\n    =======\n    >>>>>>>\n\nIt just shows we have _some_ changes to these two functions since the\nother side removed them from this file, but it does not say what the\nchange is.  However, with the \"common ancestor\" version, here is what I\nget (you need to use 'next' and \"merge.conflictstyle\" configuration set to\n\"diff3\" to reproduce this):\n\n    <<<<<<<\n    static struct commit *get_ref(const char *ref)\n    {\n    ...\n    }\n\n    static int merge_config(const char *var, const char *value, void *cb)\n    {\n    ...\n            return git_xmerge_config(var, value, cb);\n    }\n\n    |||||||\n    static struct commit *get_ref(const char *ref)\n    {\n    ...\n    }\n\n    static int merge_config(const char *var, const char *value, void *cb)\n    {\n    ...\n            return git_default_config(var, value, cb);\n    }\n\n    =======\n    >>>>>>> theirs\n\nBy comparing the first and second part, we can tell that the call to\ngit_default_config() was replaced to a call to git_xmerge_config().  And\nthis is a crucial clue to me to make an corresponding update to the\nfunction that now lives in merge-file.c.\n\nOn average, I am finding that \"diff3 -m\" format more irritating than\nuseful.  However, on occasions like this, I am finding it quite useful.\n\nMy observation so far suggests that it would be best for me to leave the\nconfiguration \"merge.conflictstyle\" to the default \"merge\", and instead\ngive an option to allow me to tell \"git checkout -m -- $path\" (which is\nalso a new feature; it overwrites the $path by the result of a fresh merge\nto reproduce the conflicted state in the working tree, using the three\nstages recorded in the index) to use \"diff3 -m\" style, when I want to.\n\nThat means the command sequence would look like:\n\n    $ git merge mv/merge-recursive ;# git merge ebe0e3b^2\n    .. oops, heavily conflicted merge and I see *1*, which is not\n    .. very useful.  Let's see what is going on.\n    .. Note that this --merge=diff3 option does not exist yet.\n    $ git checkout --merge=diff3 -- builtin-merge-recursive.c\n    $ emacs builtin-merge-recursive.c\n    .. split the buffer into two, move the cursor to the line after <<<\n    .. in one window while moving the cursor to the line after ||| line\n    .. in the other one, and say \"M-x compare-windows\".  Aha, there is\n    .. a change there.\n\nHaving said all that, I now realize that the way I described in my\nresponse to you is just as easy to view this particular case:\n\n    $ git merge mv/merge-recursive ;# git merge ebe0e3b^2\n    .. oops, heavily conflicted merge and I see *1*, which is not\n    .. very useful.  Let's see what is going on.\n    .. Note that this --merge=diff3 option does not exist yet.\n    $ git diff :{1,2}:builtin-merge-recursive.c ;# shell expands {1,2} ;-)\n\nwhich exactly shows what one side changed -- instead of calling\ngit_default_config(), the code calls git_xmerge_config().\n\n$ git diff :{1,2}:builtin-merge-recursive.c\ndiff --git a/:1:builtin-merge-recursive.c b/:2:builtin-merge-recursive.c\nindex dfb363e..f3b6ede 100644\n--- a/:1:builtin-merge-recursive.c\n+++ b/:2:builtin-merge-recursive.c\n@@ -1348,7 +1348,7 @@ static int merge_config(const char *var, const char *value, void *cb)\n                merge_rename_limit = git_config_int(var, value);\n                return 0;\n        }\n-       return git_default_config(var, value, cb);\n+       return git_xmerge_config(var, value, cb);\n }\n\n int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n"},{"id":"89329","messageId":"alpine.LFD.1.10.0808311021120.12958@nehalem.linux-foundation.org","threadId":"15268","inReplyTo":"7v1w05d5hm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-08-31T17:38:18Z","receivedAt":"2008-08-31T17:38:18Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 31 Aug 2008, Junio C Hamano wrote:\n> \n> On average, I am finding that \"diff3 -m\" format more irritating than\n> useful.  However, on occasions like this, I am finding it quite useful.\n\nI have to say that the best feature of the standard merge is the zealous \nversion, which apparently doesn't work well with the \"diff3 -m\" format.\n\nI do agree that what is often really nasty with the normal merge conflict \ninfo is when you actually need the original thing to understand why some \nhunk was left, and yes, a common case of that is that one side simply \nremoved a function, and the other side had modified it. At that point the \nresulting conflict is often hard to understand without seeing the \noriginal, exactly because you don't see any actual \"conflict\", you only \nsee one side (the other side being empty).\n\nBut what really saves that situation is the combination of\n\n - 'gitk --merge <file>'\n - 'git diff' showing the multi-way merge\n\nand I find myself really _hating_ doing rebases because the merge helpers \nare so totally useless (ie \"gitk --merge\" at least didn't use to work \nacross a rebase conflict because MERGE_HEAD isn't set)\n\nBut the biggest problem, and the reason I _really_ detest the diff3 \nformat, is that small merges are fairly often pretty easy to see anyway. \nIf the conflict markers all fit in one screenful, it's generally fairly \neasy to see why something conflicted, because you can visually compare \nthings.\n\nBut the complicated cases are when there are bigger changes, and the \nconflict is over many many lines of code, and it's really hard to visually \nsee what changed. And the diff3 format makes this worse - it not only \nmakes the conflict 50% bigger to begin with, it moves the two conflicting \nversions away from each other, making that visual comparison much harder.\n\nNow, there are tools to help with that. I think various of the graphical \nmerge tools understand the diff3 format, and then ti can really help. But \nI think it hurts for a lot of the _common_ cases.\n\n> My observation so far suggests that it would be best for me to leave the\n> configuration \"merge.conflictstyle\" to the default \"merge\", and instead\n> give an option to allow me to tell \"git checkout -m -- $path\" (which is\n> also a new feature; it overwrites the $path by the result of a fresh merge\n> to reproduce the conflicted state in the working tree, using the three\n> stages recorded in the index) to use \"diff3 -m\" style, when I want to.\n\nNow *this* I think is a great idea! \n\nThe reason I think it's a great idea is that it solves so many _different_ \nissues (which is the mark of a really good solution):\n\n - it fixes my problem with diff3 output: the fact that it's more annoying \n   by _default_ than it is occasionally useful.\n\n   If the default isn't to do it - since by default it often hurts - but \n   you have the option to do it when there is something confusing going on \n   (like the \"one side disappeared, why did it conflict?\" case), then you \n   have the best of both worlds - a good default with a way to dig deeper \n   when you need to.\n\n - it fixes another totally unrelated problem: incorrect merge \n   resolutions.\n\n   Again, I find this to be fairly rare, but what git is good at is to \n   make incremental resolutions for merge problems - you can resolve the \n   merge in the work tree, then compile and test the result before \n   actually committing it, and \"git diff\" always gives relevant and \n   interesting output for the merge.\n\n   And _occasionally_ the resolve looks obvious, but then when you compile \n   things you notice that it doesn't work because (for example) you \n   resolved it by removing one side (exactly because the other side was a \n   removal), and it turned out that the conflict was adding a function \n   that you hadn't realized was new, and was needed.\n\n   And while \"git diff\" is fine, and you can cut-and-paste things and try \n   to re-resolve it that way, I have occasionally decided to just do a \n   \"git reset\" and re-do the whole merge.\n\n   But your idea allows us to just re-do the merge for a single file.\n\nSo I think we do quite well already, but your solution really does sound \nlike a good and useful addition to the toolbox.\n\n\t\tLinus\n"},{"id":"89333","messageId":"7vd4jp2fi5.fsf@gitster.siamese.dyndns.org","threadId":"15268","inReplyTo":"alpine.LFD.1.10.0808311021120.12958@nehalem.linux-foundation.org","subject":"Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in \"diff3 -m\" style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-31T18:42:10Z","receivedAt":"2008-08-31T18:42:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sun, 31 Aug 2008, Junio C Hamano wrote:\n>> ...\n>> My observation so far suggests that it would be best for me to leave the\n>> configuration \"merge.conflictstyle\" to the default \"merge\", and instead\n>> give an option to allow me to tell \"git checkout -m -- $path\" (which is\n>> also a new feature; it overwrites the $path by the result of a fresh merge\n>> to reproduce the conflicted state in the working tree, using the three\n>> stages recorded in the index) to use \"diff3 -m\" style, when I want to.\n>\n> Now *this* I think is a great idea! \n>\n> The reason I think it's a great idea is that it solves so many _different_ \n> issues (which is the mark of a really good solution):\n> ...\n>  - it fixes another totally unrelated problem: incorrect merge \n>    resolutions.\n\nI do not know if rerere often kicks in in your workflow, but occasionally\nI notice that I have a faulty merge recorded by it, which automatically is\napplied again when reproducing a merge.  And it makes me deeply regret\nthat I invented the rerere mechanism.\n\nThe best solution for this issue I found so far is embarrasingly clumsy.\nRun \"ls -tl .git/rr-cache\" to find the newest one with \"thisimage\", make\nsure it is the right one, remove the directory and redo the merge.\n\nActually, the feature of \"git checkout -m -- $path\" to reproduce the merge\nfor the named path was conceived to address this \"I cannot easily correct\nthe faulty resolution that was re-applied\" issue.\n"}]}