{"thread":{"id":"42032","subject":"[PATCH] ll-merge: fix typo in comment","startedAt":"2016-04-14T22:12:15Z","lastAt":"2016-04-14T23:02:30Z","messageCount":3,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"283493","messageId":"xmqqbn5bg7r4.fsf@gitster.mtv.corp.google.com","threadId":"42032","inReplyTo":null,"subject":"[PATCH] ll-merge: fix typo in comment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-14T22:12:15Z","receivedAt":"2016-04-14T22:12:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When a944af1d (merge: teach -Xours/-Xtheirs to binary ll-merge\ndriver, 2012-09-08) introduced FAVOR_OURS/FAVOR_THEIRS to the binary\nll-merge driver, it changed what happens to the merge result for the\nouter merge, and updated the comment from:\n\n    The tentative merge result is \"ours\" for the final round, or\n    common ancestor for an internal merge.  Still return \"conflicted\n    merge\" status.\n\nto\n\n    The tentative merge result is the or common ancestor for an\n    internal merge.\n\nWhat happened is obvious.  I noticed the lack of definitive article\nin front of \"common\" but failed to remove \"or\".  Also I forgot to\ndescribe what I did for the final merge, probably because I was\nsatisified by the description in the log message.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n ll-merge.c | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/ll-merge.c b/ll-merge.c\nindex ff4a43a..fe22412 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -47,7 +47,9 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,\n \tassert(opts);\n \n \t/*\n-\t * The tentative merge result is the or common ancestor for an internal merge.\n+\t * The tentative merge result is the common ancestor for an\n+\t * internal merge.  For the final merge, it is \"ours\" by\n+\t * default but -Xours/-Xtheirs can tweak the choice.\n \t */\n \tif (opts->virtual_ancestor) {\n \t\tstolen = orig;\n"},{"id":"283498","messageId":"xmqqy48fer2s.fsf@gitster.mtv.corp.google.com","threadId":"42032","inReplyTo":"xmqqbn5bg7r4.fsf@gitster.mtv.corp.google.com","subject":"[PATCH/RFC] ll-merge: use a longer conflict marker for internal merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-14T22:57:47Z","receivedAt":"2016-04-14T22:57:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The primary use of conflict markers is to help the user who resolves\nthe final (outer) merge by hand to show which part came from which\nbranch by separating the blocks of lines apart.  When the conflicted\nparts from a \"virtual ancestor\" merge created by merge-recursive\nremains in the common ancestor part in the final result, however,\nthe conflict markers that are the same size as the final merge\nbecome harder to see.\n\nIncrease the conflict marker size slightly for these inner merges so\nthat the markers from the final merge and cruft from internal merge\ncan be distinguished more easily.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * This would help reduce the common issue that prevents \"rerere\"\n   from being used on a really complex conflict.  I have another\n   (arguably riskier) patch that teaches rerere's parser to ignore\n   \"<<<\" and \">>>\" markers that says \"Temporary merge branch \" at\n   the end of the line that achives a similar effect, but I think\n   this may be a cleaner solution, partly because it also deals with\n   \"===\" and \"|||\" lines that do not have such a clue to help\n   rerere's parser.\n\n ll-merge.c                        | 8 ++++++--\n t/t6024-recursive-merge.sh        | 2 +-\n t/t6036-recursive-corner-cases.sh | 3 ++-\n 3 files changed, 9 insertions(+), 4 deletions(-)\n\ndiff --git a/ll-merge.c b/ll-merge.c\nindex 5c73274..e5ff7f6 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -376,8 +376,12 @@ int ll_merge(mmbuffer_t *result_buf,\n \t\t}\n \t}\n \tdriver = find_ll_merge_driver(ll_driver_name);\n-\tif (opts->virtual_ancestor && driver->recursive)\n-\t\tdriver = find_ll_merge_driver(driver->recursive);\n+\n+\tif (opts->virtual_ancestor) {\n+\t\tif (driver->recursive)\n+\t\t\tdriver = find_ll_merge_driver(driver->recursive);\n+\t\tmarker_size += 2;\n+\t}\n \treturn driver->fn(driver, result_buf, path, ancestor, ancestor_label,\n \t\t\t  ours, our_label, theirs, their_label,\n \t\t\t  opts, marker_size);\ndiff --git a/t/t6024-recursive-merge.sh b/t/t6024-recursive-merge.sh\nindex 755d30c..3f59e58 100755\n--- a/t/t6024-recursive-merge.sh\n+++ b/t/t6024-recursive-merge.sh\n@@ -76,7 +76,7 @@ test_expect_success \"result contains a conflict\" \"test_cmp expect a1\"\n \n git ls-files --stage > out\n cat > expect << EOF\n-100644 439cc46de773d8a83c77799b7cc9191c128bfcff 1\ta1\n+100644 ec3fe2a791706733f2d8fa7ad45d9a9672031f5e 1\ta1\n 100644 cf84443e49e1b366fac938711ddf4be2d4d1d9e9 2\ta1\n 100644 fd7923529855d0b274795ae3349c5e0438333979 3\ta1\n EOF\ndiff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh\nindex a86087b..cc1ee6a 100755\n--- a/t/t6036-recursive-corner-cases.sh\n+++ b/t/t6036-recursive-corner-cases.sh\n@@ -217,7 +217,8 @@ test_expect_success 'git detects differently handled merges conflict' '\n \t\t-L \"\" \\\n \t\t-L \"Temporary merge branch 1\" \\\n \t\tmerged empty merge-me &&\n-\ttest $(git rev-parse :1:new_a) = $(git hash-object merged)\n+\tsed -e \"s/^\\([<=>]\\)/\\1\\1\\1/\" merged >merged-internal &&\n+\ttest $(git rev-parse :1:new_a) = $(git hash-object merged-internal)\n '\n \n #\n-- \n2.8.1-367-g5b624f2\n"},{"id":"283499","messageId":"xmqqtwj3equx.fsf@gitster.mtv.corp.google.com","threadId":"42032","inReplyTo":"xmqqy48fer2s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH/RFC] ll-merge: use a longer conflict marker for internal merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-14T23:02:30Z","receivedAt":"2016-04-14T23:02:30Z","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> The primary use of conflict markers is to help the user who resolves\n> the final (outer) merge by hand to show which part came from which\n> branch by separating the blocks of lines apart.  When the conflicted\n> parts from a \"virtual ancestor\" merge created by merge-recursive\n> remains in the common ancestor part in the final result, however,\n> the conflict markers that are the same size as the final merge\n> become harder to see.\n>\n> Increase the conflict marker size slightly for these inner merges so\n> that the markers from the final merge and cruft from internal merge\n> can be distinguished more easily.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  * This would help reduce the common issue that prevents \"rerere\"\n>    from being used on a really complex conflict.  I have another\n>    (arguably riskier) patch that teaches rerere's parser to ignore\n>    \"<<<\" and \">>>\" markers that says \"Temporary merge branch \" at\n>    the end of the line that achives a similar effect, but I think\n>    this may be a cleaner solution, partly because it also deals with\n>    \"===\" and \"|||\" lines that do not have such a clue to help\n>    rerere's parser.\n\nJust for comparison, this is the other \"riskier\" approach.\n\n-- >8 --\nSubject: [PATCH] rerere: try ignoring extra cruft left by merge-recursive\n\nSometimes the conflict markers from the virtual ancestor synthesis\ndone by merge-recursive can seep out to the conflicted area of the\nfinal (incomplete) merge result.  Detect and reject them by noticing\nthe \"Temporary merge branch 1\" (or \"2\") branch names.\n\nThis is an incomplete solution for \"rerere sometimes cannot parse\nthe conflicted parts\" problem, because there is no such helpful clue\non \"|||||\" (seen only when using \"diff3\" style) and \"=====\" lines.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n rerere.c | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/rerere.c b/rerere.c\nindex 1693866..02cbc24 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -359,6 +359,18 @@ static int rerere_file_getline(struct strbuf *sb, struct rerere_io *io_)\n \treturn strbuf_getwholeline(sb, io->input, '\\n');\n }\n \n+static int is_temporary_branch_mark(char *buf)\n+{\n+\tconst char *name;\n+\n+\tif (!skip_prefix(buf, \" Temporary merge branch \", &name))\n+\t\treturn 0;\n+\tif (*name != '1' && *name != '2')\n+\t\treturn 0;\n+\tname++;\n+\treturn (!*name || (*name == '\\n'));\n+}\n+\n /*\n  * Require the exact number of conflict marker letters, no more, no\n  * less, followed by SP or any whitespace\n@@ -381,7 +393,7 @@ static int is_cmarker(char *buf, int marker_char, int marker_size)\n \twhile (marker_size--)\n \t\tif (*buf++ != marker_char)\n \t\t\treturn 0;\n-\tif (want_sp && *buf != ' ')\n+\tif (want_sp && (*buf != ' ' || is_temporary_branch_mark(buf)))\n \t\treturn 0;\n \treturn isspace(*buf);\n }\n-- \n2.8.1-367-g5b624f2\n"}]}