{"thread":{"id":"63211","subject":"[PATCH 0/3] rebase -r: a bugfix and two status-related improvements","startedAt":"2025-03-28T17:03:25Z","lastAt":"2025-04-04T14:13:45Z","messageCount":17,"participants":["Philippe Blain via GitGitGadget","Eric Sunshine","Phillip Wood","Johannes Schindelin","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"515239","messageId":"pull.1897.git.1743181401.gitgitgadget@gmail.com","threadId":"63211","inReplyTo":null,"subject":"[PATCH 0/3] rebase -r: a bugfix and two status-related improvements","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-28T17:03:18Z","receivedAt":"2025-03-28T17:03:25Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi,\n\nthis series started as only 3/3, which I wrote when I noticed that 'git\nstatus' suggested 'git commit' instead of 'git rebase --continue' to\nconclude a merge, and doing that I lost the original authorship of the merge\ncommit.\n\n2/3 is a small improvement I noticed along the way, and while testing these\nI discovered the bug which I fix in 1/3. I guess 1/3 could go in a different\nseries, if we prefer, but for simplicity I'm submitting them together.\n\nPhilippe Blain (3):\n  rebase -r: do create merge commit after empty resolution\n  wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase\n  wt-status: suggest 'git rebase --continue' to conclude 'merge'\n    instruction\n\n sequencer.c                |  3 +-\n t/t3418-rebase-continue.sh | 24 ++++++++++++\n t/t7512-status-help.sh     | 75 ++++++++++++++++++++++++++++++++++++++\n wt-status.c                | 49 ++++++++++++++++++-------\n wt-status.h                |  1 +\n 5 files changed, 138 insertions(+), 14 deletions(-)\n\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1897%2Fphil-blain%2Fstatus-abbreviate-merge-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1897/phil-blain/status-abbreviate-merge-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1897\n-- \ngitgitgadget\n"},{"id":"515240","messageId":"6c8f77cb71c7e0c820704b1725331f4601d8876e.1743181401.git.gitgitgadget@gmail.com","threadId":"63211","inReplyTo":"pull.1897.git.1743181401.gitgitgadget@gmail.com","subject":"[PATCH 1/3] rebase -r: do create merge commit after empty resolution","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-28T17:03:19Z","receivedAt":"2025-03-28T17:03:27Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain <levraiphilippeblain@gmail.com>\n\nWhen a user runs 'git rebase --continue' to conclude a conflicted merge\nduring a 'git rebase -r' invocation, we do not create a merge commit if\nthe resolution was empty (i.e. if the index and HEAD are identical). We\nsimply continue the rebase as if no 'merge' instruction had been given.\nThis is confusing since all commits from the side branch are absent from\nthe rebased history. What's more, if that 'merge' is the last\ninstruction in the todo list, we fail to remove the merge state, such\nthat running 'git status' shows we are still merging after the rebase\nhas concluded.\n\nThis happens because in 'sequencer.c::commit_staged_changes', we exit\nearly before calling 'run_git_commit' if 'is_clean' is true, i.e. if\nnothing is staged. Fix this by also checking for the presence of\nMERGE_HEAD before exiting early, such that we do call 'run_git_commit'\nwhen MERGE_HEAD is present. This also ensures that we unlink\ngit_path_merge_head later in 'commit_staged_changes' to clear the merge\nstate.\n\nMake sure to also remove MERGE_HEAD when a merge command fails to start.\nWe already remove MERGE_MSG since e032abd5a0 (rebase: fix rewritten list\nfor failed pick, 2023-09-06). Removing MERGE_HEAD ensures that in this\nsituation, upon 'git rebase --continue' we still exit early in\n'commit_staged_changes', without calling 'run_git_commit'. This is\nalready covered by t5407.11, which fails without this change because we\nenter 'run_git_commit' and then fail to find 'rebase_path_message'.\n\nSigned-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n---\n sequencer.c                |  3 ++-\n t/t3418-rebase-continue.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 26 insertions(+), 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex ad0ab75c8d4..2baaf716a3c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -4349,6 +4349,7 @@ static int do_merge(struct repository *r,\n \t\terror(_(\"could not even attempt to merge '%.*s'\"),\n \t\t      merge_arg_len, arg);\n \t\tunlink(git_path_merge_msg(r));\n+\t\tunlink(git_path_merge_head(r));\n \t\tgoto leave_merge;\n \t}\n \t/*\n@@ -5364,7 +5365,7 @@ static int commit_staged_changes(struct repository *r,\n \t\tflags |= AMEND_MSG;\n \t}\n \n-\tif (is_clean) {\n+\tif (is_clean && !file_exists(git_path_merge_head(r))) {\n \t\tif (refs_ref_exists(get_main_ref_store(r),\n \t\t\t\t    \"CHERRY_PICK_HEAD\") &&\n \t\t    refs_delete_ref(get_main_ref_store(r), \"\",\ndiff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\nindex 127216f7225..f4b459fea16 100755\n--- a/t/t3418-rebase-continue.sh\n+++ b/t/t3418-rebase-continue.sh\n@@ -111,6 +111,30 @@ test_expect_success 'rebase -r passes merge strategy options correctly' '\n \tgit rebase --continue\n '\n \n+test_expect_success '--continue creates merge commit after empty resolution' '\n+\tgit reset --hard main &&\n+\tgit checkout -b rebase_i_merge &&\n+\ttest_commit unrelated &&\n+\tgit checkout -b rebase_i_merge_side &&\n+\ttest_commit side2 main.txt &&\n+\tgit checkout rebase_i_merge &&\n+\ttest_commit side1 main.txt &&\n+\tPICK=$(git rev-parse --short rebase_i_merge) &&\n+\ttest_must_fail git merge rebase_i_merge_side &&\n+\techo side1 >main.txt &&\n+\tgit add main.txt &&\n+\ttest_tick &&\n+\tgit commit --no-edit &&\n+\tFAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" &&\n+\texport FAKE_LINES &&\n+\ttest_must_fail git rebase -ir main &&\n+\techo side1 >main.txt &&\n+\tgit add main.txt &&\n+\tgit rebase --continue &&\n+\tgit log --merges >out &&\n+\ttest_grep \"Merge branch '\\''rebase_i_merge_side'\\''\" out\n+'\n+\n test_expect_success '--skip after failed fixup cleans commit message' '\n \ttest_when_finished \"test_might_fail git rebase --abort\" &&\n \tgit checkout -b with-conflicting-fixup &&\n-- \ngitgitgadget\n\n"},{"id":"515241","messageId":"e297b71ba123b642c2e724d7dda475fa52dfdeaa.1743181401.git.gitgitgadget@gmail.com","threadId":"63211","inReplyTo":"pull.1897.git.1743181401.gitgitgadget@gmail.com","subject":"[PATCH 2/3] wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-28T17:03:20Z","receivedAt":"2025-03-28T17:03:28Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain <levraiphilippeblain@gmail.com>\n\nWhen \"git status\" is invoked during a rebase, we print the last commands\ndone and the next commands to do, and abbreviate commit hashes found in\nthose lines. However, we only abbreviate hashes in 'pick', 'squash' and\nplain 'fixup' lines, not those in 'merge -C' and 'fixup -C' lines, as\nthe parsing done in wt-status.c::abbrev_oid_in_line is not prepared for\nsuch lines.\n\nImprove the parsing done by this function by special casing 'fixup' and\n'merge' such that the hash to abbreviate is the string found in the\nthird field of 'split', instead of the second one for other commands.\nIntroduce a 'hash' strbuf pointer to point to the correct field in all\ncases.\n\nSigned-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n---\n wt-status.c | 31 ++++++++++++++++++++++---------\n 1 file changed, 22 insertions(+), 9 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 1da5732f57b..d11d9f9f142 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1342,9 +1342,11 @@ static int split_commit_in_progress(struct wt_status *s)\n \n /*\n  * Turn\n- * \"pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message\"\n+ * \"pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message\" and\n+ * \"merge -C d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some-branch\"\n  * into\n- * \"pick d6a2f03 some message\"\n+ * \"pick d6a2f03 some message\" and\n+ * \"merge -C d6a2f03 some-branch\"\n  *\n  * The function assumes that the line does not contain useless spaces\n  * before or after the command.\n@@ -1360,20 +1362,31 @@ static void abbrev_oid_in_line(struct strbuf *line)\n \t    starts_with(line->buf, \"l \"))\n \t\treturn;\n \n-\tsplit = strbuf_split_max(line, ' ', 3);\n+\tsplit = strbuf_split_max(line, ' ', 4);\n \tif (split[0] && split[1]) {\n \t\tstruct object_id oid;\n-\n+\t\tstruct strbuf *hash;\n+\n+\t\tif ((!strcmp(split[0]->buf, \"merge \") ||\n+\t\t     !strcmp(split[0]->buf, \"m \"    ) ||\n+\t\t     !strcmp(split[0]->buf, \"fixup \") ||\n+\t\t     !strcmp(split[0]->buf, \"f \"    )) &&\n+\t\t    (!strcmp(split[1]->buf, \"-C \") ||\n+\t\t     !strcmp(split[1]->buf, \"-c \"))) {\n+\t\t\thash = split[2];\n+\t\t} else {\n+\t\t\thash = split[1];\n+\t\t}\n \t\t/*\n \t\t * strbuf_split_max left a space. Trim it and re-add\n \t\t * it after abbreviation.\n \t\t */\n-\t\tstrbuf_trim(split[1]);\n-\t\tif (!repo_get_oid(the_repository, split[1]->buf, &oid)) {\n-\t\t\tstrbuf_reset(split[1]);\n-\t\t\tstrbuf_add_unique_abbrev(split[1], &oid,\n+\t\tstrbuf_trim(hash);\n+\t\tif (!repo_get_oid(the_repository, hash->buf, &oid)) {\n+\t\t\tstrbuf_reset(hash);\n+\t\t\tstrbuf_add_unique_abbrev(hash, &oid,\n \t\t\t\t\t\t DEFAULT_ABBREV);\n-\t\t\tstrbuf_addch(split[1], ' ');\n+\t\t\tstrbuf_addch(hash, ' ');\n \t\t\tstrbuf_reset(line);\n \t\t\tfor (i = 0; split[i]; i++)\n \t\t\t\tstrbuf_addbuf(line, split[i]);\n-- \ngitgitgadget\n\n"},{"id":"515242","messageId":"db01acdd062a17b1cca62428eba8c3ed62ca7c6a.1743181401.git.gitgitgadget@gmail.com","threadId":"63211","inReplyTo":"pull.1897.git.1743181401.gitgitgadget@gmail.com","subject":"[PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"Philippe Blain via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-28T17:03:21Z","receivedAt":"2025-03-28T17:03:28Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"From: Philippe Blain <levraiphilippeblain@gmail.com>\n\nSince 982288e9bd (status: rebase and merge can be in progress at the\nsame time, 2018-11-12), when a merge is in progress as part of a 'git\nrebase -r' operation, 'wt_longstatus_print_state' shows information\nabout the in-progress rebase (via show_rebase_information), and then\ncalls 'show_merge_in_progress' to help the user conclude the merge. This\nfunction suggests using 'git commit' to do so, but this throws away the\nauthorship information from the original merge, which is not ideal.\nUsing 'git rebase --continue' instead preserves the authorship\ninformation, since we enter 'sequencer.c:run_git_commit' which calls\nread_env_script to read the author-script file.\n\nNote however that this only works when a merge was scheduled using a\n'merge' instruction in the rebase todo list. Indeed, when using 'exec\ngit merge', the state files necessary for 'git rebase --continue' are\nnot present, and one must use 'git commit' (or 'git merge --continue')\nin that case.\n\nBe more helpful to the user by suggesting either 'git rebase\n--continue', when the merge was scheduled using a 'merge' instruction,\nand 'git commit' otherwise. As such, add a\n'merge_during_rebase_in_progress' field to 'struct wt_status_state', and\ndetect this situation in wt_status_check_rebase by looking at the last\ncommand done. Adjust wt_longstatus_print_state to check this field and\nsuggest 'git rebase --continue' if a merge came from a 'merge'\ninstruction, by calling show_rebase_in_progress directly.\n\nAdd two tests for the new behaviour, using 'merge' and 'exec git merge'\ninstructions.\n\nSigned-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n---\n t/t7512-status-help.sh | 75 ++++++++++++++++++++++++++++++++++++++++++\n wt-status.c            | 18 +++++++---\n wt-status.h            |  1 +\n 3 files changed, 90 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh\nindex 802f8f704c6..b37e99625b4 100755\n--- a/t/t7512-status-help.sh\n+++ b/t/t7512-status-help.sh\n@@ -183,6 +183,81 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'status during rebase -ir after conflicted merge (exec git merge)' '\n+\tgit reset --hard main &&\n+\tgit checkout -b rebase_i_merge &&\n+\ttest_commit unrelated &&\n+\tgit checkout -b rebase_i_merge_side &&\n+\ttest_commit side2 main.txt &&\n+\tgit checkout rebase_i_merge &&\n+\ttest_commit side1 main.txt &&\n+\tPICK=$(git rev-parse --short rebase_i_merge) &&\n+\ttest_must_fail git merge rebase_i_merge_side &&\n+\techo side1 >main.txt &&\n+\tgit add main.txt &&\n+\ttest_tick &&\n+\tgit commit --no-edit &&\n+\tMERGE=$(git rev-parse --short rebase_i_merge) &&\n+\tONTO=$(git rev-parse --short main) &&\n+\ttest_when_finished \"git rebase --abort\" &&\n+\tFAKE_LINES=\"1 2 3 5 6 7 8 9 10 exec_git_merge_refs/rewritten/rebase-i-merge-side\" &&\n+\texport FAKE_LINES &&\n+\ttest_must_fail git rebase -ir main &&\n+\tcat >expect <<EOF &&\n+interactive rebase in progress; onto $ONTO\n+Last commands done (8 commands done):\n+   pick $PICK side1\n+   exec git merge refs/rewritten/rebase-i-merge-side\n+  (see more in file .git/rebase-merge/done)\n+No commands remaining.\n+\n+You have unmerged paths.\n+  (fix conflicts and run \"git commit\")\n+  (use \"git merge --abort\" to abort the merge)\n+\n+Unmerged paths:\n+  (use \"git add <file>...\" to mark resolution)\n+\tboth modified:   main.txt\n+\n+no changes added to commit (use \"git add\" and/or \"git commit -a\")\n+EOF\n+\tgit status --untracked-files=no >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'status during rebase -ir after replaying conflicted merge (merge)' '\n+\tPICK=$(git rev-parse --short :/side1) &&\n+\tUNRELATED=$(git rev-parse --short :/unrelated) &&\n+\tMERGE=$(git rev-parse --short rebase_i_merge) &&\n+\tONTO=$(git rev-parse --short main) &&\n+\ttest_when_finished \"git rebase --abort\" &&\n+\tFAKE_LINES=\"1 2 3 5 6 7 8 9 10 11 4\" &&\n+\texport FAKE_LINES &&\n+\ttest_must_fail git rebase -ir main &&\n+\tcat >expect <<EOF &&\n+interactive rebase in progress; onto $ONTO\n+Last commands done (8 commands done):\n+   pick $PICK side1\n+   merge -C $MERGE rebase-i-merge-side # Merge branch '\\''rebase_i_merge_side'\\'' into rebase_i_merge\n+  (see more in file .git/rebase-merge/done)\n+Next command to do (1 remaining command):\n+   pick $UNRELATED unrelated\n+  (use \"git rebase --edit-todo\" to view and edit)\n+You are currently rebasing branch '\\''rebase_i_merge'\\'' on '\\''$ONTO'\\''.\n+  (fix conflicts and then run \"git rebase --continue\")\n+  (use \"git rebase --skip\" to skip this patch)\n+  (use \"git rebase --abort\" to check out the original branch)\n+\n+Unmerged paths:\n+  (use \"git add <file>...\" to mark resolution)\n+\tboth modified:   main.txt\n+\n+no changes added to commit (use \"git add\" and/or \"git commit -a\")\n+EOF\n+\tgit status --untracked-files=no >actual &&\n+\ttest_cmp expect actual\n+'\n+\n \n test_expect_success 'status when rebasing -i in edit mode' '\n \tgit reset --hard main &&\ndiff --git a/wt-status.c b/wt-status.c\nindex d11d9f9f142..f15495039e3 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1744,6 +1744,7 @@ int wt_status_check_rebase(const struct worktree *wt,\n \t\t\t   struct wt_status_state *state)\n {\n \tstruct stat st;\n+\tstruct string_list have_done = STRING_LIST_INIT_DUP;\n \n \tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply\"), &st)) {\n \t\tif (!stat(worktree_git_path(the_repository, wt, \"rebase-apply/applying\"), &st)) {\n@@ -1760,8 +1761,12 @@ int wt_status_check_rebase(const struct worktree *wt,\n \t\t\tstate->rebase_interactive_in_progress = 1;\n \t\telse\n \t\t\tstate->rebase_in_progress = 1;\n+\t\tread_rebase_todolist(\"rebase-merge/done\", &have_done);\n+\t\tif (have_done.nr > 0 && starts_with(have_done.items[have_done.nr - 1].string, \"merge\"))\n+\t\t\t\tstate->merge_during_rebase_in_progress = 1;\n \t\tstate->branch = get_branch(wt, \"rebase-merge/head-name\");\n \t\tstate->onto = get_branch(wt, \"rebase-merge/onto\");\n+\t\tstring_list_clear(&have_done, 0);\n \t} else\n \t\treturn 0;\n \treturn 1;\n@@ -1855,10 +1860,15 @@ static void wt_longstatus_print_state(struct wt_status *s)\n \n \tif (state->merge_in_progress) {\n \t\tif (state->rebase_interactive_in_progress) {\n-\t\t\tshow_rebase_information(s, state_color);\n-\t\t\tfputs(\"\\n\", s->fp);\n-\t\t}\n-\t\tshow_merge_in_progress(s, state_color);\n+\t\t\tif (state->merge_during_rebase_in_progress)\n+\t\t\t\tshow_rebase_in_progress(s, state_color);\n+\t\t\telse {\n+\t\t\t\tshow_rebase_information(s, state_color);\n+\t\t\t\tfputs(\"\\n\", s->fp);\n+\t\t\t\tshow_merge_in_progress(s, state_color);\n+\t\t\t}\n+\t\t} else\n+\t\t\tshow_merge_in_progress(s, state_color);\n \t} else if (state->am_in_progress)\n \t\tshow_am_in_progress(s, state_color);\n \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\ndiff --git a/wt-status.h b/wt-status.h\nindex 4e377ce62b8..84bedfcd48f 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -87,6 +87,7 @@ struct wt_status_state {\n \tint am_empty_patch;\n \tint rebase_in_progress;\n \tint rebase_interactive_in_progress;\n+\tint merge_during_rebase_in_progress;\n \tint cherry_pick_in_progress;\n \tint bisect_in_progress;\n \tint revert_in_progress;\n-- \ngitgitgadget\n"},{"id":"515249","messageId":"CAPig+cS92W_gYuNsaTvQxiP3xBK7Wpg0__uVkgAU1x0OFJUZgQ@mail.gmail.com","threadId":"63211","inReplyTo":"6c8f77cb71c7e0c820704b1725331f4601d8876e.1743181401.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] rebase -r: do create merge commit after empty resolution","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-03-28T17:14:57Z","receivedAt":"2025-03-28T17:15:09Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Mar 28, 2025 at 1:03 PM Philippe Blain via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> When a user runs 'git rebase --continue' to conclude a conflicted merge\n> during a 'git rebase -r' invocation, we do not create a merge commit if\n> the resolution was empty (i.e. if the index and HEAD are identical). We\n> simply continue the rebase as if no 'merge' instruction had been given.\n> This is confusing since all commits from the side branch are absent from\n> the rebased history. What's more, if that 'merge' is the last\n> instruction in the todo list, we fail to remove the merge state, such\n> that running 'git status' shows we are still merging after the rebase\n> has concluded.\n> [...]\n> Make sure to also remove MERGE_HEAD when a merge command fails to start.\n> We already remove MERGE_MSG since e032abd5a0 (rebase: fix rewritten list\n> for failed pick, 2023-09-06). Removing MERGE_HEAD ensures that in this\n> situation, upon 'git rebase --continue' we still exit early in\n> 'commit_staged_changes', without calling 'run_git_commit'. This is\n> already covered by t5407.11, which fails without this change because we\n> enter 'run_git_commit' and then fail to find 'rebase_path_message'.\n>\n> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> ---\n> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> +test_expect_success '--continue creates merge commit after empty resolution' '\n> +       git reset --hard main &&\n> +       git checkout -b rebase_i_merge &&\n> +       test_commit unrelated &&\n> +       git checkout -b rebase_i_merge_side &&\n> +       test_commit side2 main.txt &&\n> +       git checkout rebase_i_merge &&\n> +       test_commit side1 main.txt &&\n> +       PICK=$(git rev-parse --short rebase_i_merge) &&\n> +       test_must_fail git merge rebase_i_merge_side &&\n> +       echo side1 >main.txt &&\n> +       git add main.txt &&\n> +       test_tick &&\n> +       git commit --no-edit &&\n> +       FAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" &&\n> +       export FAKE_LINES &&\n> +       test_must_fail git rebase -ir main &&\n\nI don't think you want to be setting FAKE_LINES like this since doing\nso will pollute the environment for all tests following this one. You\ncan find existing precedent in this script which demonstrates the\ncorrect way to handle this case. Specifically, you'd want:\n\n    test_must_fail env FAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" \\\n        git rebase -ir main &&\n\n> +       echo side1 >main.txt &&\n> +       git add main.txt &&\n> +       git rebase --continue &&\n> +       git log --merges >out &&\n> +       test_grep \"Merge branch '\\''rebase_i_merge_side'\\''\" out\n\nYou could take advantage of the SQ variable defined by t/test-lib.sh\nto make this a bit easier to digest:\n\n    test_grep \"Merge branch ${SQ}rebase_i_merge_side${SQ}\" out\n\nOr, even simpler, you'll find that some test scripts just use regex\nwildcard \".\" to make the needle even more readable:\n\n    test_grep \"Merge branch .rebase_i_merge_side.\" out\n"},{"id":"515250","messageId":"CAPig+cThwsBdumXB3m2ZA-_tmDVTMojkYx7_YxNp49eK6a2HMg@mail.gmail.com","threadId":"63211","inReplyTo":"CAPig+cS92W_gYuNsaTvQxiP3xBK7Wpg0__uVkgAU1x0OFJUZgQ@mail.gmail.com","subject":"Re: [PATCH 1/3] rebase -r: do create merge commit after empty resolution","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-03-28T17:23:40Z","receivedAt":"2025-03-28T17:23:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Mar 28, 2025 at 1:14 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> On Fri, Mar 28, 2025 at 1:03 PM Philippe Blain via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> > +test_expect_success '--continue creates merge commit after empty resolution' '\n> > +       [...]\n> > +       git commit --no-edit &&\n> > +       FAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" &&\n> > +       export FAKE_LINES &&\n> > +       test_must_fail git rebase -ir main &&\n>\n> I don't think you want to be setting FAKE_LINES like this since doing\n> so will pollute the environment for all tests following this one. You\n> can find existing precedent in this script which demonstrates the\n> correct way to handle this case. Specifically, you'd want:\n>\n>     test_must_fail env FAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" \\\n>         git rebase -ir main &&\n\nTo clarify, by \"pollute\", I mean that it can impact subsequent tests\nwhich don't take care to override FAKE_LINES as necessary. There\ncertainly are test scripts which use the:\n\n    FAKE_LINES=... &&\n    export FAKE_LINES &&\n\nform successfully, but such scripts are careful to override/set\nFAKE_LINES in every test. This particular script (t3418), on the other\nhand, does not otherwise employ the form in which the variable is\nexported, so introducing it in a test which is inserted into the\nmiddle of the script feels dangerous.\n"},{"id":"515373","messageId":"b0263bdb-002a-4a88-b277-fd2afe59cfe6@gmail.com","threadId":"63211","inReplyTo":"6c8f77cb71c7e0c820704b1725331f4601d8876e.1743181401.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] rebase -r: do create merge commit after empty resolution","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-31T15:37:41Z","receivedAt":"2025-03-31T15:37:44Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philippe\n\nOn 28/03/2025 17:03, Philippe Blain via GitGitGadget wrote:\n> From: Philippe Blain <levraiphilippeblain@gmail.com>\n> \n> When a user runs 'git rebase --continue' to conclude a conflicted merge\n> during a 'git rebase -r' invocation, we do not create a merge commit if\n> the resolution was empty (i.e. if the index and HEAD are identical). We\n> simply continue the rebase as if no 'merge' instruction had been given.\n> This is confusing since all commits from the side branch are absent from\n> the rebased history. What's more, if that 'merge' is the last\n> instruction in the todo list, we fail to remove the merge state, such\n> that running 'git status' shows we are still merging after the rebase\n> has concluded.\n> \n> This happens because in 'sequencer.c::commit_staged_changes', we exit\n> early before calling 'run_git_commit' if 'is_clean' is true, i.e. if\n> nothing is staged. Fix this by also checking for the presence of\n> MERGE_HEAD before exiting early, such that we do call 'run_git_commit'\n> when MERGE_HEAD is present. This also ensures that we unlink\n> git_path_merge_head later in 'commit_staged_changes' to clear the merge\n> state.\n> \n> Make sure to also remove MERGE_HEAD when a merge command fails to start.\n> We already remove MERGE_MSG since e032abd5a0 (rebase: fix rewritten list\n> for failed pick, 2023-09-06). Removing MERGE_HEAD ensures that in this\n> situation, upon 'git rebase --continue' we still exit early in\n> 'commit_staged_changes', without calling 'run_git_commit'. This is\n> already covered by t5407.11, which fails without this change because we\n> enter 'run_git_commit' and then fail to find 'rebase_path_message'.\n\nThanks for fixing this.\n\n> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> ---\n>   sequencer.c                |  3 ++-\n>   t/t3418-rebase-continue.sh | 24 ++++++++++++++++++++++++\n>   2 files changed, 26 insertions(+), 1 deletion(-)\n> \n> diff --git a/sequencer.c b/sequencer.c\n> index ad0ab75c8d4..2baaf716a3c 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -4349,6 +4349,7 @@ static int do_merge(struct repository *r,\n>   \t\terror(_(\"could not even attempt to merge '%.*s'\"),\n>   \t\t      merge_arg_len, arg);\n>   \t\tunlink(git_path_merge_msg(r));\n> +\t\tunlink(git_path_merge_head(r));\n\n\nI think we want to clean up git_path_merge_mode() as well. Perhaps we \nshould call remove_merge_branch_state() instead of deleting the \nindividual files ourselves here.\n\n> +test_expect_success '--continue creates merge commit after empty resolution' '\n> +\tgit reset --hard main &&\n> +\tgit checkout -b rebase_i_merge &&\n> +\ttest_commit unrelated &&\n> +\tgit checkout -b rebase_i_merge_side &&\n> +\ttest_commit side2 main.txt &&\n> +\tgit checkout rebase_i_merge &&\n> +\ttest_commit side1 main.txt &&\n> +\tPICK=$(git rev-parse --short rebase_i_merge) &&\n> +\ttest_must_fail git merge rebase_i_merge_side &&\n> +\techo side1 >main.txt &&\n> +\tgit add main.txt &&\n> +\ttest_tick &&\n> +\tgit commit --no-edit &&\n> +\tFAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" &&\n> +\texport FAKE_LINES &&\n> +\ttest_must_fail git rebase -ir main &&\n> +\techo side1 >main.txt &&\n> +\tgit add main.txt &&\n> +\tgit rebase --continue &&\n> +\tgit log --merges >out &&\n> +\ttest_grep \"Merge branch '\\''rebase_i_merge_side'\\''\" out\n> +'\n\nI wonder if t3430 would be a better home for this as it already has the \nsetup necessary to create a failing merge. It would be good to add a \ntest to check that \"git rebase --skip\" does not create an empty merge as \nwell.\n\nThanks\n\nPhillip\n\n>   test_expect_success '--skip after failed fixup cleans commit message' '\n>   \ttest_when_finished \"test_might_fail git rebase --abort\" &&\n>   \tgit checkout -b with-conflicting-fixup &&\n\n"},{"id":"515374","messageId":"69b0ab3f-2d6c-49da-866e-71c0eb907f7f@gmail.com","threadId":"63211","inReplyTo":"e297b71ba123b642c2e724d7dda475fa52dfdeaa.1743181401.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-31T15:37:56Z","receivedAt":"2025-03-31T15:37:59Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philippe\n\nOn 28/03/2025 17:03, Philippe Blain via GitGitGadget wrote:\n> From: Philippe Blain <levraiphilippeblain@gmail.com>\n> \n> When \"git status\" is invoked during a rebase, we print the last commands\n> done and the next commands to do, and abbreviate commit hashes found in\n> those lines. However, we only abbreviate hashes in 'pick', 'squash' and\n> plain 'fixup' lines, not those in 'merge -C' and 'fixup -C' lines, as\n> the parsing done in wt-status.c::abbrev_oid_in_line is not prepared for\n> such lines.\n> \n> Improve the parsing done by this function by special casing 'fixup' and\n> 'merge' such that the hash to abbreviate is the string found in the\n> third field of 'split', instead of the second one for other commands.\n> Introduce a 'hash' strbuf pointer to point to the correct field in all\n> cases.\n\nSounds good. It is a shame that the parsing here is not better \nintegrated with the sequencer. I think that would be a much bigger task \nthough. The patch looks good and is definitely an improvement on the \nstatus quo for the user.\n\nI was going to ask about a test but it looks like one of the tests added \nin the next patch checks that we abbreviate \"merge -C <oid>\". It would \nbe worth mentioning that in the commit message.\n\nThanks\n\nPhillip\n\n> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>\n> ---\n>   wt-status.c | 31 ++++++++++++++++++++++---------\n>   1 file changed, 22 insertions(+), 9 deletions(-)\n> \n> diff --git a/wt-status.c b/wt-status.c\n> index 1da5732f57b..d11d9f9f142 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -1342,9 +1342,11 @@ static int split_commit_in_progress(struct wt_status *s)\n>   \n>   /*\n>    * Turn\n> - * \"pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message\"\n> + * \"pick d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some message\" and\n> + * \"merge -C d6a2f0303e897ec257dd0e0a39a5ccb709bc2047 some-branch\"\n>    * into\n> - * \"pick d6a2f03 some message\"\n> + * \"pick d6a2f03 some message\" and\n> + * \"merge -C d6a2f03 some-branch\"\n>    *\n>    * The function assumes that the line does not contain useless spaces\n>    * before or after the command.\n> @@ -1360,20 +1362,31 @@ static void abbrev_oid_in_line(struct strbuf *line)\n>   \t    starts_with(line->buf, \"l \"))\n>   \t\treturn;\n>   \n> -\tsplit = strbuf_split_max(line, ' ', 3);\n> +\tsplit = strbuf_split_max(line, ' ', 4);\n>   \tif (split[0] && split[1]) {\n>   \t\tstruct object_id oid;\n> -\n> +\t\tstruct strbuf *hash;\n> +\n> +\t\tif ((!strcmp(split[0]->buf, \"merge \") ||\n> +\t\t     !strcmp(split[0]->buf, \"m \"    ) ||\n> +\t\t     !strcmp(split[0]->buf, \"fixup \") ||\n> +\t\t     !strcmp(split[0]->buf, \"f \"    )) &&\n> +\t\t    (!strcmp(split[1]->buf, \"-C \") ||\n> +\t\t     !strcmp(split[1]->buf, \"-c \"))) {\n> +\t\t\thash = split[2];\n> +\t\t} else {\n> +\t\t\thash = split[1];\n> +\t\t}\n>   \t\t/*\n>   \t\t * strbuf_split_max left a space. Trim it and re-add\n>   \t\t * it after abbreviation.\n>   \t\t */\n> -\t\tstrbuf_trim(split[1]);\n> -\t\tif (!repo_get_oid(the_repository, split[1]->buf, &oid)) {\n> -\t\t\tstrbuf_reset(split[1]);\n> -\t\t\tstrbuf_add_unique_abbrev(split[1], &oid,\n> +\t\tstrbuf_trim(hash);\n> +\t\tif (!repo_get_oid(the_repository, hash->buf, &oid)) {\n> +\t\t\tstrbuf_reset(hash);\n> +\t\t\tstrbuf_add_unique_abbrev(hash, &oid,\n>   \t\t\t\t\t\t DEFAULT_ABBREV);\n> -\t\t\tstrbuf_addch(split[1], ' ');\n> +\t\t\tstrbuf_addch(hash, ' ');\n>   \t\t\tstrbuf_reset(line);\n>   \t\t\tfor (i = 0; split[i]; i++)\n>   \t\t\t\tstrbuf_addbuf(line, split[i]);\n\n"},{"id":"515375","messageId":"f0d1f0ba-84ab-4914-9dd1-81a5d2e0dbc3@gmail.com","threadId":"63211","inReplyTo":"db01acdd062a17b1cca62428eba8c3ed62ca7c6a.1743181401.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-31T15:38:03Z","receivedAt":"2025-03-31T15:38:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philippe\n\nOn 28/03/2025 17:03, Philippe Blain via GitGitGadget wrote:\n> From: Philippe Blain <levraiphilippeblain@gmail.com>\n> \n> Since 982288e9bd (status: rebase and merge can be in progress at the\n> same time, 2018-11-12), when a merge is in progress as part of a 'git\n> rebase -r' operation, 'wt_longstatus_print_state' shows information\n> about the in-progress rebase (via show_rebase_information), and then\n> calls 'show_merge_in_progress' to help the user conclude the merge. This\n> function suggests using 'git commit' to do so, but this throws away the\n> authorship information from the original merge, which is not ideal.\n> Using 'git rebase --continue' instead preserves the authorship\n> information, since we enter 'sequencer.c:run_git_commit' which calls\n> read_env_script to read the author-script file.\n\nGood catch\n\n> Note however that this only works when a merge was scheduled using a\n> 'merge' instruction in the rebase todo list. Indeed, when using 'exec\n> git merge', the state files necessary for 'git rebase --continue' are\n> not present, and one must use 'git commit' (or 'git merge --continue')\n> in that case.\n> \n> Be more helpful to the user by suggesting either 'git rebase\n> --continue', when the merge was scheduled using a 'merge' instruction,\n> and 'git commit' otherwise. As such, add a\n> 'merge_during_rebase_in_progress' field to 'struct wt_status_state', and\n> detect this situation in wt_status_check_rebase by looking at the last\n> command done. Adjust wt_longstatus_print_state to check this field and\n> suggest 'git rebase --continue' if a merge came from a 'merge'\n> instruction, by calling show_rebase_in_progress directly.\n> \n> Add two tests for the new behaviour, using 'merge' and 'exec git merge'\n> instructions.\n\nNice, thanks for adding the tests\n\n>   \n> +test_expect_success 'status during rebase -ir after conflicted merge (exec git merge)' '\n> +\tgit reset --hard main &&\n> +\tgit checkout -b rebase_i_merge &&\n> +\ttest_commit unrelated &&\n> +\tgit checkout -b rebase_i_merge_side &&\n> +\ttest_commit side2 main.txt &&\n> +\tgit checkout rebase_i_merge &&\n> +\ttest_commit side1 main.txt &&\n> +\tPICK=$(git rev-parse --short rebase_i_merge) &&\n> +\ttest_must_fail git merge rebase_i_merge_side &&\n> +\techo side1 >main.txt &&\n> +\tgit add main.txt &&\n> +\ttest_tick &&\n> +\tgit commit --no-edit &&\n> +\tMERGE=$(git rev-parse --short rebase_i_merge) &&\n> +\tONTO=$(git rev-parse --short main) &&\n> +\ttest_when_finished \"git rebase --abort\" &&\n> +\tFAKE_LINES=\"1 2 3 5 6 7 8 9 10 exec_git_merge_refs/rewritten/rebase-i-merge-side\" &&\n> +\texport FAKE_LINES &&\n> +\ttest_must_fail git rebase -ir main &&\n\nAs with the other patch this should be\n\n\ttest_must_fail env FAKE_LINES=... git rebase ...\n\nand the same for the test below. These tests show just how opaque the \nFAKE_LINES mechanism is - I've got no idea what it's doing. If it is not \ntoo much work it might be worth writing out the desired todo list to a \nfile and using set_replace_editor. If you do that note that you can use \ntag names in the todo list you don't need to get the oid for each commit \nand you probably don't need to rebase the side branch, just the merge.\n> @@ -1760,8 +1761,12 @@ int wt_status_check_rebase(const struct worktree *wt,\n>   \t\t\tstate->rebase_interactive_in_progress = 1;\n>   \t\telse\n>   \t\t\tstate->rebase_in_progress = 1;\n> +\t\tread_rebase_todolist(\"rebase-merge/done\", &have_done);\n> +\t\tif (have_done.nr > 0 && starts_with(have_done.items[have_done.nr - 1].string, \"merge\"))\n> +\t\t\t\tstate->merge_during_rebase_in_progress = 1;\nWe already read and parse the done list in show_rebase_information() - \nis it possible to avoid doing that twice by setting this flag there?\n\n>   \t\tstate->branch = get_branch(wt, \"rebase-merge/head-name\");\n>   \t\tstate->onto = get_branch(wt, \"rebase-merge/onto\");\n> +\t\tstring_list_clear(&have_done, 0);\n>   \t} else\n>   \t\treturn 0;\n>   \treturn 1;\n> @@ -1855,10 +1860,15 @@ static void wt_longstatus_print_state(struct wt_status *s)\n>   \n>   \tif (state->merge_in_progress) {\n>   \t\tif (state->rebase_interactive_in_progress) {\n> -\t\t\tshow_rebase_information(s, state_color);\n> -\t\t\tfputs(\"\\n\", s->fp);\n> -\t\t}\n> -\t\tshow_merge_in_progress(s, state_color);\n> +\t\t\tif (state->merge_during_rebase_in_progress)\n> +\t\t\t\tshow_rebase_in_progress(s, state_color);\n> +\t\t\telse {\n> +\t\t\t\tshow_rebase_information(s, state_color);\n> +\t\t\t\tfputs(\"\\n\", s->fp);\n> +\t\t\t\tshow_merge_in_progress(s, state_color);\n> +\t\t\t}\n\nThe indentation here looks strange\n\nThanks\n\nPhillip\n\n> +\t\t} else\n> +\t\t\tshow_merge_in_progress(s, state_color);\n>   \t} else if (state->am_in_progress)\n>   \t\tshow_am_in_progress(s, state_color);\n>   \telse if (state->rebase_in_progress || state->rebase_interactive_in_progress)\n> diff --git a/wt-status.h b/wt-status.h\n> index 4e377ce62b8..84bedfcd48f 100644\n> --- a/wt-status.h\n> +++ b/wt-status.h\n> @@ -87,6 +87,7 @@ struct wt_status_state {\n>   \tint am_empty_patch;\n>   \tint rebase_in_progress;\n>   \tint rebase_interactive_in_progress;\n> +\tint merge_during_rebase_in_progress;\n>   \tint cherry_pick_in_progress;\n>   \tint bisect_in_progress;\n>   \tint revert_in_progress;\n\n"},{"id":"515376","messageId":"31f658d2-1665-4cda-9625-2c9503d549b5@gmail.com","threadId":"63211","inReplyTo":"pull.1897.git.1743181401.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] rebase -r: a bugfix and two status-related improvements","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-31T15:38:58Z","receivedAt":"2025-03-31T15:39:02Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Philippe\n\nOn 28/03/2025 17:03, Philippe Blain via GitGitGadget wrote:\n> Hi,\n> \n> this series started as only 3/3, which I wrote when I noticed that 'git\n> status' suggested 'git commit' instead of 'git rebase --continue' to\n> conclude a merge, and doing that I lost the original authorship of the merge\n> commit.\n> \n> 2/3 is a small improvement I noticed along the way, and while testing these\n> I discovered the bug which I fix in 1/3. I guess 1/3 could go in a different\n> series, if we prefer, but for simplicity I'm submitting them together.\n\nThanks for working on this. I've left some comments but the fundamentals \nof this series look sound.\n\nBest Wishes\n\nPhillip\n\n> Philippe Blain (3):\n>    rebase -r: do create merge commit after empty resolution\n>    wt-status: also abbreviate 'merge' and 'fixup -C' lines during rebase\n>    wt-status: suggest 'git rebase --continue' to conclude 'merge'\n>      instruction\n> \n>   sequencer.c                |  3 +-\n>   t/t3418-rebase-continue.sh | 24 ++++++++++++\n>   t/t7512-status-help.sh     | 75 ++++++++++++++++++++++++++++++++++++++\n>   wt-status.c                | 49 ++++++++++++++++++-------\n>   wt-status.h                |  1 +\n>   5 files changed, 138 insertions(+), 14 deletions(-)\n> \n> \n> base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1897%2Fphil-blain%2Fstatus-abbreviate-merge-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1897/phil-blain/status-abbreviate-merge-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1897\n\n"},{"id":"515445","messageId":"417b8ff4-475b-6f00-0753-d3f9e3a528b5@gmx.de","threadId":"63211","inReplyTo":"CAPig+cThwsBdumXB3m2ZA-_tmDVTMojkYx7_YxNp49eK6a2HMg@mail.gmail.com","subject":"Re: [PATCH 1/3] rebase -r: do create merge commit after empty resolution","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-04-01T16:17:35Z","receivedAt":"2025-04-01T16:17:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Fri, 28 Mar 2025, Eric Sunshine wrote:\n\n> On Fri, Mar 28, 2025 at 1:14 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> > On Fri, Mar 28, 2025 at 1:03 PM Philippe Blain via GitGitGadget\n> > <gitgitgadget@gmail.com> wrote:\n> > > diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh\n> > > +test_expect_success '--continue creates merge commit after empty resolution' '\n> > > +       [...]\n> > > +       git commit --no-edit &&\n> > > +       FAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" &&\n> > > +       export FAKE_LINES &&\n> > > +       test_must_fail git rebase -ir main &&\n> >\n> > I don't think you want to be setting FAKE_LINES like this since doing\n> > so will pollute the environment for all tests following this one. You\n> > can find existing precedent in this script which demonstrates the\n> > correct way to handle this case. Specifically, you'd want:\n> >\n> >     test_must_fail env FAKE_LINES=\"1 2 3 5 6 7 8 9 10 11\" \\\n> >         git rebase -ir main &&\n>\n> To clarify, by \"pollute\", I mean that it can impact subsequent tests\n> which don't take care to override FAKE_LINES as necessary. There\n> certainly are test scripts which use the:\n>\n>     FAKE_LINES=... &&\n>     export FAKE_LINES &&\n>\n> form successfully, but such scripts are careful to override/set\n> FAKE_LINES in every test. This particular script (t3418), on the other\n> hand, does not otherwise employ the form in which the variable is\n> exported, so introducing it in a test which is inserted into the\n> middle of the script feels dangerous.\n\nThe entire `FAKE_LINES` paradigm is broken, and since I suspect that it\nwas me who introduced it, I apologize.\n\nA much better way to have done this would have been to write the string to\na certain file, say, $(git rev-parse --git-path sequencer.pick-lines), and\nin the `fake-editor.sh`:\n\n- test for the existence of that file, and if it exists\n  - use its contents\n  - delete that file\n\nCiao,\nJohannes\n"},{"id":"515446","messageId":"0bd7e0c1-fe73-9e16-0737-d6b175a60dd3@gmx.de","threadId":"63211","inReplyTo":"db01acdd062a17b1cca62428eba8c3ed62ca7c6a.1743181401.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-04-01T16:22:24Z","receivedAt":"2025-04-01T16:22:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philippe,\n\nOn Fri, 28 Mar 2025, Philippe Blain via GitGitGadget wrote:\n\n> From: Philippe Blain <levraiphilippeblain@gmail.com>\n>\n> Since 982288e9bd (status: rebase and merge can be in progress at the\n> same time, 2018-11-12), when a merge is in progress as part of a 'git\n> rebase -r' operation, 'wt_longstatus_print_state' shows information\n> about the in-progress rebase (via show_rebase_information), and then\n> calls 'show_merge_in_progress' to help the user conclude the merge. This\n> function suggests using 'git commit' to do so, but this throws away the\n> authorship information from the original merge, which is not ideal.\n\nIt is unfortunate that we cannot fix this, as `git commit` with an\ninterrupted `pick` _would_ retain authorship, right? (Why is that so? Can\nwe really not use the same trick with `merge`s?)\n\nCiao,\nJohannes\n"},{"id":"515518","messageId":"a81dbb21-b50b-4358-b2d4-7f804b66bcbc@gmail.com","threadId":"63211","inReplyTo":"0bd7e0c1-fe73-9e16-0737-d6b175a60dd3@gmx.de","subject":"Re: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-04-02T13:09:22Z","receivedAt":"2025-04-02T13:09:30Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 01/04/2025 17:22, Johannes Schindelin wrote:\n> Hi Philippe,\n> \n> On Fri, 28 Mar 2025, Philippe Blain via GitGitGadget wrote:\n> \n>> From: Philippe Blain <levraiphilippeblain@gmail.com>\n>>\n>> Since 982288e9bd (status: rebase and merge can be in progress at the\n>> same time, 2018-11-12), when a merge is in progress as part of a 'git\n>> rebase -r' operation, 'wt_longstatus_print_state' shows information\n>> about the in-progress rebase (via show_rebase_information), and then\n>> calls 'show_merge_in_progress' to help the user conclude the merge. This\n>> function suggests using 'git commit' to do so, but this throws away the\n>> authorship information from the original merge, which is not ideal.\n> \n> It is unfortunate that we cannot fix this, as `git commit` with an\n> interrupted `pick` _would_ retain authorship, right?\n\nUnfortunately not. Running \"git commit\" rather than \"git rebase \n--continue\" to commit a conflict resolution when rebasing always loses \nthe authorship.\n\nBest Wishes\n\nPhillip\n\n  (Why is that so? Can\n> we really not use the same trick with `merge`s?)\n> \n> Ciao,\n> Johannes\n\n"},{"id":"515582","messageId":"15222e69-9452-fd61-6ffc-8c8de0c68d8a@gmx.de","threadId":"63211","inReplyTo":"a81dbb21-b50b-4358-b2d4-7f804b66bcbc@gmail.com","subject":"Re: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-04-03T12:17:57Z","receivedAt":"2025-04-03T12:18:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Wed, 2 Apr 2025, phillip.wood123@gmail.com wrote:\n\n> On 01/04/2025 17:22, Johannes Schindelin wrote:\n>\n> > On Fri, 28 Mar 2025, Philippe Blain via GitGitGadget wrote:\n> >\n> > > From: Philippe Blain <levraiphilippeblain@gmail.com>\n> > >\n> > > Since 982288e9bd (status: rebase and merge can be in progress at the\n> > > same time, 2018-11-12), when a merge is in progress as part of a\n> > > 'git rebase -r' operation, 'wt_longstatus_print_state' shows\n> > > information about the in-progress rebase (via\n> > > show_rebase_information), and then calls 'show_merge_in_progress' to\n> > > help the user conclude the merge. This function suggests using 'git\n> > > commit' to do so, but this throws away the authorship information\n> > > from the original merge, which is not ideal.\n> >\n> > It is unfortunate that we cannot fix this, as `git commit` with an\n> > interrupted `pick` _would_ retain authorship, right?\n>\n> Unfortunately not. Running \"git commit\" rather than \"git rebase\n> --continue\" to commit a conflict resolution when rebasing always loses\n> the authorship.\n>\n> > (Why is that so? Can we really not use the same trick with `merge`s?)\n\nAuthorship is retained when a `git cherry-pick` (what an unwieldy command\nname for _such_ a common operation!) failed with merge conflicts and those\nconflicts were resolved and the user then calls `git commit`, though.\n\nWhy can this technique not be used in interrupted `pick`/`merge` commands\nof `git rebase`?\n\nCiao,\nJohannes\n"},{"id":"515589","messageId":"08837a1a-b46d-4456-beba-5c889fe9e674@gmail.com","threadId":"63211","inReplyTo":"15222e69-9452-fd61-6ffc-8c8de0c68d8a@gmx.de","subject":"Re: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-04-03T15:08:22Z","receivedAt":"2025-04-03T15:08:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 03/04/2025 13:17, Johannes Schindelin wrote:\n> Hi Phillip,\n> On Wed, 2 Apr 2025, phillip.wood123@gmail.com wrote:\n>> On 01/04/2025 17:22, Johannes Schindelin wrote:\n>>\n>>> It is unfortunate that we cannot fix this, as `git commit` with an\n>>> interrupted `pick` _would_ retain authorship, right?\n>>\n>> Unfortunately not. Running \"git commit\" rather than \"git rebase\n>> --continue\" to commit a conflict resolution when rebasing always loses\n>> the authorship.\n>>\n>>> (Why is that so? Can we really not use the same trick with `merge`s?)\n> \n> Authorship is retained when a `git cherry-pick` (what an unwieldy command\n> name for _such_ a common operation!) failed with merge conflicts and those\n> conflicts were resolved and the user then calls `git commit`, though.\n> \n> Why can this technique not be used in interrupted `pick`/`merge` commands\n> of `git rebase`?`git cherry-pick` retains authorship by writing CHERRY_PICK_HEAD which \n`git commit` uses to look up the commit message and authorship. When \nwe're rebasing the sequencer removes CHERRY_PICK_HEAD and instead writes \nthe commit message to MERGE_MSG and the authorship to \n.git/rebase-merge/author-script. I think the reason for the different \nbehavior is to avoid confusing things like `git status`. \nCHERRY_PICK_HEAD has been removed when rebasing since it was introduced \nin d7e5c0cbfb0 (Introduce CHERRY_PICK_HEAD, 2011-02-19). These days \nrebase supports --reset-author-date which means it cannot use the same \nmechanism as cherry-pick. Personally I'd much rather we tell people to \nuse \"git rebase --continue\" to commit their conflict resolutions as \nusing \"git commit\" has never worked if one wanted to preserve authorship \nand I think making it work would be a pain and probably fragile as I'm \nnot sure how we'd ensure \"git commit\" knew it was committing a conflict \nresolution created by \"git rebase\" rather than one created by some other \ncommit run while the rebase was stopped or by an exec command.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"515659","messageId":"c2f93d99-2f4d-ee6d-7087-42320c6df0f2@gmx.de","threadId":"63211","inReplyTo":"08837a1a-b46d-4456-beba-5c889fe9e674@gmail.com","subject":"Re: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2025-04-04T11:41:07Z","receivedAt":"2025-04-04T11:41:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Thu, 3 Apr 2025, phillip.wood123@gmail.com wrote:\n\n> On 03/04/2025 13:17, Johannes Schindelin wrote:\n>\n> > On Wed, 2 Apr 2025, phillip.wood123@gmail.com wrote:\n> > > On 01/04/2025 17:22, Johannes Schindelin wrote:\n> > >\n> > > > It is unfortunate that we cannot fix this, as `git commit` with an\n> > > > interrupted `pick` _would_ retain authorship, right?\n> > >\n> > > Unfortunately not. Running \"git commit\" rather than \"git rebase\n> > > --continue\" to commit a conflict resolution when rebasing always\n> > > loses the authorship.\n> > >\n> > > > (Why is that so? Can we really not use the same trick with `merge`s?)\n> >\n> > Authorship is retained when a `git cherry-pick` (what an unwieldy command\n> > name for _such_ a common operation!) failed with merge conflicts and those\n> > conflicts were resolved and the user then calls `git commit`, though.\n> >\n> > Why can this technique not be used in interrupted `pick`/`merge` commands\n> > of `git rebase`?\n\n[Fixed totally garbled formatting that pretended that the first half of\nthis sentence was written by me, the second half by you:]\n\n> `git cherry-pick` retains authorship by writing CHERRY_PICK_HEAD which\n> `git commit` uses to look up the commit message and authorship.\n\nAnd why can we not teach `git commit` to use the author information\nrecorded in `.git/rebase-merge/author-script`, too, and teach `git reset\n--hard` to delete it?\n\n> When we're rebasing the sequencer removes CHERRY_PICK_HEAD and instead\n> writes the commit message to MERGE_MSG and the authorship to\n> .git/rebase-merge/author-script. I think the reason for the different\n> behavior is to avoid confusing things like `git status`.\n\nThe reason is probably more that you can mix `git rebase` and `git\ncherry-pick` (why does this common operation have such a long name,\nagain?). I actually do this quite often, I frequently even have something\nlike this in my rebase scripts:\n\n\texec git cherry-pick ..upstream/seen^{xx/something-something}^2\n\n> CHERRY_PICK_HEAD has been removed when rebasing since it was\n> introduced in d7e5c0cbfb0 (Introduce CHERRY_PICK_HEAD, 2011-02-19). These days\n> rebase supports --reset-author-date which means it cannot use the same\n> mechanism as cherry-pick.\n\nRight. But it can recapitulate cherry-pick's strategy in spirit. After\nall, `git commit` had to be taught about an interrupted `git cherry-pick`\nso that it _could_ pick up the necessary information and use that.\nLikewise, `git commit` could be taught about an interrupted `git rebase`\nand similarly pick up the author information from what `git rebase`\nrecorded.\n\n> Personally I'd much rather we tell people to use \"git rebase --continue\"\n> to commit their conflict resolutions as using \"git commit\" has never\n> worked if one wanted to preserve authorship and I think making it work\n> would be a pain and probably fragile as I'm not sure how we'd ensure\n> \"git commit\" knew it was committing a conflict resolution created by\n> \"git rebase\" rather than one created by some other commit run while the\n> rebase was stopped or by an exec command.\n\nEven I, the inventor of `git rebase -i`, have run afoul of this authorship\nresetting on more than a dozen occasions.\n\nThis is proof enough for me that Git is unnecessarily confusing (no big\nrevelation there, right? Git earned that reputation very effortlessly, not\nonly in this particular scenario).\n\nI'd rather like this usability problem to be fixed, even if it is a pain.\nIf the pain stems from the way the source code is organized, well, then\nmaybe this hints at the need to clean up a little?\n\nCiao,\nJohannes\n"},{"id":"515669","messageId":"8fd9d4d0-93e1-4a88-a1ed-1d84b2150893@gmail.com","threadId":"63211","inReplyTo":"c2f93d99-2f4d-ee6d-7087-42320c6df0f2@gmx.de","subject":"Re: [PATCH 3/3] wt-status: suggest 'git rebase --continue' to conclude 'merge' instruction","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-04-04T14:13:40Z","receivedAt":"2025-04-04T14:13:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Johannes\n\nOn 04/04/2025 12:41, Johannes Schindelin wrote:\n> On Thu, 3 Apr 2025, phillip.wood123@gmail.com wrote:\n>> On 03/04/2025 13:17, Johannes Schindelin wrote:\n>>> On Wed, 2 Apr 2025, phillip.wood123@gmail.com wrote:\n>>>> On 01/04/2025 17:22, Johannes Schindelin wrote:\n>>>>\n>>>>> It is unfortunate that we cannot fix this, as `git commit` with an\n>>>>> interrupted `pick` _would_ retain authorship, right?\n>>>>\n>>>> Unfortunately not. Running \"git commit\" rather than \"git rebase\n>>>> --continue\" to commit a conflict resolution when rebasing always\n>>>> loses the authorship.\n>>>>\n>>>>> (Why is that so? Can we really not use the same trick with `merge`s?)\n>>>\n>>> Authorship is retained when a `git cherry-pick` (what an unwieldy command\n>>> name for _such_ a common operation!) failed with merge conflicts and those\n>>> conflicts were resolved and the user then calls `git commit`, though.\n>>>\n>>> Why can this technique not be used in interrupted `pick`/`merge` commands\n>>> of `git rebase`?\n> \n> [Fixed totally garbled formatting that pretended that the first half of\n> this sentence was written by me, the second half by you:]\n\nSorry I'm not sure what happened there\n\n>> `git cherry-pick` retains authorship by writing CHERRY_PICK_HEAD which\n>> `git commit` uses to look up the commit message and authorship.\n> \n> And why can we not teach `git commit` to use the author information\n> recorded in `.git/rebase-merge/author-script`, too, and teach `git reset\n> --hard` to delete it?\n\nIf the user passes \"--committer-date-is-author-date\" then we write the \nauthor-script when stopping for an unconflicted edit command. However if \nthe user runs \"git commit\" rather than \"git commit --amend\" we do not \nwant to use that script because they are creating a new commit. That \nmeans that \"git commit\" cannot simply use the author-script if it \nexists. I expect we could read all of the rebase state to figure out \nwhat to do but I think it is a much simpler UI to say that the user \nshould run \"git rebase --continue\" unless the user is creating a new \ncommit. Otherwise in a world where \"git commit\" knows about the author \nscript the user has to figure out whether or not they need to pass \n\"--amend\" when running \"git commit\". If they're committing a conflict \nresolution for a normal pick they should run \"git commit\". However if \nthey are committing a conflict resolution for a fixup then they need to \nadd \"--amend\". If \"git commit\" starts deciding whether to amend or not \nto avoid the user having to remember that is even more confusing because \nit is behaving differently during a rebase compared to any other time.\n\n>> When we're rebasing the sequencer removes CHERRY_PICK_HEAD and instead\n>> writes the commit message to MERGE_MSG and the authorship to\n>> .git/rebase-merge/author-script. I think the reason for the different\n>> behavior is to avoid confusing things like `git status`.\n> \n> The reason is probably more that you can mix `git rebase` and `git\n> cherry-pick` (why does this common operation have such a long name,\n> again?). I actually do this quite often, I frequently even have something\n> like this in my rebase scripts:\n> \n> \texec git cherry-pick ..upstream/seen^{xx/something-something}^2\n> \n>> CHERRY_PICK_HEAD has been removed when rebasing since it was\n>> introduced in d7e5c0cbfb0 (Introduce CHERRY_PICK_HEAD, 2011-02-19). These days\n>> rebase supports --reset-author-date which means it cannot use the same\n>> mechanism as cherry-pick.\n> \n> Right. But it can recapitulate cherry-pick's strategy in spirit. After\n> all, `git commit` had to be taught about an interrupted `git cherry-pick`\n> so that it _could_ pick up the necessary information and use that.\n> Likewise, `git commit` could be taught about an interrupted `git rebase`\n> and similarly pick up the author information from what `git rebase`\n> recorded.\n> \n>> Personally I'd much rather we tell people to use \"git rebase --continue\"\n>> to commit their conflict resolutions as using \"git commit\" has never\n>> worked if one wanted to preserve authorship and I think making it work\n>> would be a pain and probably fragile as I'm not sure how we'd ensure\n>> \"git commit\" knew it was committing a conflict resolution created by\n>> \"git rebase\" rather than one created by some other commit run while the\n>> rebase was stopped or by an exec command.\n> \n> Even I, the inventor of `git rebase -i`, have run afoul of this authorship\n> resetting on more than a dozen occasions.\n> \n> This is proof enough for me that Git is unnecessarily confusing (no big\n> revelation there, right? Git earned that reputation very effortlessly, not\n> only in this particular scenario).\n\nI think it's confusing because \"git commit\" tries to do too much and \nthat it was a mistake to allow merge conflicts to be committed by \"git \ncommit\" rather than \"git <cmd> --continue\". I believe the reason \"git \ncommit\" allows conflict resolutions to be committed is historical and \nthat \"git merge --continue\" was a later addition. Originally \"git \ncommit\" was the only way to conclude a conflicted merge. Arguably that's \nnot too bad for a merge or a single cherry-pick but I'd argue it would \nbe much less confusing if \"git commit\" refused to run when it looked \nlike the user was committing a conflict resolution and told them to run \n\"git <cmd> --continue\" instead.\n\n> I'd rather like this usability problem to be fixed, even if it is a pain.\n> If the pain stems from the way the source code is organized, well, then\n> maybe this hints at the need to clean up a little?\nThe sequencer could certainly use a clean up but I fear it would be a \nhuge time sink for both the patch author and the reviewer.\n\nBest Wishes\n\nPhillip\n"}]}