{"thread":{"id":"36877","subject":"[PATCH] rebase -i: Remember merge options beyond continue actions","startedAt":"2014-06-10T00:02:58Z","lastAt":"2015-12-11T20:30:36Z","messageCount":8,"participants":["Fabian Ruch","Eric Sunshine","Michael Haggerty","Ralf Thielow","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"243690","messageId":"53964B32.2090804@gmail.com","threadId":"36877","inReplyTo":null,"subject":"[PATCH] rebase -i: Remember merge options beyond continue actions","fromName":"Fabian Ruch","fromEmail":"bafain@gmail.com","sentAt":"2014-06-10T00:02:58Z","receivedAt":"2014-06-10T00:02:58Z","isPatch":true,"sender":{"key":"bafain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1150972?v=4"},"body":"If the user explicitly specified a merge strategy or strategy options,\n\"rebase --interactive\" started using the default merge strategy again\nafter \"rebase --continue\".\n\nThis problem gets fixed by this commit. Add test.\n\nSince the \"rebase\" options \"-s\" and \"-X\" imply \"--merge\", we can simply\nremove the \"do_merge\" guard in the interactive mode and always compile\nthe \"cherry-pick\" arguments from the \"rebase\" state variables \"strategy\"\nand \"strategy_opts\".\n---\n git-rebase--interactive.sh | 18 +++++++-----------\n t/t3404-rebase-interactive.sh | 16 ++++++++++++++++\n 2 files changed, 23 insertions(+), 11 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 6ec9d3c..817c933 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -77,17 +77,13 @@ amend=\"$state_dir\"/amend\n rewritten_list=\"$state_dir\"/rewritten-list\n rewritten_pending=\"$state_dir\"/rewritten-pending\n \n-strategy_args=\n-if test -n \"$do_merge\"\n-then\n-\tstrategy_args=${strategy:+--strategy=$strategy}\n-\teval '\n-\t\tfor strategy_opt in '\"$strategy_opts\"'\n-\t\tdo\n-\t\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n-\t\tdone\n-\t'\n-fi\n+strategy_args=${strategy:+--strategy=$strategy}\n+eval '\n+\tfor strategy_opt in '\"$strategy_opts\"'\n+\tdo\n+\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n+\tdone\n+'\n \n GIT_CHERRY_PICK_HELP=\"$resolvemsg\"\n export GIT_CHERRY_PICK_HELP\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex c0023a5..73849f1 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '\n \ttest $(cat file1) = Z\n '\n \n+test_expect_success 'interrupted rebase -i with --strategy and -X' '\n+\tgit checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&\n+\tgit reset --hard HEAD^ &&\n+\t>breakpoint &&\n+\tgit add breakpoint &&\n+\tgit commit -m \"breakpoint for interactive mode\" &&\n+\techo five >conflict &&\n+\techo Z >file1 &&\n+\tgit commit -a -m \"one file conflict\" &&\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"edit 1 2\" git rebase -i --strategy=recursive -Xours conflict-branch &&\n+\tgit rebase --continue &&\n+\ttest $(git show conflict-branch:conflict) = $(cat conflict) &&\n+\ttest $(cat file1) = Z\n+'\n+\n test_expect_success 'rebase -i error on commits with \\ in message' '\n \tcurrent_head=$(git rev-parse HEAD)\n \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n-- \n2.0.0\n"},{"id":"243691","messageId":"CAPig+cSHFFPUEQz8==HLQr0My2Bfsth_F16wVf9giytqGwzZww@mail.gmail.com","threadId":"36877","inReplyTo":"53964B32.2090804@gmail.com","subject":"Re: [PATCH] rebase -i: Remember merge options beyond continue actions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-06-10T00:17:41Z","receivedAt":"2014-06-10T00:17:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 9, 2014 at 8:02 PM, Fabian Ruch wrote:\n> If the user explicitly specified a merge strategy or strategy options,\n> \"rebase --interactive\" started using the default merge strategy again\n> after \"rebase --continue\".\n\nFor reference, this problem was reported as far back as 2013-08-09 [1].\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/232013\n\n> This problem gets fixed by this commit. Add test.\n>\n> Since the \"rebase\" options \"-s\" and \"-X\" imply \"--merge\", we can simply\n> remove the \"do_merge\" guard in the interactive mode and always compile\n> the \"cherry-pick\" arguments from the \"rebase\" state variables \"strategy\"\n> and \"strategy_opts\".\n\nMissing sign-off.\n\n> ---\n> git-rebase--interactive.sh | 18 +++++++-----------\n> t/t3404-rebase-interactive.sh | 16 ++++++++++++++++\n> 2 files changed, 23 insertions(+), 11 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 6ec9d3c..817c933 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -77,17 +77,13 @@ amend=\"$state_dir\"/amend\n> rewritten_list=\"$state_dir\"/rewritten-list\n> rewritten_pending=\"$state_dir\"/rewritten-pending\n>\n> -strategy_args=\n> -if test -n \"$do_merge\"\n> -then\n> - strategy_args=${strategy:+--strategy=$strategy}\n> - eval '\n> - for strategy_opt in '\"$strategy_opts\"'\n> - do\n> - strategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n> - done\n> - '\n> -fi\n> +strategy_args=${strategy:+--strategy=$strategy}\n> +eval '\n> + for strategy_opt in '\"$strategy_opts\"'\n> + do\n> + strategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n> + done\n> +'\n>\n> GIT_CHERRY_PICK_HELP=\"$resolvemsg\"\n> export GIT_CHERRY_PICK_HELP\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index c0023a5..73849f1 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '\n> test $(cat file1) = Z\n> '\n>\n> +test_expect_success 'interrupted rebase -i with --strategy and -X' '\n> + git checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&\n> + git reset --hard HEAD^ &&\n> + >breakpoint &&\n> + git add breakpoint &&\n> + git commit -m \"breakpoint for interactive mode\" &&\n> + echo five >conflict &&\n> + echo Z >file1 &&\n> + git commit -a -m \"one file conflict\" &&\n> + set_fake_editor &&\n> + FAKE_LINES=\"edit 1 2\" git rebase -i --strategy=recursive -Xours conflict-branch &&\n> + git rebase --continue &&\n> + test $(git show conflict-branch:conflict) = $(cat conflict) &&\n> + test $(cat file1) = Z\n> +'\n> +\n> test_expect_success 'rebase -i error on commits with \\ in message' '\n> current_head=$(git rev-parse HEAD)\n> test_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n> --\n> 2.0.0\n"},{"id":"243692","messageId":"53965334.3030206@gmail.com","threadId":"36877","inReplyTo":"CAPig+cSHFFPUEQz8==HLQr0My2Bfsth_F16wVf9giytqGwzZww@mail.gmail.com","subject":"Re: [PATCH] rebase -i: Remember merge options beyond continue actions","fromName":"Fabian Ruch","fromEmail":"bafain@gmail.com","sentAt":"2014-06-10T00:37:08Z","receivedAt":"2014-06-10T00:37:08Z","isPatch":true,"sender":{"key":"bafain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1150972?v=4"},"body":"Hi Eric,\n\nthanks a lot for the reference.\n\nI added the Reported-by: and Signed-off-by: lines to the commit message.\n\n Fabian\n\n-- >8 --\nSubject: rebase -i: Remember merge options beyond continue actions\n\nIf the user explicitly specified a merge strategy or strategy options,\n\"rebase --interactive\" started using the default merge strategy again\nafter \"rebase --continue\".\n\nThis problem gets fixed by this commit. Add test.\n\nSince the \"rebase\" options \"-s\" and \"-X\" imply \"--merge\", we can simply\nremove the \"do_merge\" guard in the interactive mode and always compile\nthe \"cherry-pick\" arguments from the \"rebase\" state variables \"strategy\"\nand \"strategy_opts\".\n\nReported-by: Diogo de Campos \nSigned-off-by: Fabian Ruch \n---\n git-rebase--interactive.sh | 18 +++++++-----------\n t/t3404-rebase-interactive.sh | 16 ++++++++++++++++\n 2 files changed, 23 insertions(+), 11 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 6ec9d3c..817c933 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -77,17 +77,13 @@ amend=\"$state_dir\"/amend\n rewritten_list=\"$state_dir\"/rewritten-list\n rewritten_pending=\"$state_dir\"/rewritten-pending\n \n-strategy_args=\n-if test -n \"$do_merge\"\n-then\n-\tstrategy_args=${strategy:+--strategy=$strategy}\n-\teval '\n-\t\tfor strategy_opt in '\"$strategy_opts\"'\n-\t\tdo\n-\t\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n-\t\tdone\n-\t'\n-fi\n+strategy_args=${strategy:+--strategy=$strategy}\n+eval '\n+\tfor strategy_opt in '\"$strategy_opts\"'\n+\tdo\n+\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n+\tdone\n+'\n \n GIT_CHERRY_PICK_HELP=\"$resolvemsg\"\n export GIT_CHERRY_PICK_HELP\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex c0023a5..73849f1 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '\n \ttest $(cat file1) = Z\n '\n \n+test_expect_success 'interrupted rebase -i with --strategy and -X' '\n+\tgit checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&\n+\tgit reset --hard HEAD^ &&\n+\t>breakpoint &&\n+\tgit add breakpoint &&\n+\tgit commit -m \"breakpoint for interactive mode\" &&\n+\techo five >conflict &&\n+\techo Z >file1 &&\n+\tgit commit -a -m \"one file conflict\" &&\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"edit 1 2\" git rebase -i --strategy=recursive -Xours conflict-branch &&\n+\tgit rebase --continue &&\n+\ttest $(git show conflict-branch:conflict) = $(cat conflict) &&\n+\ttest $(cat file1) = Z\n+'\n+\n test_expect_success 'rebase -i error on commits with \\ in message' '\n \tcurrent_head=$(git rev-parse HEAD)\n \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n-- \n2.0.0\n"},{"id":"243926","messageId":"5398C686.8050400@alum.mit.edu","threadId":"36877","inReplyTo":"53965334.3030206@gmail.com","subject":"Re: [PATCH] rebase -i: Remember merge options beyond continue actions","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-06-11T21:13:42Z","receivedAt":"2014-06-11T21:13:42Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The fix seems reasonable to me. See below for a couple of suggestions\nregarding the commit message.\n\nOn 06/10/2014 02:37 AM, Fabian Ruch wrote:\n> [...]\n> -- >8 --\n> Subject: rebase -i: Remember merge options beyond continue actions\n> \n> If the user explicitly specified a merge strategy or strategy options,\n> \"rebase --interactive\" started using the default merge strategy again\n> after \"rebase --continue\".\n> \n> This problem gets fixed by this commit. Add test.\n\nPlease phrase commit messages in the imperative voice, as if commanding\ngit to fix itself. Maybe\n\n If the user explicitly specified a merge strategy or strategy\n options, continue to use that strategy/option after\n \"rebase --continue\". Add a test of the corrected behavior.\n\n> Since the \"rebase\" options \"-s\" and \"-X\" imply \"--merge\", we can simply\n> remove the \"do_merge\" guard in the interactive mode and always compile\n> the \"cherry-pick\" arguments from the \"rebase\" state variables \"strategy\"\n> and \"strategy_opts\".\n> \n> Reported-by: Diogo de Campos \n> Signed-off-by: Fabian Ruch \n\nI expect it took you a while to figure out how the strategy-related\noptions are handled and propagated from one step of an interactive\nrebase to the next and why your fix is correct. It certainly took *me*\na while even though I had your patch in front of me :-)\n\nIt is helpful if you give reviewers and future readers the benefit of\nyour hard work by spoon-feeding them more of the backstory. I suggest\nexpanding your justification to something like this (correct me if I've\nmisunderstood something!):\n\n If --merge is specified or implied by -s or -X, then \"strategy\" and\n \"strategy_opts\" are set to values from which \"strategy_args\" can be\n derived; otherwise they are set to empty strings. Either way,\n their values are propagated from one step of an interactive rebase\n to the next via state files.\n\n \"do_merge\", on the other hand, is *not* propagated to later steps of\n an interactive rebase. Therefore, making the initialization of\n \"strategy_args\" conditional on \"do_merge\" being set prevents later\n steps of an interactive rebase from setting it correctly.\n\n Luckily, we don't need the \"do_merge\" guard at all. If the rebase\n was started without --merge, then \"strategy\" and \"strategy_opts\"\n are both the empty string, which results in \"strategy_args\" also\n being set to the empty string, which is just what we want in that\n situation. So remove the \"do_merge\" guard and derive\n \"strategy_args\" from \"strategy\" and \"strategy_opts\" every time.\n\nMichael\n\n> ---\n> git-rebase--interactive.sh | 18 +++++++-----------\n> t/t3404-rebase-interactive.sh | 16 ++++++++++++++++\n> 2 files changed, 23 insertions(+), 11 deletions(-)\n> \n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 6ec9d3c..817c933 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -77,17 +77,13 @@ amend=\"$state_dir\"/amend\n> rewritten_list=\"$state_dir\"/rewritten-list\n> rewritten_pending=\"$state_dir\"/rewritten-pending\n> \n> -strategy_args=\n> -if test -n \"$do_merge\"\n> -then\n> -\tstrategy_args=${strategy:+--strategy=$strategy}\n> -\teval '\n> -\t\tfor strategy_opt in '\"$strategy_opts\"'\n> -\t\tdo\n> -\t\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n> -\t\tdone\n> -\t'\n> -fi\n> +strategy_args=${strategy:+--strategy=$strategy}\n> +eval '\n> +\tfor strategy_opt in '\"$strategy_opts\"'\n> +\tdo\n> +\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n> +\tdone\n> +'\n> \n> GIT_CHERRY_PICK_HELP=\"$resolvemsg\"\n> export GIT_CHERRY_PICK_HELP\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index c0023a5..73849f1 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -998,6 +998,22 @@ test_expect_success 'rebase -i with --strategy and -X' '\n> \ttest $(cat file1) = Z\n> '\n> \n> +test_expect_success 'interrupted rebase -i with --strategy and -X' '\n> +\tgit checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&\n> +\tgit reset --hard HEAD^ &&\n> +\t>breakpoint &&\n> +\tgit add breakpoint &&\n> +\tgit commit -m \"breakpoint for interactive mode\" &&\n> +\techo five >conflict &&\n> +\techo Z >file1 &&\n> +\tgit commit -a -m \"one file conflict\" &&\n> +\tset_fake_editor &&\n> +\tFAKE_LINES=\"edit 1 2\" git rebase -i --strategy=recursive -Xours conflict-branch &&\n> +\tgit rebase --continue &&\n> +\ttest $(git show conflict-branch:conflict) = $(cat conflict) &&\n> +\ttest $(cat file1) = Z\n> +'\n> +\n> test_expect_success 'rebase -i error on commits with \\ in message' '\n> \tcurrent_head=$(git rev-parse HEAD)\n> \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n> \n\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"246319","messageId":"1405703017-15934-1-git-send-email-ralf.thielow@gmail.com","threadId":"36877","inReplyTo":"53965334.3030206@gmail.com","subject":"Re: [PATCH] rebase -i: Remember merge options beyond continue actions","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2014-07-18T17:03:37Z","receivedAt":"2014-07-18T17:03:37Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"Hi,\n\nThanks for the patch. I've had this issue today\nand the patch has fixed it. I hope the patch makes\nits way to Git.\n\nRalf\n\n> Hi Eric,\n> \n> thanks a lot for the reference.\n> \n> I added the Reported-by: and Signed-off-by: lines to the commit message.\n> \n> Fabian\n> \n> -- >8 --\n> Subject: rebase -i: Remember merge options beyond continue actions\n> \n> If the user explicitly specified a merge strategy or strategy options,\n> \"rebase --interactive\" started using the default merge strategy again\n> after \"rebase --continue\".\n> \n> This problem gets fixed by this commit. Add test.\n> \n> Since the \"rebase\" options \"-s\" and \"-X\" imply \"--merge\", we can simply\n> remove the \"do_merge\" guard in the interactive mode and always compile\n> the \"cherry-pick\" arguments from the \"rebase\" state variables \"strategy\"\n> and \"strategy_opts\".\n...\n"},{"id":"274290","messageId":"1449863646-26067-1-git-send-email-ralf.thielow@gmail.com","threadId":"36877","inReplyTo":"53965334.3030206@gmail.com","subject":"[PATCH] rebase -i: remember merge options beyond continue actions","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2015-12-11T19:54:06Z","receivedAt":"2015-12-11T19:54:06Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"From: Fabian Ruch \n\nIf the user specifies a merge strategy or strategy options in\n\"rebase --interactive\", also use these in subsequent calls of\n\"rebase --continue\".\n\nIn the implementation, the \"do_merge\" guard to check for a given\nmerge strategy or strategy options is implied by passing these\noptions, but not stored. This prevents subsequent calls of\n\"rebase --continue\" to use this setting again later. Remove this\n\"do_merge\" guard to allow a later usage.\n\nReported-by: Diogo de Campos \nSigned-off-by: Fabian Ruch \nHelped-by: Michael Haggerty \nSigned-off-by: Ralf Thielow \n---\nI've been applying the original patch for a long time to my tree,\nas it helps me to resolve conflicts e.g. when rebasing .po files.\nI also think it's a reasonable change for git-rebase.\n\nI've rebased it agains the current master and rewrote the\ncommit message to try to make this change technically a bit more\neasy to understand.\n\nOriginal patch submit:\nhttp://thread.gmane.org/gmane.comp.version-control.git/251147/\n\n git-rebase--interactive.sh | 18 +++++++-----------\n t/t3404-rebase-interactive.sh | 16 ++++++++++++++++\n 2 files changed, 23 insertions(+), 11 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex b938a6d..c0cfe88 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -81,17 +81,13 @@ rewritten_pending=\"$state_dir\"/rewritten-pending\n # and leaves CR at the end instead.\n cr=$(printf \"\\015\")\n \n-strategy_args=\n-if test -n \"$do_merge\"\n-then\n-\tstrategy_args=${strategy:+--strategy=$strategy}\n-\teval '\n-\t\tfor strategy_opt in '\"$strategy_opts\"'\n-\t\tdo\n-\t\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n-\t\tdone\n-\t'\n-fi\n+strategy_args=${strategy:+--strategy=$strategy}\n+eval '\n+\tfor strategy_opt in '\"$strategy_opts\"'\n+\tdo\n+\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n+\tdone\n+'\n \n GIT_CHERRY_PICK_HELP=\"$resolvemsg\"\n export GIT_CHERRY_PICK_HELP\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 98eb49a..9a2461c 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1006,6 +1006,22 @@ test_expect_success 'rebase -i with --strategy and -X' '\n \ttest $(cat file1) = Z\n '\n \n+test_expect_success 'interrupted rebase -i with --strategy and -X' '\n+\tgit checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&\n+\tgit reset --hard HEAD^ &&\n+\t>breakpoint &&\n+\tgit add breakpoint &&\n+\tgit commit -m \"breakpoint for interactive mode\" &&\n+\techo five >conflict &&\n+\techo Z >file1 &&\n+\tgit commit -a -m \"one file conflict\" &&\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"edit 1 2\" git rebase -i --strategy=recursive -Xours conflict-branch &&\n+\tgit rebase --continue &&\n+\ttest $(git show conflict-branch:conflict) = $(cat conflict) &&\n+\ttest $(cat file1) = Z\n+'\n+\n test_expect_success 'rebase -i error on commits with \\ in message' '\n \tcurrent_head=$(git rev-parse HEAD) &&\n \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n-- \n2.7.0.rc0.174.g1b62464\n"},{"id":"274292","messageId":"CAPc5daV_tPh9pt4YSpsBSCvrvGOqC7+9eTZkS1bV2ZAE2YoxzA@mail.gmail.com","threadId":"36877","inReplyTo":"1449863646-26067-1-git-send-email-ralf.thielow@gmail.com","subject":"Re: [PATCH] rebase -i: remember merge options beyond continue actions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-11T20:07:18Z","receivedAt":"2015-12-11T20:07:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Fri, Dec 11, 2015 at 11:54 AM, Ralf Thielow wrote:\n> From: Fabian Ruch \n>\n> If the user specifies a merge strategy or strategy options in\n> \"rebase --interactive\", also use these in subsequent calls of\n> \"rebase --continue\".\n>\n> In the implementation, the \"do_merge\" guard to check for a given\n> merge strategy or strategy options is implied by passing these\n> options, but not stored. This prevents subsequent calls of\n> \"rebase --continue\" to use this setting again later. Remove this\n> \"do_merge\" guard to allow a later usage.\n>\n> Reported-by: Diogo de Campos \n> Signed-off-by: Fabian Ruch \n> Helped-by: Michael Haggerty \n> Signed-off-by: Ralf Thielow \n> ---\n> I've been applying the original patch for a long time to my tree,\n> as it helps me to resolve conflicts e.g. when rebasing .po files.\n> I also think it's a reasonable change for git-rebase.\n>\n> I've rebased it agains the current master and rewrote the\n> commit message to try to make this change technically a bit more\n> easy to understand.\n\nThanks. I wonder if Michael's rephrasing in $gmane/251386 still applies, which\nI found by far the most readable.\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/251147/focus=251386\n\n\n\n>\n> Original patch submit:\n> http://thread.gmane.org/gmane.comp.version-control.git/251147/\n>\n> git-rebase--interactive.sh | 18 +++++++-----------\n> t/t3404-rebase-interactive.sh | 16 ++++++++++++++++\n> 2 files changed, 23 insertions(+), 11 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index b938a6d..c0cfe88 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -81,17 +81,13 @@ rewritten_pending=\"$state_dir\"/rewritten-pending\n> # and leaves CR at the end instead.\n> cr=$(printf \"\\015\")\n>\n> -strategy_args=\n> -if test -n \"$do_merge\"\n> -then\n> - strategy_args=${strategy:+--strategy=$strategy}\n> - eval '\n> - for strategy_opt in '\"$strategy_opts\"'\n> - do\n> - strategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n> - done\n> - '\n> -fi\n> +strategy_args=${strategy:+--strategy=$strategy}\n> +eval '\n> + for strategy_opt in '\"$strategy_opts\"'\n> + do\n> + strategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n> + done\n> +'\n>\n> GIT_CHERRY_PICK_HELP=\"$resolvemsg\"\n> export GIT_CHERRY_PICK_HELP\n> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\n> index 98eb49a..9a2461c 100755\n> --- a/t/t3404-rebase-interactive.sh\n> +++ b/t/t3404-rebase-interactive.sh\n> @@ -1006,6 +1006,22 @@ test_expect_success 'rebase -i with --strategy and -X' '\n> test $(cat file1) = Z\n> '\n>\n> +test_expect_success 'interrupted rebase -i with --strategy and -X' '\n> + git checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&\n> + git reset --hard HEAD^ &&\n> + >breakpoint &&\n> + git add breakpoint &&\n> + git commit -m \"breakpoint for interactive mode\" &&\n> + echo five >conflict &&\n> + echo Z >file1 &&\n> + git commit -a -m \"one file conflict\" &&\n> + set_fake_editor &&\n> + FAKE_LINES=\"edit 1 2\" git rebase -i --strategy=recursive -Xours conflict-branch &&\n> + git rebase --continue &&\n> + test $(git show conflict-branch:conflict) = $(cat conflict) &&\n> + test $(cat file1) = Z\n> +'\n> +\n> test_expect_success 'rebase -i error on commits with \\ in message' '\n> current_head=$(git rev-parse HEAD) &&\n> test_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n> --\n> 2.7.0.rc0.174.g1b62464\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"274293","messageId":"1449865836-27180-1-git-send-email-ralf.thielow@gmail.com","threadId":"36877","inReplyTo":"CAPc5daV_tPh9pt4YSpsBSCvrvGOqC7+9eTZkS1bV2ZAE2YoxzA@mail.gmail.com","subject":"[PATCH v2] rebase -i: remember merge options beyond continue actions","fromName":"Ralf Thielow","fromEmail":"ralf.thielow@gmail.com","sentAt":"2015-12-11T20:30:36Z","receivedAt":"2015-12-11T20:30:36Z","isPatch":true,"sender":{"key":"ralf.thielow@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1275832?v=4"},"body":"From: Fabian Ruch \n\nIf the user explicitly specified a merge strategy or strategy\noptions, continue to use that strategy/option after\n\"rebase --continue\". Add a test of the corrected behavior.\n\nIf --merge is specified or implied by -s or -X, then \"strategy and\n\"strategy_opts\" are set to values from which \"strategy_args\" can be\nderived; otherwise they are set to empty strings. Either way,\ntheir values are propagated from one step of an interactive rebase\nto the next via state files.\n\n\"do_merge\", on the other hand, is *not* propagated to later steps of\nan interactive rebase. Therefore, making the initialization of\n\"strategy_args\" conditional on \"do_merge\" being set prevents later\nsteps of an interactive rebase from setting it correctly.\n\nLuckily, we don't need the \"do_merge\" guard at all. If the rebase\nwas started without --merge, then \"strategy\" and \"strategy_opts\"\nare both the empty string, which results in \"strategy_args\" also\nbeing set to the empty string, which is just what we want in that\nsituation. So remove the \"do_merge\" guard and derive\n\"strategy_args\" from \"strategy\" and \"strategy_opts\" every time.\n\nReported-by: Diogo de Campos \nSigned-off-by: Fabian Ruch \nHelped-by: Michael Haggerty \nSigned-off-by: Ralf Thielow \n---\n2015-12-11 21:07 GMT+01:00 Junio C Hamano :\n> On Fri, Dec 11, 2015 at 11:54 AM, Ralf Thielow wrote:\n\n>\n> Thanks. I wonder if Michael's rephrasing in $gmane/251386 still applies, which\n> I found by far the most readable.\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/251147/focus=251386\n>\n\nSure.\n\n git-rebase--interactive.sh | 18 +++++++-----------\n t/t3404-rebase-interactive.sh | 16 ++++++++++++++++\n 2 files changed, 23 insertions(+), 11 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex b938a6d..c0cfe88 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -81,17 +81,13 @@ rewritten_pending=\"$state_dir\"/rewritten-pending\n # and leaves CR at the end instead.\n cr=$(printf \"\\015\")\n \n-strategy_args=\n-if test -n \"$do_merge\"\n-then\n-\tstrategy_args=${strategy:+--strategy=$strategy}\n-\teval '\n-\t\tfor strategy_opt in '\"$strategy_opts\"'\n-\t\tdo\n-\t\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n-\t\tdone\n-\t'\n-fi\n+strategy_args=${strategy:+--strategy=$strategy}\n+eval '\n+\tfor strategy_opt in '\"$strategy_opts\"'\n+\tdo\n+\t\tstrategy_args=\"$strategy_args -X$(git rev-parse --sq-quote \"${strategy_opt#--}\")\"\n+\tdone\n+'\n \n GIT_CHERRY_PICK_HELP=\"$resolvemsg\"\n export GIT_CHERRY_PICK_HELP\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 98eb49a..9a2461c 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -1006,6 +1006,22 @@ test_expect_success 'rebase -i with --strategy and -X' '\n \ttest $(cat file1) = Z\n '\n \n+test_expect_success 'interrupted rebase -i with --strategy and -X' '\n+\tgit checkout -b conflict-merge-use-theirs-interrupted conflict-branch &&\n+\tgit reset --hard HEAD^ &&\n+\t>breakpoint &&\n+\tgit add breakpoint &&\n+\tgit commit -m \"breakpoint for interactive mode\" &&\n+\techo five >conflict &&\n+\techo Z >file1 &&\n+\tgit commit -a -m \"one file conflict\" &&\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"edit 1 2\" git rebase -i --strategy=recursive -Xours conflict-branch &&\n+\tgit rebase --continue &&\n+\ttest $(git show conflict-branch:conflict) = $(cat conflict) &&\n+\ttest $(cat file1) = Z\n+'\n+\n test_expect_success 'rebase -i error on commits with \\ in message' '\n \tcurrent_head=$(git rev-parse HEAD) &&\n \ttest_when_finished \"git rebase --abort; git reset --hard $current_head; rm -f error\" &&\n-- \n2.7.0.rc0.174.g1b62464\n"}]}