{"thread":{"id":"58313","subject":"[PATCH 0/3] Fixups for cw/submodule-merge-messages","startedAt":"2022-08-17T00:28:07Z","lastAt":"2022-08-18T07:15:39Z","messageCount":16,"participants":["Elijah Newren via GitGitGadget","Junio C Hamano","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"461324","messageId":"pull.1325.git.1660696081.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":null,"subject":"[PATCH 0/3] Fixups for cw/submodule-merge-messages","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T00:27:58Z","receivedAt":"2022-08-17T00:28:07Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series fixes a few issues I noted in cw/submodule-merge-messages (which\nmerged to next a few days ago). Sorry for not responding before that topic\nmerged to next; I caught Covid near the end of my vacation and it took me\nout for a while. So here are a few patches on top instead.\n\n(Note that my first patch builds on Junio's patch-on-top, though it kind of\nobviates the need for his patch. Let me know if you want me to rebase\ndirectly on Calvin's patch and remove the need for your patch, Junio.)\n\nElijah Newren (3):\n merge-ort: remove translator lego in new \"submodule conflict\n suggestion\"\n merge-ort: add comment to avoid surprise with new sub_flag variable\n merge-ort: provide helpful submodule update message when possible\n\n merge-ort.c | 106 +++++++++++++++++-----------------------------------\n 1 file changed, 34 insertions(+), 72 deletions(-)\n\n\nbase-commit: 38e3c211f000ebde511099b62be2af104eb20c12\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1325%2Fnewren%2Fsubmodule-merge-messages-fixups-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1325/newren/submodule-merge-messages-fixups-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1325\n-- \ngitgitgadget\n"},{"id":"461325","messageId":"46c9b48871bd8112866fa16062a4bf0f4776f09e.1660696081.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.git.1660696081.gitgitgadget@gmail.com","subject":"[PATCH 1/3] merge-ort: remove translator lego in new \"submodule conflict suggestion\"","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T00:27:59Z","receivedAt":"2022-08-17T00:28:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn commit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04), the new \"submodule conflict suggestion\" code was\ntranslating 6 different pieces of the new message and then used\ncarefully crafted logic to allow stitching it back together with special\nformatting. Keep the components of the message together as much as\npossible, so that:\n * we reduce the number of things translators have to translate\n * translators have more control over the format of the output\n * the code is much easier for developers to understand too\n\nAlso, reformat some comments running beyond the 80th column while at it.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 90 +++++++++++++++++------------------------------------\n 1 file changed, 28 insertions(+), 62 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex f33df3ff65d..67159fc6ef9 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4481,36 +4481,6 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n \treturn errs;\n }\n \n-static void format_submodule_conflict_suggestion(struct strbuf *msg) {\n-\tstruct strbuf tmp = STRBUF_INIT;\n-\tstruct string_list msg_list = STRING_LIST_INIT_DUP;\n-\tint i;\n-\n-\tstring_list_split(&msg_list, msg->buf, '\\n', -1);\n-\tfor (i = 0; i < msg_list.nr; i++) {\n-\t\tif (!i)\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: This is line item of submodule conflict message\n-\t\t\t * from print_submodule_conflict_suggestion() below. For RTL\n-\t\t\t * languages, the following swap is suggested:\n-\t\t\t * \" - %s\\n\" -> \"%s - \\n\"\n-\t\t\t */\n-\t\t\tstrbuf_addf(&tmp, _(\" - %s\\n\"), msg_list.items[i].string);\n-\t\telse\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: This is line item of submodule conflict message\n-\t\t\t * from print_submodule_conflict_suggestion() below. For RTL\n-\t\t\t * languages, the following swap is suggested:\n-\t\t\t * \" %s\\n\" -> \"%s \\n\"\n-\t\t\t */\n-\t\t\tstrbuf_addf(&tmp, _(\" %s\\n\"), msg_list.items[i].string);\n-\t}\n-\tstrbuf_reset(msg);\n-\tstrbuf_add(msg, tmp.buf, tmp.len);\n-\tstring_list_clear(&msg_list, 0);\n-\tstrbuf_release(&tmp);\n-}\n-\n static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \tstruct string_list_item *item;\n \tstruct strbuf msg = STRBUF_INIT;\n@@ -4532,45 +4502,41 @@ static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \t\t\treturn;\n \t}\n \n-\tprintf(_(\"Recursive merging with submodules currently only supports \"\n-\t\t\"trivial cases.\\nPlease manually handle the merging of each \"\n-\t\t\"conflicted submodule.\\nThis can be accomplished with the following \"\n-\t\t\"steps:\"));\n-\tputchar('\\n');\n-\n+\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n \tfor_each_string_list_item(item, csub) {\n \t\tstruct conflicted_submodule_item *util = item->util;\n+\n \t\t/*\n-\t\t * TRANSLATORS: This is a line of advice to resolve a merge conflict\n-\t\t * in a submodule. The second argument is the abbreviated id of the\n-\t\t * commit that needs to be merged.\n-\t\t * E.g. - go to submodule (sub), and either merge commit abc1234\"\n+\t\t * TRANSLATORS: This is a line of advice to resolve a merge\n+\t\t * conflict in a submodule. The first argument is the submodule\n+\t\t * name, and the second argument is the abbreviated id of the\n+\t\t * commit that needs to be merged. For example:\n+\t\t * - go to submodule (mysubmodule), and either merge commit abc1234\"\n \t\t */\n-\t\tstrbuf_addf(&tmp, _(\"go to submodule (%s), and either merge commit %s\\n\"\n-\t\t\t\"or update to an existing commit which has merged those changes\"),\n-\t\t\titem->string, util->abbrev);\n-\t\tformat_submodule_conflict_suggestion(&tmp);\n-\t\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\t\tstrbuf_reset(&tmp);\n+\t\tstrbuf_addf(&tmp, _(\" - go to submodule (%s), and either merge commit %s\\n\"\n+\t\t\t\t \" or update to an existing commit which has merged those changes\\n\"),\n+\t\t\t item->string, util->abbrev);\n \t}\n-\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n-\tstrbuf_addstr(&tmp, _(\"come back to superproject and run:\"));\n-\tstrbuf_addf(&tmp, \"\\n\\ngit add %s\\n\\n\", subs.buf);\n-\tstrbuf_addstr(&tmp, _(\"to record the above merge or update\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\tstrbuf_reset(&tmp);\n-\n-\tstrbuf_addstr(&tmp, _(\"resolve any other conflicts in the superproject\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\tstrbuf_reset(&tmp);\n-\n-\tstrbuf_addstr(&tmp, _(\"commit the resulting index in the superproject\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n+\n+\t/*\n+\t * TRANSLATORS: This is a detailed message for resolving submodule\n+\t * conflicts. The first argument is string containing one step per\n+\t * submodule. The second is a space-separated list of submodule names.\n+\t */\n+\tstrbuf_addf(&msg,\n+\t\t _(\"Recursive merging with submodules currently only supports trivial cases.\\n\"\n+\t\t \"Please manually handle the merging of each conflicted submodule.\\n\"\n+\t\t \"This can be accomplished with the following steps:\\n\"\n+\t\t \"%s\"\n+\t\t \" - come back to superproject and run:\\n\\n\"\n+\t\t \" git add %s\\n\\n\"\n+\t\t \" to record the above merge or update\\n\"\n+\t\t \" - resolve any other conflicts in the superproject\\n\"\n+\t\t \" - commit the resulting index in the superproject\\n\"),\n+\t\t tmp.buf, subs.buf);\n \n \tprintf(\"%s\", msg.buf);\n+\n \tstrbuf_release(&subs);\n \tstrbuf_release(&tmp);\n \tstrbuf_release(&msg);\n-- \ngitgitgadget\n\n"},{"id":"461326","messageId":"269714da7062cf9d9de5200ab564bd727e921359.1660696081.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.git.1660696081.gitgitgadget@gmail.com","subject":"[PATCH 2/3] merge-ort: add comment to avoid surprise with new sub_flag variable","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T00:28:00Z","receivedAt":"2022-08-17T00:28:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04) added a sub_flag variable that is used to store a value from\nenum conflict_and_info_types, but initializes it with an invalid value\nof -1. The code may never set it to a valid value, and use the invalid\none. This can be surprising when reading over the code at first, but it\nwas intentional. Add a comment making it clear that it is okay to be\nusing an invalid value, due to how it is used later.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 67159fc6ef9..0a935a8135f 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -1886,7 +1886,7 @@ cleanup:\n \t\tconst char *abbrev;\n \n \t\tutil = xmalloc(sizeof(*util));\n-\t\tutil->flag = sub_flag;\n+\t\tutil->flag = sub_flag; /* May still be -1 */\n \t\tutil->abbrev = NULL;\n \t\tif (!sub_not_initialized) {\n \t\t\tabbrev = repo_find_unique_abbrev(&subrepo, b, DEFAULT_ABBREV);\n-- \ngitgitgadget\n\n"},{"id":"461327","messageId":"010cf4ff3442902031360ed87a6f86cc9486581a.1660696081.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.git.1660696081.gitgitgadget@gmail.com","subject":"[PATCH 3/3] merge-ort: provide helpful submodule update message when possible","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T00:28:01Z","receivedAt":"2022-08-17T00:28:19Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn commit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04), a more detailed message was provided when submodules\nconflict, in order to help users know how to resolve those conflicts.\nThere were a couple situations for which a different message would be\nmore appropriate, but that commit left handling those for future work.\nUnfortunately, that commit would check if any submodules were of the\ntype that it didn't know how to explain, and, if so, would avoid\nproviding the more detailed explanation even for the submodules it did\nknow how to explain.\n\nChange this to have the code print the helpful messages for the subset\nof submodules it knows how to explain.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 18 +++++++-----------\n 1 file changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 0a935a8135f..404f05e32d6 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4490,21 +4490,17 @@ static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \tif (!csub->nr)\n \t\treturn;\n \n-\t/*\n-\t * NEEDSWORK: The steps to resolve these errors deserve a more\n-\t * detailed explanation than what is currently printed below.\n-\t */\n+\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n \tfor_each_string_list_item(item, csub) {\n \t\tstruct conflicted_submodule_item *util = item->util;\n \n+\t\t/*\n+\t\t * NEEDSWORK: The steps to resolve these errors deserve a more\n+\t\t * detailed explanation than what is currently printed below.\n+\t\t */\n \t\tif (util->flag == CONFLICT_SUBMODULE_NOT_INITIALIZED ||\n-\t\t\tutil->flag == CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE)\n-\t\t\treturn;\n-\t}\n-\n-\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n-\tfor_each_string_list_item(item, csub) {\n-\t\tstruct conflicted_submodule_item *util = item->util;\n+\t\t util->flag == CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE)\n+\t\t\tcontinue;\n \n \t\t/*\n \t\t * TRANSLATORS: This is a line of advice to resolve a merge\n-- \ngitgitgadget\n"},{"id":"461338","messageId":"xmqqk0771lcb.fsf@gitster.g","threadId":"58313","inReplyTo":"pull.1325.git.1660696081.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] Fixups for cw/submodule-merge-messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-17T05:37:56Z","receivedAt":"2022-08-17T05:38:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> (Note that my first patch builds on Junio's patch-on-top, though it kind of\n> obviates the need for his patch. Let me know if you want me to rebase\n> directly on Calvin's patch and remove the need for your patch, Junio.)\n\nIt paid off that I didn't merge the leakfix to 'next' ;-) Please\njust tell me to drop the patch and build on top of what Calvin has\nin 'next' directly.\n\nThanks.\n"},{"id":"461349","messageId":"pull.1325.v2.git.1660718028.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.git.1660696081.gitgitgadget@gmail.com","subject":"[PATCH v2 0/3] Fixups for cw/submodule-merge-messages","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T06:33:45Z","receivedAt":"2022-08-17T06:33:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series fixes a few issues I noted in cw/submodule-merge-messages (which\nmerged to next a few days ago). Sorry for not responding before that topic\nmerged to next; I caught Covid near the end of my vacation and it took me\nout for a while. So here are a few patches on top instead.\n\nChanges since v1:\n\n * rebased directly on top of Calvin's patch, allowing Junio's patch in\n cw/submodule-merge-messages to be dropped.\n\nElijah Newren (3):\n merge-ort: remove translator lego in new \"submodule conflict\n suggestion\"\n merge-ort: add comment to avoid surprise with new sub_flag variable\n merge-ort: provide helpful submodule update message when possible\n\n merge-ort.c | 104 +++++++++++++++++-----------------------------------\n 1 file changed, 34 insertions(+), 70 deletions(-)\n\n\nbase-commit: 4057523a4061092e9181220d54dca9eadcb75bdc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1325%2Fnewren%2Fsubmodule-merge-messages-fixups-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1325/newren/submodule-merge-messages-fixups-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1325\n\nRange-diff vs v1:\n\n 1: 46c9b48871b ! 1: 374278c6a1d merge-ort: remove translator lego in new \"submodule conflict suggestion\"\n @@ merge-ort.c: static int record_conflicted_index_entries(struct merge_options *op\n -\t}\n -\tstrbuf_reset(msg);\n -\tstrbuf_add(msg, tmp.buf, tmp.len);\n --\tstring_list_clear(&msg_list, 0);\n --\tstrbuf_release(&tmp);\n -}\n -\n static void print_submodule_conflict_suggestion(struct string_list *csub) {\n 2: 269714da706 = 2: 340c0f46f74 merge-ort: add comment to avoid surprise with new sub_flag variable\n 3: 010cf4ff344 = 3: 80207d18334 merge-ort: provide helpful submodule update message when possible\n\n-- \ngitgitgadget\n"},{"id":"461350","messageId":"374278c6a1d64e4f9ae43c9c49801997dd338850.1660718028.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.v2.git.1660718028.gitgitgadget@gmail.com","subject":"[PATCH v2 1/3] merge-ort: remove translator lego in new \"submodule conflict suggestion\"","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T06:33:46Z","receivedAt":"2022-08-17T06:33:58Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn commit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04), the new \"submodule conflict suggestion\" code was\ntranslating 6 different pieces of the new message and then used\ncarefully crafted logic to allow stitching it back together with special\nformatting. Keep the components of the message together as much as\npossible, so that:\n * we reduce the number of things translators have to translate\n * translators have more control over the format of the output\n * the code is much easier for developers to understand too\n\nAlso, reformat some comments running beyond the 80th column while at it.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 88 +++++++++++++++++------------------------------------\n 1 file changed, 28 insertions(+), 60 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex a52faf6e218..67159fc6ef9 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4481,34 +4481,6 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n \treturn errs;\n }\n \n-static void format_submodule_conflict_suggestion(struct strbuf *msg) {\n-\tstruct strbuf tmp = STRBUF_INIT;\n-\tstruct string_list msg_list = STRING_LIST_INIT_DUP;\n-\tint i;\n-\n-\tstring_list_split(&msg_list, msg->buf, '\\n', -1);\n-\tfor (i = 0; i < msg_list.nr; i++) {\n-\t\tif (!i)\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: This is line item of submodule conflict message\n-\t\t\t * from print_submodule_conflict_suggestion() below. For RTL\n-\t\t\t * languages, the following swap is suggested:\n-\t\t\t * \" - %s\\n\" -> \"%s - \\n\"\n-\t\t\t */\n-\t\t\tstrbuf_addf(&tmp, _(\" - %s\\n\"), msg_list.items[i].string);\n-\t\telse\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: This is line item of submodule conflict message\n-\t\t\t * from print_submodule_conflict_suggestion() below. For RTL\n-\t\t\t * languages, the following swap is suggested:\n-\t\t\t * \" %s\\n\" -> \"%s \\n\"\n-\t\t\t */\n-\t\t\tstrbuf_addf(&tmp, _(\" %s\\n\"), msg_list.items[i].string);\n-\t}\n-\tstrbuf_reset(msg);\n-\tstrbuf_add(msg, tmp.buf, tmp.len);\n-}\n-\n static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \tstruct string_list_item *item;\n \tstruct strbuf msg = STRBUF_INIT;\n@@ -4530,45 +4502,41 @@ static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \t\t\treturn;\n \t}\n \n-\tprintf(_(\"Recursive merging with submodules currently only supports \"\n-\t\t\"trivial cases.\\nPlease manually handle the merging of each \"\n-\t\t\"conflicted submodule.\\nThis can be accomplished with the following \"\n-\t\t\"steps:\"));\n-\tputchar('\\n');\n-\n+\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n \tfor_each_string_list_item(item, csub) {\n \t\tstruct conflicted_submodule_item *util = item->util;\n+\n \t\t/*\n-\t\t * TRANSLATORS: This is a line of advice to resolve a merge conflict\n-\t\t * in a submodule. The second argument is the abbreviated id of the\n-\t\t * commit that needs to be merged.\n-\t\t * E.g. - go to submodule (sub), and either merge commit abc1234\"\n+\t\t * TRANSLATORS: This is a line of advice to resolve a merge\n+\t\t * conflict in a submodule. The first argument is the submodule\n+\t\t * name, and the second argument is the abbreviated id of the\n+\t\t * commit that needs to be merged. For example:\n+\t\t * - go to submodule (mysubmodule), and either merge commit abc1234\"\n \t\t */\n-\t\tstrbuf_addf(&tmp, _(\"go to submodule (%s), and either merge commit %s\\n\"\n-\t\t\t\"or update to an existing commit which has merged those changes\"),\n-\t\t\titem->string, util->abbrev);\n-\t\tformat_submodule_conflict_suggestion(&tmp);\n-\t\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\t\tstrbuf_reset(&tmp);\n+\t\tstrbuf_addf(&tmp, _(\" - go to submodule (%s), and either merge commit %s\\n\"\n+\t\t\t\t \" or update to an existing commit which has merged those changes\\n\"),\n+\t\t\t item->string, util->abbrev);\n \t}\n-\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n-\tstrbuf_addstr(&tmp, _(\"come back to superproject and run:\"));\n-\tstrbuf_addf(&tmp, \"\\n\\ngit add %s\\n\\n\", subs.buf);\n-\tstrbuf_addstr(&tmp, _(\"to record the above merge or update\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\tstrbuf_reset(&tmp);\n-\n-\tstrbuf_addstr(&tmp, _(\"resolve any other conflicts in the superproject\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\tstrbuf_reset(&tmp);\n-\n-\tstrbuf_addstr(&tmp, _(\"commit the resulting index in the superproject\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n+\n+\t/*\n+\t * TRANSLATORS: This is a detailed message for resolving submodule\n+\t * conflicts. The first argument is string containing one step per\n+\t * submodule. The second is a space-separated list of submodule names.\n+\t */\n+\tstrbuf_addf(&msg,\n+\t\t _(\"Recursive merging with submodules currently only supports trivial cases.\\n\"\n+\t\t \"Please manually handle the merging of each conflicted submodule.\\n\"\n+\t\t \"This can be accomplished with the following steps:\\n\"\n+\t\t \"%s\"\n+\t\t \" - come back to superproject and run:\\n\\n\"\n+\t\t \" git add %s\\n\\n\"\n+\t\t \" to record the above merge or update\\n\"\n+\t\t \" - resolve any other conflicts in the superproject\\n\"\n+\t\t \" - commit the resulting index in the superproject\\n\"),\n+\t\t tmp.buf, subs.buf);\n \n \tprintf(\"%s\", msg.buf);\n+\n \tstrbuf_release(&subs);\n \tstrbuf_release(&tmp);\n \tstrbuf_release(&msg);\n-- \ngitgitgadget\n\n"},{"id":"461351","messageId":"340c0f46f74acd641945fceba5ab5feac011ae60.1660718028.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.v2.git.1660718028.gitgitgadget@gmail.com","subject":"[PATCH v2 2/3] merge-ort: add comment to avoid surprise with new sub_flag variable","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T06:33:47Z","receivedAt":"2022-08-17T06:34:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04) added a sub_flag variable that is used to store a value from\nenum conflict_and_info_types, but initializes it with an invalid value\nof -1. The code may never set it to a valid value, and use the invalid\none. This can be surprising when reading over the code at first, but it\nwas intentional. Add a comment making it clear that it is okay to be\nusing an invalid value, due to how it is used later.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 67159fc6ef9..0a935a8135f 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -1886,7 +1886,7 @@ cleanup:\n \t\tconst char *abbrev;\n \n \t\tutil = xmalloc(sizeof(*util));\n-\t\tutil->flag = sub_flag;\n+\t\tutil->flag = sub_flag; /* May still be -1 */\n \t\tutil->abbrev = NULL;\n \t\tif (!sub_not_initialized) {\n \t\t\tabbrev = repo_find_unique_abbrev(&subrepo, b, DEFAULT_ABBREV);\n-- \ngitgitgadget\n\n"},{"id":"461352","messageId":"80207d183343e6f1397abdb70fee68bfb26f1756.1660718028.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.v2.git.1660718028.gitgitgadget@gmail.com","subject":"[PATCH v2 3/3] merge-ort: provide helpful submodule update message when possible","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-17T06:33:48Z","receivedAt":"2022-08-17T06:34:01Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn commit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04), a more detailed message was provided when submodules\nconflict, in order to help users know how to resolve those conflicts.\nThere were a couple situations for which a different message would be\nmore appropriate, but that commit left handling those for future work.\nUnfortunately, that commit would check if any submodules were of the\ntype that it didn't know how to explain, and, if so, would avoid\nproviding the more detailed explanation even for the submodules it did\nknow how to explain.\n\nChange this to have the code print the helpful messages for the subset\nof submodules it knows how to explain.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 18 +++++++-----------\n 1 file changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 0a935a8135f..404f05e32d6 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4490,21 +4490,17 @@ static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \tif (!csub->nr)\n \t\treturn;\n \n-\t/*\n-\t * NEEDSWORK: The steps to resolve these errors deserve a more\n-\t * detailed explanation than what is currently printed below.\n-\t */\n+\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n \tfor_each_string_list_item(item, csub) {\n \t\tstruct conflicted_submodule_item *util = item->util;\n \n+\t\t/*\n+\t\t * NEEDSWORK: The steps to resolve these errors deserve a more\n+\t\t * detailed explanation than what is currently printed below.\n+\t\t */\n \t\tif (util->flag == CONFLICT_SUBMODULE_NOT_INITIALIZED ||\n-\t\t\tutil->flag == CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE)\n-\t\t\treturn;\n-\t}\n-\n-\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n-\tfor_each_string_list_item(item, csub) {\n-\t\tstruct conflicted_submodule_item *util = item->util;\n+\t\t util->flag == CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE)\n+\t\t\tcontinue;\n \n \t\t/*\n \t\t * TRANSLATORS: This is a line of advice to resolve a merge\n-- \ngitgitgadget\n"},{"id":"461389","messageId":"xmqqsflutv4r.fsf@gitster.g","threadId":"58313","inReplyTo":"pull.1325.v2.git.1660718028.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/3] Fixups for cw/submodule-merge-messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-17T21:31:16Z","receivedAt":"2022-08-17T21:31:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> Elijah Newren (3):\n> merge-ort: remove translator lego in new \"submodule conflict\n> suggestion\"\n> merge-ort: add comment to avoid surprise with new sub_flag variable\n> merge-ort: provide helpful submodule update message when possible\n\nThanks. The last step was especially the pleasant one.\n"},{"id":"461390","messageId":"xmqqedxetuhg.fsf@gitster.g","threadId":"58313","inReplyTo":"340c0f46f74acd641945fceba5ab5feac011ae60.1660718028.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/3] merge-ort: add comment to avoid surprise with new sub_flag variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-17T21:45:15Z","receivedAt":"2022-08-17T21:45:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> From: Elijah Newren \n>\n> Commit 4057523a40 (\"submodule merge: update conflict error message\",\n> 2022-08-04) added a sub_flag variable that is used to store a value from\n> enum conflict_and_info_types, but initializes it with an invalid value\n> of -1. The code may never set it to a valid value, and use the invalid\n> one. This can be surprising when reading over the code at first, but it\n> was intentional. Add a comment making it clear that it is okay to be\n> using an invalid value, due to how it is used later.\n\nThe current code uses -1 as the \"suggest the default course of\naction\", so -1 is very much a \"valid value\" from the viewpoint of\nthe code that suggests how to resolve. It indeed is an invalid\nvalue from the viewpoint of those who maintain conflict_and_info_types\nenum.\n\n> Signed-off-by: Elijah Newren \n> ---\n> merge-ort.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/merge-ort.c b/merge-ort.c\n> index 67159fc6ef9..0a935a8135f 100644\n> --- a/merge-ort.c\n> +++ b/merge-ort.c\n> @@ -1886,7 +1886,7 @@ cleanup:\n> \t\tconst char *abbrev;\n> \n> \t\tutil = xmalloc(sizeof(*util));\n> -\t\tutil->flag = sub_flag;\n> +\t\tutil->flag = sub_flag; /* May still be -1 */\n> \t\tutil->abbrev = NULL;\n> \t\tif (!sub_not_initialized) {\n> \t\t\tabbrev = repo_find_unique_abbrev(&subrepo, b, DEFAULT_ABBREV);\n\nThis new comment may be a slight improvement, but a valid value of\nsub_flag is used only to signal the situation where the code does\nnot know what to suggest, which feels backwards for longer-term code\nevolution. Presumably, we would use the util->flag field to store\nwhich of the known cases we know what to suggest as we know better.\n\nI wonder if we should initialize the variable to the most generic\nCONFLICT_SUBMODULE_FAILED_TO_MERGE instead of -1. The value would\nmean \"use the default suggestion\", and the two known unworkable\nvalues (not-initialized and history-not-available) are currently\nhandled according to what these two values mean. We may later add\nmore specialization based on other CONFLICT_SUBMODULE_* values.\n\nThanks.\n\n"},{"id":"461402","messageId":"CABPp-BEwvnMxXze64mxMJjp32N=RYet_7nkG2is=qWqy_=zwig@mail.gmail.com","threadId":"58313","inReplyTo":"xmqqedxetuhg.fsf@gitster.g","subject":"Re: [PATCH v2 2/3] merge-ort: add comment to avoid surprise with new sub_flag variable","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2022-08-18T06:36:35Z","receivedAt":"2022-08-18T06:36:50Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, Aug 17, 2022 at 2:45 PM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > From: Elijah Newren \n> >\n> > Commit 4057523a40 (\"submodule merge: update conflict error message\",\n> > 2022-08-04) added a sub_flag variable that is used to store a value from\n> > enum conflict_and_info_types, but initializes it with an invalid value\n> > of -1. The code may never set it to a valid value, and use the invalid\n> > one. This can be surprising when reading over the code at first, but it\n> > was intentional. Add a comment making it clear that it is okay to be\n> > using an invalid value, due to how it is used later.\n>\n> The current code uses -1 as the \"suggest the default course of\n> action\", so -1 is very much a \"valid value\" from the viewpoint of\n> the code that suggests how to resolve. It indeed is an invalid\n> value from the viewpoint of those who maintain conflict_and_info_types\n> enum.\n>\n> > Signed-off-by: Elijah Newren \n> > ---\n> > merge-ort.c | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/merge-ort.c b/merge-ort.c\n> > index 67159fc6ef9..0a935a8135f 100644\n> > --- a/merge-ort.c\n> > +++ b/merge-ort.c\n> > @@ -1886,7 +1886,7 @@ cleanup:\n> > const char *abbrev;\n> >\n> > util = xmalloc(sizeof(*util));\n> > - util->flag = sub_flag;\n> > + util->flag = sub_flag; /* May still be -1 */\n> > util->abbrev = NULL;\n> > if (!sub_not_initialized) {\n> > abbrev = repo_find_unique_abbrev(&subrepo, b, DEFAULT_ABBREV);\n>\n> This new comment may be a slight improvement, but a valid value of\n> sub_flag is used only to signal the situation where the code does\n> not know what to suggest, which feels backwards for longer-term code\n> evolution. Presumably, we would use the util->flag field to store\n> which of the known cases we know what to suggest as we know better.\n>\n> I wonder if we should initialize the variable to the most generic\n> CONFLICT_SUBMODULE_FAILED_TO_MERGE instead of -1. The value would\n> mean \"use the default suggestion\", and the two known unworkable\n> values (not-initialized and history-not-available) are currently\n> handled according to what these two values mean. We may later add\n> more specialization based on other CONFLICT_SUBMODULE_* values.\n\nI like that; I'll make the switch.\n"},{"id":"461406","messageId":"pull.1325.v3.git.1660806927.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.v2.git.1660718028.gitgitgadget@gmail.com","subject":"[PATCH v3 0/3] Fixups for cw/submodule-merge-messages","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T07:15:24Z","receivedAt":"2022-08-18T07:15:34Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"This series fixes a few issues I noted in cw/submodule-merge-messages (which\nmerged to next a few days ago). Sorry for not responding before that topic\nmerged to next; I caught Covid near the end of my vacation and it took me\nout for a while. So here are a few patches on top instead.\n\nChanges since v1:\n\n * rebased directly on top of Calvin's patch, allowing Junio's patch in\n cw/submodule-merge-messages to be dropped.\n\nChanges since v2:\n\n * changed the second patch to instead initialize sub_flag to\n CONFLICT_SUBMODULE_FAILED_TO_MERGE, as suggested by Junio. (thanks!)\n\nElijah Newren (3):\n merge-ort: remove translator lego in new \"submodule conflict\n suggestion\"\n merge-ort: avoid surprise with new sub_flag variable\n merge-ort: provide helpful submodule update message when possible\n\n merge-ort.c | 104 +++++++++++++++++-----------------------------------\n 1 file changed, 34 insertions(+), 70 deletions(-)\n\n\nbase-commit: 4057523a4061092e9181220d54dca9eadcb75bdc\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1325%2Fnewren%2Fsubmodule-merge-messages-fixups-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1325/newren/submodule-merge-messages-fixups-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1325\n\nRange-diff vs v2:\n\n 1: 374278c6a1d = 1: 374278c6a1d merge-ort: remove translator lego in new \"submodule conflict suggestion\"\n 2: 340c0f46f74 < -: ----------- merge-ort: add comment to avoid surprise with new sub_flag variable\n -: ----------- > 2: 48200773a1b merge-ort: avoid surprise with new sub_flag variable\n 3: 80207d18334 = 3: 4c4a8f028d4 merge-ort: provide helpful submodule update message when possible\n\n-- \ngitgitgadget\n"},{"id":"461407","messageId":"374278c6a1d64e4f9ae43c9c49801997dd338850.1660806927.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.v3.git.1660806927.gitgitgadget@gmail.com","subject":"[PATCH v3 1/3] merge-ort: remove translator lego in new \"submodule conflict suggestion\"","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T07:15:25Z","receivedAt":"2022-08-18T07:15:35Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn commit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04), the new \"submodule conflict suggestion\" code was\ntranslating 6 different pieces of the new message and then used\ncarefully crafted logic to allow stitching it back together with special\nformatting. Keep the components of the message together as much as\npossible, so that:\n * we reduce the number of things translators have to translate\n * translators have more control over the format of the output\n * the code is much easier for developers to understand too\n\nAlso, reformat some comments running beyond the 80th column while at it.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 88 +++++++++++++++++------------------------------------\n 1 file changed, 28 insertions(+), 60 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex a52faf6e218..67159fc6ef9 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4481,34 +4481,6 @@ static int record_conflicted_index_entries(struct merge_options *opt)\n \treturn errs;\n }\n \n-static void format_submodule_conflict_suggestion(struct strbuf *msg) {\n-\tstruct strbuf tmp = STRBUF_INIT;\n-\tstruct string_list msg_list = STRING_LIST_INIT_DUP;\n-\tint i;\n-\n-\tstring_list_split(&msg_list, msg->buf, '\\n', -1);\n-\tfor (i = 0; i < msg_list.nr; i++) {\n-\t\tif (!i)\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: This is line item of submodule conflict message\n-\t\t\t * from print_submodule_conflict_suggestion() below. For RTL\n-\t\t\t * languages, the following swap is suggested:\n-\t\t\t * \" - %s\\n\" -> \"%s - \\n\"\n-\t\t\t */\n-\t\t\tstrbuf_addf(&tmp, _(\" - %s\\n\"), msg_list.items[i].string);\n-\t\telse\n-\t\t\t/*\n-\t\t\t * TRANSLATORS: This is line item of submodule conflict message\n-\t\t\t * from print_submodule_conflict_suggestion() below. For RTL\n-\t\t\t * languages, the following swap is suggested:\n-\t\t\t * \" %s\\n\" -> \"%s \\n\"\n-\t\t\t */\n-\t\t\tstrbuf_addf(&tmp, _(\" %s\\n\"), msg_list.items[i].string);\n-\t}\n-\tstrbuf_reset(msg);\n-\tstrbuf_add(msg, tmp.buf, tmp.len);\n-}\n-\n static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \tstruct string_list_item *item;\n \tstruct strbuf msg = STRBUF_INIT;\n@@ -4530,45 +4502,41 @@ static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \t\t\treturn;\n \t}\n \n-\tprintf(_(\"Recursive merging with submodules currently only supports \"\n-\t\t\"trivial cases.\\nPlease manually handle the merging of each \"\n-\t\t\"conflicted submodule.\\nThis can be accomplished with the following \"\n-\t\t\"steps:\"));\n-\tputchar('\\n');\n-\n+\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n \tfor_each_string_list_item(item, csub) {\n \t\tstruct conflicted_submodule_item *util = item->util;\n+\n \t\t/*\n-\t\t * TRANSLATORS: This is a line of advice to resolve a merge conflict\n-\t\t * in a submodule. The second argument is the abbreviated id of the\n-\t\t * commit that needs to be merged.\n-\t\t * E.g. - go to submodule (sub), and either merge commit abc1234\"\n+\t\t * TRANSLATORS: This is a line of advice to resolve a merge\n+\t\t * conflict in a submodule. The first argument is the submodule\n+\t\t * name, and the second argument is the abbreviated id of the\n+\t\t * commit that needs to be merged. For example:\n+\t\t * - go to submodule (mysubmodule), and either merge commit abc1234\"\n \t\t */\n-\t\tstrbuf_addf(&tmp, _(\"go to submodule (%s), and either merge commit %s\\n\"\n-\t\t\t\"or update to an existing commit which has merged those changes\"),\n-\t\t\titem->string, util->abbrev);\n-\t\tformat_submodule_conflict_suggestion(&tmp);\n-\t\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\t\tstrbuf_reset(&tmp);\n+\t\tstrbuf_addf(&tmp, _(\" - go to submodule (%s), and either merge commit %s\\n\"\n+\t\t\t\t \" or update to an existing commit which has merged those changes\\n\"),\n+\t\t\t item->string, util->abbrev);\n \t}\n-\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n-\tstrbuf_addstr(&tmp, _(\"come back to superproject and run:\"));\n-\tstrbuf_addf(&tmp, \"\\n\\ngit add %s\\n\\n\", subs.buf);\n-\tstrbuf_addstr(&tmp, _(\"to record the above merge or update\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\tstrbuf_reset(&tmp);\n-\n-\tstrbuf_addstr(&tmp, _(\"resolve any other conflicts in the superproject\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n-\tstrbuf_reset(&tmp);\n-\n-\tstrbuf_addstr(&tmp, _(\"commit the resulting index in the superproject\"));\n-\tformat_submodule_conflict_suggestion(&tmp);\n-\tstrbuf_add(&msg, tmp.buf, tmp.len);\n+\n+\t/*\n+\t * TRANSLATORS: This is a detailed message for resolving submodule\n+\t * conflicts. The first argument is string containing one step per\n+\t * submodule. The second is a space-separated list of submodule names.\n+\t */\n+\tstrbuf_addf(&msg,\n+\t\t _(\"Recursive merging with submodules currently only supports trivial cases.\\n\"\n+\t\t \"Please manually handle the merging of each conflicted submodule.\\n\"\n+\t\t \"This can be accomplished with the following steps:\\n\"\n+\t\t \"%s\"\n+\t\t \" - come back to superproject and run:\\n\\n\"\n+\t\t \" git add %s\\n\\n\"\n+\t\t \" to record the above merge or update\\n\"\n+\t\t \" - resolve any other conflicts in the superproject\\n\"\n+\t\t \" - commit the resulting index in the superproject\\n\"),\n+\t\t tmp.buf, subs.buf);\n \n \tprintf(\"%s\", msg.buf);\n+\n \tstrbuf_release(&subs);\n \tstrbuf_release(&tmp);\n \tstrbuf_release(&msg);\n-- \ngitgitgadget\n\n"},{"id":"461408","messageId":"48200773a1bbe8467e37db637c99c85be2c809ec.1660806927.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.v3.git.1660806927.gitgitgadget@gmail.com","subject":"[PATCH v3 2/3] merge-ort: avoid surprise with new sub_flag variable","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T07:15:26Z","receivedAt":"2022-08-18T07:15:37Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nCommit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04) added a sub_flag variable that is used to store a value from\nenum conflict_and_info_types, but initializes it with a value of -1 that\ndoes not correspond to any of the conflict_and_info_types. The code may\nnever set it to a valid value and yet still use it, which can be\nsurprising when reading over the code at first. Initialize it instead\nto the generic CONFLICT_SUBMODULE_FAILED_TO_MERGE value, which is still\ndistinct from the two values we need to special case.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 67159fc6ef9..8f14f1ad0b2 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -1767,7 +1767,7 @@ static int merge_submodule(struct merge_options *opt,\n \tint i;\n \tint search = !opt->priv->call_depth;\n \tint sub_not_initialized = 1;\n-\tint sub_flag = -1;\n+\tint sub_flag = CONFLICT_SUBMODULE_FAILED_TO_MERGE;\n \n \t/* store fallback answer in result in case we fail */\n \toidcpy(result, opt->priv->call_depth ? o : a);\n-- \ngitgitgadget\n\n"},{"id":"461409","messageId":"4c4a8f028d4e0a60aa30ba8c049aed339bd02381.1660806927.git.gitgitgadget@gmail.com","threadId":"58313","inReplyTo":"pull.1325.v3.git.1660806927.gitgitgadget@gmail.com","subject":"[PATCH v3 3/3] merge-ort: provide helpful submodule update message when possible","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T07:15:27Z","receivedAt":"2022-08-18T07:15:39Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nIn commit 4057523a40 (\"submodule merge: update conflict error message\",\n2022-08-04), a more detailed message was provided when submodules\nconflict, in order to help users know how to resolve those conflicts.\nThere were a couple situations for which a different message would be\nmore appropriate, but that commit left handling those for future work.\nUnfortunately, that commit would check if any submodules were of the\ntype that it didn't know how to explain, and, if so, would avoid\nproviding the more detailed explanation even for the submodules it did\nknow how to explain.\n\nChange this to have the code print the helpful messages for the subset\nof submodules it knows how to explain.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort.c | 18 +++++++-----------\n 1 file changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 8f14f1ad0b2..d94e3f17d7b 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -4490,21 +4490,17 @@ static void print_submodule_conflict_suggestion(struct string_list *csub) {\n \tif (!csub->nr)\n \t\treturn;\n \n-\t/*\n-\t * NEEDSWORK: The steps to resolve these errors deserve a more\n-\t * detailed explanation than what is currently printed below.\n-\t */\n+\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n \tfor_each_string_list_item(item, csub) {\n \t\tstruct conflicted_submodule_item *util = item->util;\n \n+\t\t/*\n+\t\t * NEEDSWORK: The steps to resolve these errors deserve a more\n+\t\t * detailed explanation than what is currently printed below.\n+\t\t */\n \t\tif (util->flag == CONFLICT_SUBMODULE_NOT_INITIALIZED ||\n-\t\t\tutil->flag == CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE)\n-\t\t\treturn;\n-\t}\n-\n-\tstrbuf_add_separated_string_list(&subs, \" \", csub);\n-\tfor_each_string_list_item(item, csub) {\n-\t\tstruct conflicted_submodule_item *util = item->util;\n+\t\t util->flag == CONFLICT_SUBMODULE_HISTORY_NOT_AVAILABLE)\n+\t\t\tcontinue;\n \n \t\t/*\n \t\t * TRANSLATORS: This is a line of advice to resolve a merge\n-- \ngitgitgadget\n"}]}