{"thread":{"id":"48117","subject":"[RFC PATCH v3 0/9] rebase-interactive:","startedAt":"2018-03-22T16:57:42Z","lastAt":"2018-03-23T20:10:02Z","messageCount":20,"participants":["Wink Saville","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":3,"patchTotal":9},"messages":[{"id":"342511","messageId":"cover.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":null,"subject":"[RFC PATCH v3 0/9] rebase-interactive:","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:20Z","receivedAt":"2018-03-22T16:57:42Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"I've incorporated review feed back to date. I'm split the change\ninto 9 commits with each commit do a single class of operation.\n\nI've prepared these commits using github and have Travis-CI setup to\ntest my changes. Of the 9 commits 2 failed, the 1st and 5th commits, I\ntested those two locally and they were fine. I then restarted the builds\non Travis-CI they finished fine so it seems the errors were spurious.\n\nWink Saville (9):\n  Simplify pick_on_preserving_merges\n  Call git_rebase__interactive from run_specific_rebase\n  Indent function git_rebase__interactive\n  Extract functions out of git_rebase__interactive\n  Use new functions in git_rebase__interactive\n  Add and use git_rebase__interactive__preserve_merges\n  Remove unused code paths from git_rebase__interactive\n  Remove unused code paths from git_rebase__interactive__preserve_merges\n  Remove merges_option and a blank line\n\n git-rebase--interactive.sh | 407 ++++++++++++++++++++++++---------------------\n git-rebase.sh              |  16 +-\n 2 files changed, 229 insertions(+), 194 deletions(-)\n\n-- \n2.16.2\n\n"},{"id":"342512","messageId":"41b0b34f7d56fd1600dc1dcadd9836f02f139fe3.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 1/9] Simplify pick_on_preserving_merges","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:21Z","receivedAt":"2018-03-22T16:57:45Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"Signed-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 331c8dfea..561e2660e 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -307,17 +307,14 @@ pick_one_preserving_merges () {\n \tesac\n \tsha1=$(git rev-parse $sha1)\n \n-\tif test -f \"$state_dir\"/current-commit\n+\tif test -f \"$state_dir\"/current-commit && test \"$fast_forward\" = t\n \tthen\n-\t\tif test \"$fast_forward\" = t\n-\t\tthen\n-\t\t\twhile read current_commit\n-\t\t\tdo\n-\t\t\t\tgit rev-parse HEAD > \"$rewritten\"/$current_commit\n-\t\t\tdone <\"$state_dir\"/current-commit\n-\t\t\trm \"$state_dir\"/current-commit ||\n-\t\t\t\tdie \"$(gettext \"Cannot write current commit's replacement sha1\")\"\n-\t\tfi\n+\t\twhile read current_commit\n+\t\tdo\n+\t\t\tgit rev-parse HEAD > \"$rewritten\"/$current_commit\n+\t\tdone <\"$state_dir\"/current-commit\n+\t\trm \"$state_dir\"/current-commit ||\n+\t\t\tdie \"$(gettext \"Cannot write current commit's replacement sha1\")\"\n \tfi\n \n \techo $sha1 >> \"$state_dir\"/current-commit\n-- \n2.16.2\n\n"},{"id":"342513","messageId":"47b224af1c22900a69d0e292935f37a719b6a9f3.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:22Z","receivedAt":"2018-03-22T16:57:48Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"Instead of indirectly invoking git_rebase__interactive this invokes\nit directly after sourcing.\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 11 -----------\n git-rebase.sh              | 11 +++++++++--\n 2 files changed, 9 insertions(+), 13 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 561e2660e..213d75f43 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -740,15 +740,6 @@ get_missing_commit_check_level () {\n \tprintf '%s' \"$check_level\" | tr 'A-Z' 'a-z'\n }\n \n-# The whole contents of this file is run by dot-sourcing it from\n-# inside a shell function.  It used to be that \"return\"s we see\n-# below were not inside any function, and expected to return\n-# to the function that dot-sourced us.\n-#\n-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a\n-# construct and continue to run the statements that follow such a \"return\".\n-# As a work-around, we introduce an extra layer of a function\n-# here, and immediately call it after defining it.\n git_rebase__interactive () {\n \n case \"$action\" in\n@@ -1029,5 +1020,3 @@ fi\n do_rest\n \n }\n-# ... and then we call the whole thing.\n-git_rebase__interactive\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex a1f6e5de6..c4ec7c21b 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -196,8 +196,15 @@ run_specific_rebase () {\n \t\texport GIT_EDITOR\n \t\tautosquash=\n \tfi\n-\t. git-rebase--$type\n-\tret=$?\n+\tif test \"$type\" = interactive\n+\tthen\n+\t\t. git-rebase--interactive\n+\t\tgit_rebase__interactive\n+\t\tret=$?\n+\telse\n+\t\t. git-rebase--$type\n+\t\tret=$?\n+\tfi\n \tif test $ret -eq 0\n \tthen\n \t\tfinish_rebase\n-- \n2.16.2\n\n"},{"id":"342514","messageId":"34f2b9350324772dcce9cead650188601a35ac15.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 3/9] Indent function git_rebase__interactive","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:23Z","receivedAt":"2018-03-22T16:57:52Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"Signed-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 432 ++++++++++++++++++++++-----------------------\n 1 file changed, 215 insertions(+), 217 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 213d75f43..a79330f45 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -741,27 +741,26 @@ get_missing_commit_check_level () {\n }\n \n git_rebase__interactive () {\n-\n-case \"$action\" in\n-continue)\n-\tif test ! -d \"$rewritten\"\n-\tthen\n-\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n-\t\t\t--continue\n-\tfi\n-\t# do we have anything to commit?\n-\tif git diff-index --cached --quiet HEAD --\n-\tthen\n-\t\t# Nothing to commit -- skip this commit\n-\n-\t\ttest ! -f \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n-\t\trm \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n-\t\tdie \"$(gettext \"Could not remove CHERRY_PICK_HEAD\")\"\n-\telse\n-\t\tif ! test -f \"$author_script\"\n+\tcase \"$action\" in\n+\tcontinue)\n+\t\tif test ! -d \"$rewritten\"\n \t\tthen\n-\t\t\tgpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")}\n-\t\t\tdie \"$(eval_gettext \"\\\n+\t\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n+\t\t\t\t--continue\n+\t\tfi\n+\t\t# do we have anything to commit?\n+\t\tif git diff-index --cached --quiet HEAD --\n+\t\tthen\n+\t\t\t# Nothing to commit -- skip this commit\n+\n+\t\t\ttest ! -f \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n+\t\t\trm \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n+\t\t\tdie \"$(gettext \"Could not remove CHERRY_PICK_HEAD\")\"\n+\t\telse\n+\t\t\tif ! test -f \"$author_script\"\n+\t\t\tthen\n+\t\t\t\tgpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")}\n+\t\t\t\tdie \"$(eval_gettext \"\\\n You have staged changes in your working tree.\n If these changes are meant to be\n squashed into the previous commit, run:\n@@ -776,197 +775,197 @@ In both cases, once you're done, continue with:\n \n   git rebase --continue\n \")\"\n-\t\tfi\n-\t\t. \"$author_script\" ||\n-\t\t\tdie \"$(gettext \"Error trying to find the author identity to amend commit\")\"\n-\t\tif test -f \"$amend\"\n-\t\tthen\n-\t\t\tcurrent_head=$(git rev-parse --verify HEAD)\n-\t\t\ttest \"$current_head\" = $(cat \"$amend\") ||\n-\t\t\tdie \"$(gettext \"\\\n+\t\t\tfi\n+\t\t\t. \"$author_script\" ||\n+\t\t\t\tdie \"$(gettext \"Error trying to find the author identity to amend commit\")\"\n+\t\t\tif test -f \"$amend\"\n+\t\t\tthen\n+\t\t\t\tcurrent_head=$(git rev-parse --verify HEAD)\n+\t\t\t\ttest \"$current_head\" = $(cat \"$amend\") ||\n+\t\t\t\tdie \"$(gettext \"\\\n You have uncommitted changes in your working tree. Please commit them\n first and then run 'git rebase --continue' again.\")\"\n-\t\t\tdo_with_author git commit --amend --no-verify -F \"$msg\" -e \\\n-\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n-\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n-\t\telse\n-\t\t\tdo_with_author git commit --no-verify -F \"$msg\" -e \\\n-\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n-\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n+\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$msg\" -e \\\n+\t\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n+\t\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n+\t\t\telse\n+\t\t\t\tdo_with_author git commit --no-verify -F \"$msg\" -e \\\n+\t\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n+\t\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n+\t\t\tfi\n \t\tfi\n-\tfi\n \n-\tif test -r \"$state_dir\"/stopped-sha\n-\tthen\n-\t\trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n-\tfi\n+\t\tif test -r \"$state_dir\"/stopped-sha\n+\t\tthen\n+\t\t\trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n+\t\tfi\n \n-\trequire_clean_work_tree \"rebase\"\n-\tdo_rest\n-\treturn 0\n-\t;;\n-skip)\n-\tgit rerere clear\n+\t\trequire_clean_work_tree \"rebase\"\n+\t\tdo_rest\n+\t\treturn 0\n+\t\t;;\n+\tskip)\n+\t\tgit rerere clear\n \n-\tif test ! -d \"$rewritten\"\n-\tthen\n-\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n-\t\t\t--continue\n-\tfi\n-\tdo_rest\n-\treturn 0\n-\t;;\n-edit-todo)\n-\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n-\tmv -f \"$todo\".new \"$todo\"\n-\tcollapse_todo_ids\n-\tappend_todo_help\n-\tgettext \"\n+\t\tif test ! -d \"$rewritten\"\n+\t\tthen\n+\t\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n+\t\t\t\t--continue\n+\t\tfi\n+\t\tdo_rest\n+\t\treturn 0\n+\t\t;;\n+\tedit-todo)\n+\t\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n+\t\tmv -f \"$todo\".new \"$todo\"\n+\t\tcollapse_todo_ids\n+\t\tappend_todo_help\n+\t\tgettext \"\n You are editing the todo file of an ongoing interactive rebase.\n To continue rebase after editing, run:\n     git rebase --continue\n \n \" | git stripspace --comment-lines >>\"$todo\"\n \n-\tgit_sequence_editor \"$todo\" ||\n-\t\tdie \"$(gettext \"Could not execute editor\")\"\n-\texpand_todo_ids\n-\n-\texit\n-\t;;\n-show-current-patch)\n-\texec git show REBASE_HEAD --\n-\t;;\n-esac\n-\n-comment_for_reflog start\n+\t\tgit_sequence_editor \"$todo\" ||\n+\t\t\tdie \"$(gettext \"Could not execute editor\")\"\n+\t\texpand_todo_ids\n \n-if test ! -z \"$switch_to\"\n-then\n-\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n-\toutput git checkout \"$switch_to\" -- ||\n-\t\tdie \"$(eval_gettext \"Could not checkout \\$switch_to\")\"\n+\t\texit\n+\t\t;;\n+\tshow-current-patch)\n+\t\texec git show REBASE_HEAD --\n+\t\t;;\n+\tesac\n \n \tcomment_for_reflog start\n-fi\n-\n-orig_head=$(git rev-parse --verify HEAD) || die \"$(gettext \"No HEAD?\")\"\n-mkdir -p \"$state_dir\" || die \"$(eval_gettext \"Could not create temporary \\$state_dir\")\"\n-rm -f \"$(git rev-parse --git-path REBASE_HEAD)\"\n \n-: > \"$state_dir\"/interactive || die \"$(gettext \"Could not mark as interactive\")\"\n-write_basic_state\n-if test t = \"$preserve_merges\"\n-then\n-\tif test -z \"$rebase_root\"\n+\tif test ! -z \"$switch_to\"\n \tthen\n-\t\tmkdir \"$rewritten\" &&\n-\t\tfor c in $(git merge-base --all $orig_head $upstream)\n-\t\tdo\n-\t\t\techo $onto > \"$rewritten\"/$c ||\n-\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n-\t\tdone\n-\telse\n-\t\tmkdir \"$rewritten\" &&\n-\t\techo $onto > \"$rewritten\"/root ||\n-\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n+\t\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n+\t\toutput git checkout \"$switch_to\" -- ||\n+\t\t\tdie \"$(eval_gettext \"Could not checkout \\$switch_to\")\"\n+\n+\t\tcomment_for_reflog start\n \tfi\n-\t# No cherry-pick because our first pass is to determine\n-\t# parents to rewrite and skipping dropped commits would\n-\t# prematurely end our probe\n-\tmerges_option=\n-else\n-\tmerges_option=\"--no-merges --cherry-pick\"\n-fi\n-\n-shorthead=$(git rev-parse --short $orig_head)\n-shortonto=$(git rev-parse --short $onto)\n-if test -z \"$rebase_root\"\n-\t# this is now equivalent to ! -z \"$upstream\"\n-then\n-\tshortupstream=$(git rev-parse --short $upstream)\n-\trevisions=$upstream...$orig_head\n-\tshortrevisions=$shortupstream..$shorthead\n-else\n-\trevisions=$onto...$orig_head\n-\tshortrevisions=$shorthead\n-fi\n-if test t != \"$preserve_merges\"\n-then\n-\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n-\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\" ||\n-\tdie \"$(gettext \"Could not generate todo list\")\"\n-else\n-\tformat=$(git config --get rebase.instructionFormat)\n-\t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n-\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n-\t\t--reverse --left-right --topo-order \\\n-\t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n-\t\tsed -n \"s/^>//p\" |\n-\twhile read -r sha1 rest\n-\tdo\n \n-\t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n-\t\tthen\n-\t\t\tcomment_out=\"$comment_char \"\n-\t\telse\n-\t\t\tcomment_out=\n-\t\tfi\n+\torig_head=$(git rev-parse --verify HEAD) || die \"$(gettext \"No HEAD?\")\"\n+\tmkdir -p \"$state_dir\" || die \"$(eval_gettext \"Could not create temporary \\$state_dir\")\"\n+\trm -f \"$(git rev-parse --git-path REBASE_HEAD)\"\n \n+\t: > \"$state_dir\"/interactive || die \"$(gettext \"Could not mark as interactive\")\"\n+\twrite_basic_state\n+\tif test t = \"$preserve_merges\"\n+\tthen\n \t\tif test -z \"$rebase_root\"\n \t\tthen\n-\t\t\tpreserve=t\n-\t\t\tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)\n+\t\t\tmkdir \"$rewritten\" &&\n+\t\t\tfor c in $(git merge-base --all $orig_head $upstream)\n \t\t\tdo\n-\t\t\t\tif test -f \"$rewritten\"/$p\n-\t\t\t\tthen\n-\t\t\t\t\tpreserve=f\n-\t\t\t\tfi\n+\t\t\t\techo $onto > \"$rewritten\"/$c ||\n+\t\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n \t\t\tdone\n \t\telse\n-\t\t\tpreserve=f\n-\t\tfi\n-\t\tif test f = \"$preserve\"\n-\t\tthen\n-\t\t\ttouch \"$rewritten\"/$sha1\n-\t\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n-\t\tfi\n-\tdone\n-fi\n-\n-# Watch for commits that been dropped by --cherry-pick\n-if test t = \"$preserve_merges\"\n-then\n-\tmkdir \"$dropped\"\n-\t# Save all non-cherry-picked changes\n-\tgit rev-list $revisions --left-right --cherry-pick | \\\n-\t\tsed -n \"s/^>//p\" > \"$state_dir\"/not-cherry-picks\n-\t# Now all commits and note which ones are missing in\n-\t# not-cherry-picks and hence being dropped\n-\tgit rev-list $revisions |\n-\twhile read rev\n-\tdo\n-\t\tif test -f \"$rewritten\"/$rev &&\n-\t\t   ! sane_grep \"$rev\" \"$state_dir\"/not-cherry-picks >/dev/null\n-\t\tthen\n-\t\t\t# Use -f2 because if rev-list is telling us this commit is\n-\t\t\t# not worthwhile, we don't want to track its multiple heads,\n-\t\t\t# just the history of its first-parent for others that will\n-\t\t\t# be rebasing on top of it\n-\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$dropped\"/$rev\n-\t\t\tsha1=$(git rev-list -1 $rev)\n-\t\t\tsane_grep -v \"^[a-z][a-z]* $sha1\" <\"$todo\" > \"${todo}2\" ; mv \"${todo}2\" \"$todo\"\n-\t\t\trm \"$rewritten\"/$rev\n+\t\t\tmkdir \"$rewritten\" &&\n+\t\t\techo $onto > \"$rewritten\"/root ||\n+\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n \t\tfi\n-\tdone\n-fi\n+\t\t# No cherry-pick because our first pass is to determine\n+\t\t# parents to rewrite and skipping dropped commits would\n+\t\t# prematurely end our probe\n+\t\tmerges_option=\n+\telse\n+\t\tmerges_option=\"--no-merges --cherry-pick\"\n+\tfi\n+\n+\tshorthead=$(git rev-parse --short $orig_head)\n+\tshortonto=$(git rev-parse --short $onto)\n+\tif test -z \"$rebase_root\"\n+\t\t# this is now equivalent to ! -z \"$upstream\"\n+\tthen\n+\t\tshortupstream=$(git rev-parse --short $upstream)\n+\t\trevisions=$upstream...$orig_head\n+\t\tshortrevisions=$shortupstream..$shorthead\n+\telse\n+\t\trevisions=$onto...$orig_head\n+\t\tshortrevisions=$shorthead\n+\tfi\n+\tif test t != \"$preserve_merges\"\n+\tthen\n+\t\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n+\t\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\" ||\n+\t\tdie \"$(gettext \"Could not generate todo list\")\"\n+\telse\n+\t\tformat=$(git config --get rebase.instructionFormat)\n+\t\t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n+\t\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n+\t\t\t--reverse --left-right --topo-order \\\n+\t\t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n+\t\t\tsed -n \"s/^>//p\" |\n+\t\twhile read -r sha1 rest\n+\t\tdo\n+\n+\t\t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n+\t\t\tthen\n+\t\t\t\tcomment_out=\"$comment_char \"\n+\t\t\telse\n+\t\t\t\tcomment_out=\n+\t\t\tfi\n+\n+\t\t\tif test -z \"$rebase_root\"\n+\t\t\tthen\n+\t\t\t\tpreserve=t\n+\t\t\t\tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)\n+\t\t\t\tdo\n+\t\t\t\t\tif test -f \"$rewritten\"/$p\n+\t\t\t\t\tthen\n+\t\t\t\t\t\tpreserve=f\n+\t\t\t\t\tfi\n+\t\t\t\tdone\n+\t\t\telse\n+\t\t\t\tpreserve=f\n+\t\t\tfi\n+\t\t\tif test f = \"$preserve\"\n+\t\t\tthen\n+\t\t\t\ttouch \"$rewritten\"/$sha1\n+\t\t\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n+\t\t\tfi\n+\t\tdone\n+\tfi\n+\n+\t# Watch for commits that been dropped by --cherry-pick\n+\tif test t = \"$preserve_merges\"\n+\tthen\n+\t\tmkdir \"$dropped\"\n+\t\t# Save all non-cherry-picked changes\n+\t\tgit rev-list $revisions --left-right --cherry-pick | \\\n+\t\t\tsed -n \"s/^>//p\" > \"$state_dir\"/not-cherry-picks\n+\t\t# Now all commits and note which ones are missing in\n+\t\t# not-cherry-picks and hence being dropped\n+\t\tgit rev-list $revisions |\n+\t\twhile read rev\n+\t\tdo\n+\t\t\tif test -f \"$rewritten\"/$rev &&\n+\t\t\t   ! sane_grep \"$rev\" \"$state_dir\"/not-cherry-picks >/dev/null\n+\t\t\tthen\n+\t\t\t\t# Use -f2 because if rev-list is telling us this commit is\n+\t\t\t\t# not worthwhile, we don't want to track its multiple heads,\n+\t\t\t\t# just the history of its first-parent for others that will\n+\t\t\t\t# be rebasing on top of it\n+\t\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$dropped\"/$rev\n+\t\t\t\tsha1=$(git rev-list -1 $rev)\n+\t\t\t\tsane_grep -v \"^[a-z][a-z]* $sha1\" <\"$todo\" > \"${todo}2\" ; mv \"${todo}2\" \"$todo\"\n+\t\t\t\trm \"$rewritten\"/$rev\n+\t\t\tfi\n+\t\tdone\n+\tfi\n \n-test -s \"$todo\" || echo noop >> \"$todo\"\n-test -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n-test -n \"$cmd\" && git rebase--helper --add-exec-commands \"$cmd\"\n+\ttest -s \"$todo\" || echo noop >> \"$todo\"\n+\ttest -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n+\ttest -n \"$cmd\" && git rebase--helper --add-exec-commands \"$cmd\"\n \n-todocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n-todocount=${todocount##* }\n+\ttodocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n+\ttodocount=${todocount##* }\n \n cat >>\"$todo\" <<EOF\n \n@@ -975,48 +974,47 @@ $comment_char $(eval_ngettext \\\n \t\"Rebase \\$shortrevisions onto \\$shortonto (\\$todocount commands)\" \\\n \t\"$todocount\")\n EOF\n-append_todo_help\n-gettext \"\n-However, if you remove everything, the rebase will be aborted.\n-\n-\" | git stripspace --comment-lines >>\"$todo\"\n+\tappend_todo_help\n+\tgettext \"\n+\tHowever, if you remove everything, the rebase will be aborted.\n \n-if test -z \"$keep_empty\"\n-then\n-\tprintf '%s\\n' \"$comment_char $(gettext \"Note that empty commits are commented out\")\" >>\"$todo\"\n-fi\n+\t\" | git stripspace --comment-lines >>\"$todo\"\n \n+\tif test -z \"$keep_empty\"\n+\tthen\n+\t\tprintf '%s\\n' \"$comment_char $(gettext \"Note that empty commits are commented out\")\" >>\"$todo\"\n+\tfi\n \n-has_action \"$todo\" ||\n-\treturn 2\n \n-cp \"$todo\" \"$todo\".backup\n-collapse_todo_ids\n-git_sequence_editor \"$todo\" ||\n-\tdie_abort \"$(gettext \"Could not execute editor\")\"\n+\thas_action \"$todo\" ||\n+\t\treturn 2\n \n-has_action \"$todo\" ||\n-\treturn 2\n+\tcp \"$todo\" \"$todo\".backup\n+\tcollapse_todo_ids\n+\tgit_sequence_editor \"$todo\" ||\n+\t\tdie_abort \"$(gettext \"Could not execute editor\")\"\n \n-git rebase--helper --check-todo-list || {\n-\tret=$?\n-\tcheckout_onto\n-\texit $ret\n-}\n+\thas_action \"$todo\" ||\n+\t\treturn 2\n \n-expand_todo_ids\n+\tgit rebase--helper --check-todo-list || {\n+\t\tret=$?\n+\t\tcheckout_onto\n+\t\texit $ret\n+\t}\n \n-test -d \"$rewritten\" || test -n \"$force_rebase\" ||\n-onto=\"$(git rebase--helper --skip-unnecessary-picks)\" ||\n-die \"Could not skip unnecessary pick commands\"\n+\texpand_todo_ids\n \n-checkout_onto\n-if test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n-then\n-\trequire_clean_work_tree \"rebase\"\n-\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n-\t\t--continue\n-fi\n-do_rest\n+\ttest -d \"$rewritten\" || test -n \"$force_rebase\" ||\n+\tonto=\"$(git rebase--helper --skip-unnecessary-picks)\" ||\n+\tdie \"Could not skip unnecessary pick commands\"\n \n+\tcheckout_onto\n+\tif test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n+\tthen\n+\t\trequire_clean_work_tree \"rebase\"\n+\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n+\t\t\t--continue\n+\tfi\n+\tdo_rest\n }\n-- \n2.16.2\n\n"},{"id":"342515","messageId":"fd4a622bf7a7b94924c9f9e5dd2ed1bd5255ec7d.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 4/9] Extract functions out of git_rebase__interactive","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:24Z","receivedAt":"2018-03-22T16:57:54Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"The extracted functions are:\n  - initiate_action\n  - setup_reflog_action\n  - init_basic_state\n  - init_revisions_and_shortrevisions\n  - complete_action\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 211 +++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 211 insertions(+)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex a79330f45..b72f80ae8 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -740,6 +740,217 @@ get_missing_commit_check_level () {\n \tprintf '%s' \"$check_level\" | tr 'A-Z' 'a-z'\n }\n \n+# Initiate an action. If the cannot be any\n+# further action it  may exec a command\n+# or exit and not return.\n+#\n+# TODO: Consider a cleaner return model so it\n+# never exits and always return 0 if process\n+# is complete.\n+#\n+# Parameter 1 is the action to initiate.\n+#\n+# Returns 0 if the action was able to complete\n+# and if 1 if further processing is required.\n+initiate_action () {\n+\tcase \"$1\" in\n+\tcontinue)\n+\t\tif test ! -d \"$rewritten\"\n+\t\tthen\n+\t\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n+\t\t\t\t--continue\n+\t\tfi\n+\t\t# do we have anything to commit?\n+\t\tif git diff-index --cached --quiet HEAD --\n+\t\tthen\n+\t\t\t# Nothing to commit -- skip this commit\n+\n+\t\t\ttest ! -f \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n+\t\t\trm \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n+\t\t\tdie \"$(gettext \"Could not remove CHERRY_PICK_HEAD\")\"\n+\t\telse\n+\t\t\tif ! test -f \"$author_script\"\n+\t\t\tthen\n+\t\t\t\tgpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")}\n+\t\t\t\tdie \"$(eval_gettext \"\\\n+You have staged changes in your working tree.\n+If these changes are meant to be\n+squashed into the previous commit, run:\n+\n+  git commit --amend \\$gpg_sign_opt_quoted\n+\n+If they are meant to go into a new commit, run:\n+\n+  git commit \\$gpg_sign_opt_quoted\n+\n+In both cases, once you're done, continue with:\n+\n+  git rebase --continue\n+\")\"\n+\t\t\tfi\n+\t\t\t. \"$author_script\" ||\n+\t\t\t\tdie \"$(gettext \"Error trying to find the author identity to amend commit\")\"\n+\t\t\tif test -f \"$amend\"\n+\t\t\tthen\n+\t\t\t\tcurrent_head=$(git rev-parse --verify HEAD)\n+\t\t\t\ttest \"$current_head\" = $(cat \"$amend\") ||\n+\t\t\t\tdie \"$(gettext \"\\\n+You have uncommitted changes in your working tree. Please commit them\n+first and then run 'git rebase --continue' again.\")\"\n+\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$msg\" -e \\\n+\t\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n+\t\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n+\t\t\telse\n+\t\t\t\tdo_with_author git commit --no-verify -F \"$msg\" -e \\\n+\t\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n+\t\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n+\t\t\tfi\n+\t\tfi\n+\n+\t\tif test -r \"$state_dir\"/stopped-sha\n+\t\tthen\n+\t\t\trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n+\t\tfi\n+\n+\t\trequire_clean_work_tree \"rebase\"\n+\t\tdo_rest\n+\t\treturn 0\n+\t\t;;\n+\tskip)\n+\t\tgit rerere clear\n+\n+\t\tif test ! -d \"$rewritten\"\n+\t\tthen\n+\t\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n+\t\t\t\t--continue\n+\t\tfi\n+\t\tdo_rest\n+\t\treturn 0\n+\t\t;;\n+\tedit-todo)\n+\t\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n+\t\tmv -f \"$todo\".new \"$todo\"\n+\t\tcollapse_todo_ids\n+\t\tappend_todo_help\n+\t\tgettext \"\n+You are editing the todo file of an ongoing interactive rebase.\n+To continue rebase after editing, run:\n+    git rebase --continue\n+\n+\" | git stripspace --comment-lines >>\"$todo\"\n+\n+\t\tgit_sequence_editor \"$todo\" ||\n+\t\t\tdie \"$(gettext \"Could not execute editor\")\"\n+\t\texpand_todo_ids\n+\n+\t\texit\n+\t\t;;\n+\tshow-current-patch)\n+\t\texec git show REBASE_HEAD --\n+\t\t;;\n+\t*)\n+\t\treturn 1 # continue\n+\t\t;;\n+\tesac\n+}\n+\n+setup_reflog_action () {\n+\tcomment_for_reflog start\n+\n+\tif test ! -z \"$switch_to\"\n+\tthen\n+\t\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n+\t\toutput git checkout \"$switch_to\" -- ||\n+\t\t\tdie \"$(eval_gettext \"Could not checkout \\$switch_to\")\"\n+\n+\t\tcomment_for_reflog start\n+\tfi\n+}\n+\n+init_basic_state () {\n+\torig_head=$(git rev-parse --verify HEAD) || die \"$(gettext \"No HEAD?\")\"\n+\tmkdir -p \"$state_dir\" || die \"$(eval_gettext \"Could not create temporary \\$state_dir\")\"\n+\trm -f \"$(git rev-parse --git-path REBASE_HEAD)\"\n+\n+\t: > \"$state_dir\"/interactive || die \"$(gettext \"Could not mark as interactive\")\"\n+\twrite_basic_state\n+}\n+\n+init_revisions_and_shortrevisions () {\n+\tshorthead=$(git rev-parse --short $orig_head)\n+\tshortonto=$(git rev-parse --short $onto)\n+\tif test -z \"$rebase_root\"\n+\t\t# this is now equivalent to ! -z \"$upstream\"\n+\tthen\n+\t\tshortupstream=$(git rev-parse --short $upstream)\n+\t\trevisions=$upstream...$orig_head\n+\t\tshortrevisions=$shortupstream..$shorthead\n+\telse\n+\t\trevisions=$onto...$orig_head\n+\t\tshortrevisions=$shorthead\n+\tfi\n+}\n+\n+complete_action() {\n+\ttest -s \"$todo\" || echo noop >> \"$todo\"\n+\ttest -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n+\ttest -n \"$cmd\" && git rebase--helper --add-exec-commands \"$cmd\"\n+\n+\ttodocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n+\ttodocount=${todocount##* }\n+\n+cat >>\"$todo\" <<EOF\n+\n+$comment_char $(eval_ngettext \\\n+\t\"Rebase \\$shortrevisions onto \\$shortonto (\\$todocount command)\" \\\n+\t\"Rebase \\$shortrevisions onto \\$shortonto (\\$todocount commands)\" \\\n+\t\"$todocount\")\n+EOF\n+\tappend_todo_help\n+\tgettext \"\n+\tHowever, if you remove everything, the rebase will be aborted.\n+\n+\t\" | git stripspace --comment-lines >>\"$todo\"\n+\n+\tif test -z \"$keep_empty\"\n+\tthen\n+\t\tprintf '%s\\n' \"$comment_char $(gettext \"Note that empty commits are commented out\")\" >>\"$todo\"\n+\tfi\n+\n+\n+\thas_action \"$todo\" ||\n+\t\treturn 2\n+\n+\tcp \"$todo\" \"$todo\".backup\n+\tcollapse_todo_ids\n+\tgit_sequence_editor \"$todo\" ||\n+\t\tdie_abort \"$(gettext \"Could not execute editor\")\"\n+\n+\thas_action \"$todo\" ||\n+\t\treturn 2\n+\n+\tgit rebase--helper --check-todo-list || {\n+\t\tret=$?\n+\t\tcheckout_onto\n+\t\texit $ret\n+\t}\n+\n+\texpand_todo_ids\n+\n+\ttest -d \"$rewritten\" || test -n \"$force_rebase\" ||\n+\tonto=\"$(git rebase--helper --skip-unnecessary-picks)\" ||\n+\tdie \"Could not skip unnecessary pick commands\"\n+\n+\tcheckout_onto\n+\tif test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n+\tthen\n+\t\trequire_clean_work_tree \"rebase\"\n+\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n+\t\t\t--continue\n+\tfi\n+\tdo_rest\n+}\n+\n git_rebase__interactive () {\n \tcase \"$action\" in\n \tcontinue)\n-- \n2.16.2\n\n"},{"id":"342516","messageId":"387b3dc01562fd30f5894c91109d1a9d00d07720.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 5/9] Use new functions in git_rebase__interactive","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:25Z","receivedAt":"2018-03-22T16:57:58Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"Use initiate_action, setup_reflog_action, init_basic_state,\ninit_revisions_and_shortrevisions and complete_action.\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 187 ++-------------------------------------------\n 1 file changed, 8 insertions(+), 179 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex b72f80ae8..2c10a7f1a 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -952,120 +952,15 @@ EOF\n }\n \n git_rebase__interactive () {\n-\tcase \"$action\" in\n-\tcontinue)\n-\t\tif test ! -d \"$rewritten\"\n-\t\tthen\n-\t\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n-\t\t\t\t--continue\n-\t\tfi\n-\t\t# do we have anything to commit?\n-\t\tif git diff-index --cached --quiet HEAD --\n-\t\tthen\n-\t\t\t# Nothing to commit -- skip this commit\n-\n-\t\t\ttest ! -f \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n-\t\t\trm \"$GIT_DIR\"/CHERRY_PICK_HEAD ||\n-\t\t\tdie \"$(gettext \"Could not remove CHERRY_PICK_HEAD\")\"\n-\t\telse\n-\t\t\tif ! test -f \"$author_script\"\n-\t\t\tthen\n-\t\t\t\tgpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")}\n-\t\t\t\tdie \"$(eval_gettext \"\\\n-You have staged changes in your working tree.\n-If these changes are meant to be\n-squashed into the previous commit, run:\n-\n-  git commit --amend \\$gpg_sign_opt_quoted\n-\n-If they are meant to go into a new commit, run:\n-\n-  git commit \\$gpg_sign_opt_quoted\n-\n-In both cases, once you're done, continue with:\n-\n-  git rebase --continue\n-\")\"\n-\t\t\tfi\n-\t\t\t. \"$author_script\" ||\n-\t\t\t\tdie \"$(gettext \"Error trying to find the author identity to amend commit\")\"\n-\t\t\tif test -f \"$amend\"\n-\t\t\tthen\n-\t\t\t\tcurrent_head=$(git rev-parse --verify HEAD)\n-\t\t\t\ttest \"$current_head\" = $(cat \"$amend\") ||\n-\t\t\t\tdie \"$(gettext \"\\\n-You have uncommitted changes in your working tree. Please commit them\n-first and then run 'git rebase --continue' again.\")\"\n-\t\t\t\tdo_with_author git commit --amend --no-verify -F \"$msg\" -e \\\n-\t\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n-\t\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n-\t\t\telse\n-\t\t\t\tdo_with_author git commit --no-verify -F \"$msg\" -e \\\n-\t\t\t\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message ||\n-\t\t\t\t\tdie \"$(gettext \"Could not commit staged changes.\")\"\n-\t\t\tfi\n-\t\tfi\n-\n-\t\tif test -r \"$state_dir\"/stopped-sha\n-\t\tthen\n-\t\t\trecord_in_rewritten \"$(cat \"$state_dir\"/stopped-sha)\"\n-\t\tfi\n-\n-\t\trequire_clean_work_tree \"rebase\"\n-\t\tdo_rest\n-\t\treturn 0\n-\t\t;;\n-\tskip)\n-\t\tgit rerere clear\n-\n-\t\tif test ! -d \"$rewritten\"\n-\t\tthen\n-\t\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n-\t\t\t\t--continue\n-\t\tfi\n-\t\tdo_rest\n+\tinitiate_action \"$action\"\n+\tret=$?\n+\tif test $ret = 0; then\n \t\treturn 0\n-\t\t;;\n-\tedit-todo)\n-\t\tgit stripspace --strip-comments <\"$todo\" >\"$todo\".new\n-\t\tmv -f \"$todo\".new \"$todo\"\n-\t\tcollapse_todo_ids\n-\t\tappend_todo_help\n-\t\tgettext \"\n-You are editing the todo file of an ongoing interactive rebase.\n-To continue rebase after editing, run:\n-    git rebase --continue\n-\n-\" | git stripspace --comment-lines >>\"$todo\"\n-\n-\t\tgit_sequence_editor \"$todo\" ||\n-\t\t\tdie \"$(gettext \"Could not execute editor\")\"\n-\t\texpand_todo_ids\n-\n-\t\texit\n-\t\t;;\n-\tshow-current-patch)\n-\t\texec git show REBASE_HEAD --\n-\t\t;;\n-\tesac\n-\n-\tcomment_for_reflog start\n-\n-\tif test ! -z \"$switch_to\"\n-\tthen\n-\t\tGIT_REFLOG_ACTION=\"$GIT_REFLOG_ACTION: checkout $switch_to\"\n-\t\toutput git checkout \"$switch_to\" -- ||\n-\t\t\tdie \"$(eval_gettext \"Could not checkout \\$switch_to\")\"\n-\n-\t\tcomment_for_reflog start\n \tfi\n \n-\torig_head=$(git rev-parse --verify HEAD) || die \"$(gettext \"No HEAD?\")\"\n-\tmkdir -p \"$state_dir\" || die \"$(eval_gettext \"Could not create temporary \\$state_dir\")\"\n-\trm -f \"$(git rev-parse --git-path REBASE_HEAD)\"\n+\tsetup_reflog_action\n+\tinit_basic_state\n \n-\t: > \"$state_dir\"/interactive || die \"$(gettext \"Could not mark as interactive\")\"\n-\twrite_basic_state\n \tif test t = \"$preserve_merges\"\n \tthen\n \t\tif test -z \"$rebase_root\"\n@@ -1089,18 +984,8 @@ To continue rebase after editing, run:\n \t\tmerges_option=\"--no-merges --cherry-pick\"\n \tfi\n \n-\tshorthead=$(git rev-parse --short $orig_head)\n-\tshortonto=$(git rev-parse --short $onto)\n-\tif test -z \"$rebase_root\"\n-\t\t# this is now equivalent to ! -z \"$upstream\"\n-\tthen\n-\t\tshortupstream=$(git rev-parse --short $upstream)\n-\t\trevisions=$upstream...$orig_head\n-\t\tshortrevisions=$shortupstream..$shorthead\n-\telse\n-\t\trevisions=$onto...$orig_head\n-\t\tshortrevisions=$shorthead\n-\tfi\n+\tinit_revisions_and_shortrevisions\n+\n \tif test t != \"$preserve_merges\"\n \tthen\n \t\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n@@ -1171,61 +1056,5 @@ To continue rebase after editing, run:\n \t\tdone\n \tfi\n \n-\ttest -s \"$todo\" || echo noop >> \"$todo\"\n-\ttest -z \"$autosquash\" || git rebase--helper --rearrange-squash || exit\n-\ttest -n \"$cmd\" && git rebase--helper --add-exec-commands \"$cmd\"\n-\n-\ttodocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n-\ttodocount=${todocount##* }\n-\n-cat >>\"$todo\" <<EOF\n-\n-$comment_char $(eval_ngettext \\\n-\t\"Rebase \\$shortrevisions onto \\$shortonto (\\$todocount command)\" \\\n-\t\"Rebase \\$shortrevisions onto \\$shortonto (\\$todocount commands)\" \\\n-\t\"$todocount\")\n-EOF\n-\tappend_todo_help\n-\tgettext \"\n-\tHowever, if you remove everything, the rebase will be aborted.\n-\n-\t\" | git stripspace --comment-lines >>\"$todo\"\n-\n-\tif test -z \"$keep_empty\"\n-\tthen\n-\t\tprintf '%s\\n' \"$comment_char $(gettext \"Note that empty commits are commented out\")\" >>\"$todo\"\n-\tfi\n-\n-\n-\thas_action \"$todo\" ||\n-\t\treturn 2\n-\n-\tcp \"$todo\" \"$todo\".backup\n-\tcollapse_todo_ids\n-\tgit_sequence_editor \"$todo\" ||\n-\t\tdie_abort \"$(gettext \"Could not execute editor\")\"\n-\n-\thas_action \"$todo\" ||\n-\t\treturn 2\n-\n-\tgit rebase--helper --check-todo-list || {\n-\t\tret=$?\n-\t\tcheckout_onto\n-\t\texit $ret\n-\t}\n-\n-\texpand_todo_ids\n-\n-\ttest -d \"$rewritten\" || test -n \"$force_rebase\" ||\n-\tonto=\"$(git rebase--helper --skip-unnecessary-picks)\" ||\n-\tdie \"Could not skip unnecessary pick commands\"\n-\n-\tcheckout_onto\n-\tif test -z \"$rebase_root\" && test ! -d \"$rewritten\"\n-\tthen\n-\t\trequire_clean_work_tree \"rebase\"\n-\t\texec git rebase--helper ${force_rebase:+--no-ff} $allow_empty_message \\\n-\t\t\t--continue\n-\tfi\n-\tdo_rest\n+\tcomplete_action\n }\n-- \n2.16.2\n\n"},{"id":"342517","messageId":"4b0a72043d351ced18b299c8bb9b0f3f27f101fb.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 6/9] Add and use git_rebase__interactive__preserve_merges","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:26Z","receivedAt":"2018-03-22T16:57:59Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"At the moment it's an exact copy of git_rebase__interactive except\nthe name has changed.\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 108 +++++++++++++++++++++++++++++++++++++++++++++\n git-rebase.sh              |   7 ++-\n 2 files changed, 114 insertions(+), 1 deletion(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 2c10a7f1a..ab5513d80 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -1058,3 +1058,111 @@ git_rebase__interactive () {\n \n \tcomplete_action\n }\n+\n+git_rebase__interactive__preserve_merges () {\n+\tinitiate_action \"$action\"\n+\tret=$?\n+\tif test $ret = 0; then\n+\t\treturn 0\n+\tfi\n+\n+\tsetup_reflog_action\n+\tinit_basic_state\n+\n+\tif test t = \"$preserve_merges\"\n+\tthen\n+\t\tif test -z \"$rebase_root\"\n+\t\tthen\n+\t\t\tmkdir \"$rewritten\" &&\n+\t\t\tfor c in $(git merge-base --all $orig_head $upstream)\n+\t\t\tdo\n+\t\t\t\techo $onto > \"$rewritten\"/$c ||\n+\t\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n+\t\t\tdone\n+\t\telse\n+\t\t\tmkdir \"$rewritten\" &&\n+\t\t\techo $onto > \"$rewritten\"/root ||\n+\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n+\t\tfi\n+\t\t# No cherry-pick because our first pass is to determine\n+\t\t# parents to rewrite and skipping dropped commits would\n+\t\t# prematurely end our probe\n+\t\tmerges_option=\n+\telse\n+\t\tmerges_option=\"--no-merges --cherry-pick\"\n+\tfi\n+\n+\tinit_revisions_and_shortrevisions\n+\n+\tif test t != \"$preserve_merges\"\n+\tthen\n+\t\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n+\t\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\" ||\n+\t\tdie \"$(gettext \"Could not generate todo list\")\"\n+\telse\n+\t\tformat=$(git config --get rebase.instructionFormat)\n+\t\t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n+\t\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n+\t\t\t--reverse --left-right --topo-order \\\n+\t\t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n+\t\t\tsed -n \"s/^>//p\" |\n+\t\twhile read -r sha1 rest\n+\t\tdo\n+\n+\t\t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n+\t\t\tthen\n+\t\t\t\tcomment_out=\"$comment_char \"\n+\t\t\telse\n+\t\t\t\tcomment_out=\n+\t\t\tfi\n+\n+\t\t\tif test -z \"$rebase_root\"\n+\t\t\tthen\n+\t\t\t\tpreserve=t\n+\t\t\t\tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)\n+\t\t\t\tdo\n+\t\t\t\t\tif test -f \"$rewritten\"/$p\n+\t\t\t\t\tthen\n+\t\t\t\t\t\tpreserve=f\n+\t\t\t\t\tfi\n+\t\t\t\tdone\n+\t\t\telse\n+\t\t\t\tpreserve=f\n+\t\t\tfi\n+\t\t\tif test f = \"$preserve\"\n+\t\t\tthen\n+\t\t\t\ttouch \"$rewritten\"/$sha1\n+\t\t\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n+\t\t\tfi\n+\t\tdone\n+\tfi\n+\n+\t# Watch for commits that been dropped by --cherry-pick\n+\tif test t = \"$preserve_merges\"\n+\tthen\n+\t\tmkdir \"$dropped\"\n+\t\t# Save all non-cherry-picked changes\n+\t\tgit rev-list $revisions --left-right --cherry-pick | \\\n+\t\t\tsed -n \"s/^>//p\" > \"$state_dir\"/not-cherry-picks\n+\t\t# Now all commits and note which ones are missing in\n+\t\t# not-cherry-picks and hence being dropped\n+\t\tgit rev-list $revisions |\n+\t\twhile read rev\n+\t\tdo\n+\t\t\tif test -f \"$rewritten\"/$rev &&\n+\t\t\t   ! sane_grep \"$rev\" \"$state_dir\"/not-cherry-picks >/dev/null\n+\t\t\tthen\n+\t\t\t\t# Use -f2 because if rev-list is telling us this commit is\n+\t\t\t\t# not worthwhile, we don't want to track its multiple heads,\n+\t\t\t\t# just the history of its first-parent for others that will\n+\t\t\t\t# be rebasing on top of it\n+\t\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$dropped\"/$rev\n+\t\t\t\tsha1=$(git rev-list -1 $rev)\n+\t\t\t\tsane_grep -v \"^[a-z][a-z]* $sha1\" <\"$todo\" > \"${todo}2\" ; mv \"${todo}2\" \"$todo\"\n+\t\t\t\trm \"$rewritten\"/$rev\n+\t\t\tfi\n+\t\tdone\n+\tfi\n+\n+\tcomplete_action\n+}\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex c4ec7c21b..b32282f4c 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -199,7 +199,12 @@ run_specific_rebase () {\n \tif test \"$type\" = interactive\n \tthen\n \t\t. git-rebase--interactive\n-\t\tgit_rebase__interactive\n+\t\tif test \"$preserve_merges\" = t\n+\t\tthen\n+\t\t\tgit_rebase__interactive__preserve_merges\n+\t\telse\n+\t\t\tgit_rebase__interactive\n+\t\tfi\n \t\tret=$?\n \telse\n \t\t. git-rebase--$type\n-- \n2.16.2\n\n"},{"id":"342518","messageId":"83c4c853725abd80b0f3db9ea4eff4fdfde7634f.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 8/9] Remove unused code paths from git_rebase__interactive__preserve_merges","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:28Z","receivedAt":"2018-03-22T16:58:03Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"Since git_rebase__interactive__preserve_merges is now always called with\n$preserve_merges = t we can remove the unused code paths.\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 152 ++++++++++++++++++++-------------------------\n 1 file changed, 69 insertions(+), 83 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 346da0f67..ddbd126f2 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -982,100 +982,86 @@ git_rebase__interactive__preserve_merges () {\n \tsetup_reflog_action\n \tinit_basic_state\n \n-\tif test t = \"$preserve_merges\"\n+\tif test -z \"$rebase_root\"\n \tthen\n-\t\tif test -z \"$rebase_root\"\n-\t\tthen\n-\t\t\tmkdir \"$rewritten\" &&\n-\t\t\tfor c in $(git merge-base --all $orig_head $upstream)\n-\t\t\tdo\n-\t\t\t\techo $onto > \"$rewritten\"/$c ||\n-\t\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n-\t\t\tdone\n-\t\telse\n-\t\t\tmkdir \"$rewritten\" &&\n-\t\t\techo $onto > \"$rewritten\"/root ||\n+\t\tmkdir \"$rewritten\" &&\n+\t\tfor c in $(git merge-base --all $orig_head $upstream)\n+\t\tdo\n+\t\t\techo $onto > \"$rewritten\"/$c ||\n \t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n-\t\tfi\n-\t\t# No cherry-pick because our first pass is to determine\n-\t\t# parents to rewrite and skipping dropped commits would\n-\t\t# prematurely end our probe\n-\t\tmerges_option=\n+\t\tdone\n \telse\n-\t\tmerges_option=\"--no-merges --cherry-pick\"\n+\t\tmkdir \"$rewritten\" &&\n+\t\techo $onto > \"$rewritten\"/root ||\n+\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n \tfi\n \n+\t# No cherry-pick because our first pass is to determine\n+\t# parents to rewrite and skipping dropped commits would\n+\t# prematurely end our probe\n+\tmerges_option=\n+\n \tinit_revisions_and_shortrevisions\n \n-\tif test t != \"$preserve_merges\"\n-\tthen\n-\t\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n-\t\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\" ||\n-\t\tdie \"$(gettext \"Could not generate todo list\")\"\n-\telse\n-\t\tformat=$(git config --get rebase.instructionFormat)\n-\t\t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n-\t\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n-\t\t\t--reverse --left-right --topo-order \\\n-\t\t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n-\t\t\tsed -n \"s/^>//p\" |\n-\t\twhile read -r sha1 rest\n-\t\tdo\n+\tformat=$(git config --get rebase.instructionFormat)\n+\t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n+\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n+\t\t--reverse --left-right --topo-order \\\n+\t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n+\t\tsed -n \"s/^>//p\" |\n+\twhile read -r sha1 rest\n+\tdo\n \n-\t\t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n-\t\t\tthen\n-\t\t\t\tcomment_out=\"$comment_char \"\n-\t\t\telse\n-\t\t\t\tcomment_out=\n-\t\t\tfi\n+\t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n+\t\tthen\n+\t\t\tcomment_out=\"$comment_char \"\n+\t\telse\n+\t\t\tcomment_out=\n+\t\tfi\n \n-\t\t\tif test -z \"$rebase_root\"\n-\t\t\tthen\n-\t\t\t\tpreserve=t\n-\t\t\t\tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)\n-\t\t\t\tdo\n-\t\t\t\t\tif test -f \"$rewritten\"/$p\n-\t\t\t\t\tthen\n-\t\t\t\t\t\tpreserve=f\n-\t\t\t\t\tfi\n-\t\t\t\tdone\n-\t\t\telse\n-\t\t\t\tpreserve=f\n-\t\t\tfi\n-\t\t\tif test f = \"$preserve\"\n-\t\t\tthen\n-\t\t\t\ttouch \"$rewritten\"/$sha1\n-\t\t\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n-\t\t\tfi\n-\t\tdone\n-\tfi\n+\t\tif test -z \"$rebase_root\"\n+\t\tthen\n+\t\t\tpreserve=t\n+\t\t\tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)\n+\t\t\tdo\n+\t\t\t\tif test -f \"$rewritten\"/$p\n+\t\t\t\tthen\n+\t\t\t\t\tpreserve=f\n+\t\t\t\tfi\n+\t\t\tdone\n+\t\telse\n+\t\t\tpreserve=f\n+\t\tfi\n+\t\tif test f = \"$preserve\"\n+\t\tthen\n+\t\t\ttouch \"$rewritten\"/$sha1\n+\t\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n+\t\tfi\n+\tdone\n \n \t# Watch for commits that been dropped by --cherry-pick\n-\tif test t = \"$preserve_merges\"\n-\tthen\n-\t\tmkdir \"$dropped\"\n-\t\t# Save all non-cherry-picked changes\n-\t\tgit rev-list $revisions --left-right --cherry-pick | \\\n-\t\t\tsed -n \"s/^>//p\" > \"$state_dir\"/not-cherry-picks\n-\t\t# Now all commits and note which ones are missing in\n-\t\t# not-cherry-picks and hence being dropped\n-\t\tgit rev-list $revisions |\n-\t\twhile read rev\n-\t\tdo\n-\t\t\tif test -f \"$rewritten\"/$rev &&\n-\t\t\t   ! sane_grep \"$rev\" \"$state_dir\"/not-cherry-picks >/dev/null\n-\t\t\tthen\n-\t\t\t\t# Use -f2 because if rev-list is telling us this commit is\n-\t\t\t\t# not worthwhile, we don't want to track its multiple heads,\n-\t\t\t\t# just the history of its first-parent for others that will\n-\t\t\t\t# be rebasing on top of it\n-\t\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$dropped\"/$rev\n-\t\t\t\tsha1=$(git rev-list -1 $rev)\n-\t\t\t\tsane_grep -v \"^[a-z][a-z]* $sha1\" <\"$todo\" > \"${todo}2\" ; mv \"${todo}2\" \"$todo\"\n-\t\t\t\trm \"$rewritten\"/$rev\n-\t\t\tfi\n-\t\tdone\n-\tfi\n+\tmkdir \"$dropped\"\n+\t# Save all non-cherry-picked changes\n+\tgit rev-list $revisions --left-right --cherry-pick | \\\n+\t\tsed -n \"s/^>//p\" > \"$state_dir\"/not-cherry-picks\n+\t# Now all commits and note which ones are missing in\n+\t# not-cherry-picks and hence being dropped\n+\tgit rev-list $revisions |\n+\twhile read rev\n+\tdo\n+\t\tif test -f \"$rewritten\"/$rev &&\n+\t\t   ! sane_grep \"$rev\" \"$state_dir\"/not-cherry-picks >/dev/null\n+\t\tthen\n+\t\t\t# Use -f2 because if rev-list is telling us this commit is\n+\t\t\t# not worthwhile, we don't want to track its multiple heads,\n+\t\t\t# just the history of its first-parent for others that will\n+\t\t\t# be rebasing on top of it\n+\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$dropped\"/$rev\n+\t\t\tsha1=$(git rev-list -1 $rev)\n+\t\t\tsane_grep -v \"^[a-z][a-z]* $sha1\" <\"$todo\" > \"${todo}2\" ; mv \"${todo}2\" \"$todo\"\n+\t\t\trm \"$rewritten\"/$rev\n+\t\tfi\n+\tdone\n \n \tcomplete_action\n }\n-- \n2.16.2\n\n"},{"id":"342519","messageId":"ab8794c766349f5aab1746797817ffa1d576a04f.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 9/9] Remove merges_option and a blank line","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:29Z","receivedAt":"2018-03-22T16:58:05Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"merges_option is unused in git_rebase__interactive and always empty in\ngit_rebase__interactive__preserve_merges so it can be removed.\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 10 +---------\n 1 file changed, 1 insertion(+), 9 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex ddbd126f2..50323fc27 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -961,8 +961,6 @@ git_rebase__interactive () {\n \tsetup_reflog_action\n \tinit_basic_state\n \n-\tmerges_option=\"--no-merges --cherry-pick\"\n-\n \tinit_revisions_and_shortrevisions\n \n \tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n@@ -996,22 +994,16 @@ git_rebase__interactive__preserve_merges () {\n \t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n \tfi\n \n-\t# No cherry-pick because our first pass is to determine\n-\t# parents to rewrite and skipping dropped commits would\n-\t# prematurely end our probe\n-\tmerges_option=\n-\n \tinit_revisions_and_shortrevisions\n \n \tformat=$(git config --get rebase.instructionFormat)\n \t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n-\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n+\tgit rev-list --format=\"%m%H ${format:-%s}\" \\\n \t\t--reverse --left-right --topo-order \\\n \t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n \t\tsed -n \"s/^>//p\" |\n \twhile read -r sha1 rest\n \tdo\n-\n \t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n \t\tthen\n \t\t\tcomment_out=\"$comment_char \"\n-- \n2.16.2\n\n"},{"id":"342520","messageId":"f8acfb100bc65009843605b4ee24c39decea7ea0.1521690197.git.wink@saville.com","threadId":"48117","inReplyTo":"cover.1521690197.git.wink@saville.com","subject":"[RFC PATCH v3 7/9] Remove unused code paths from git_rebase__interactive","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T16:57:27Z","receivedAt":"2018-03-22T16:58:08Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"Since git_rebase__interactive is now never called with\n$preserve_merges = t we can remove those code paths.\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--interactive.sh | 95 ++--------------------------------------------\n 1 file changed, 4 insertions(+), 91 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex ab5513d80..346da0f67 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -961,100 +961,13 @@ git_rebase__interactive () {\n \tsetup_reflog_action\n \tinit_basic_state\n \n-\tif test t = \"$preserve_merges\"\n-\tthen\n-\t\tif test -z \"$rebase_root\"\n-\t\tthen\n-\t\t\tmkdir \"$rewritten\" &&\n-\t\t\tfor c in $(git merge-base --all $orig_head $upstream)\n-\t\t\tdo\n-\t\t\t\techo $onto > \"$rewritten\"/$c ||\n-\t\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n-\t\t\tdone\n-\t\telse\n-\t\t\tmkdir \"$rewritten\" &&\n-\t\t\techo $onto > \"$rewritten\"/root ||\n-\t\t\t\tdie \"$(gettext \"Could not init rewritten commits\")\"\n-\t\tfi\n-\t\t# No cherry-pick because our first pass is to determine\n-\t\t# parents to rewrite and skipping dropped commits would\n-\t\t# prematurely end our probe\n-\t\tmerges_option=\n-\telse\n-\t\tmerges_option=\"--no-merges --cherry-pick\"\n-\tfi\n+\tmerges_option=\"--no-merges --cherry-pick\"\n \n \tinit_revisions_and_shortrevisions\n \n-\tif test t != \"$preserve_merges\"\n-\tthen\n-\t\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n-\t\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\" ||\n-\t\tdie \"$(gettext \"Could not generate todo list\")\"\n-\telse\n-\t\tformat=$(git config --get rebase.instructionFormat)\n-\t\t# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to parse\n-\t\tgit rev-list $merges_option --format=\"%m%H ${format:-%s}\" \\\n-\t\t\t--reverse --left-right --topo-order \\\n-\t\t\t$revisions ${restrict_revision+^$restrict_revision} | \\\n-\t\t\tsed -n \"s/^>//p\" |\n-\t\twhile read -r sha1 rest\n-\t\tdo\n-\n-\t\t\tif test -z \"$keep_empty\" && is_empty_commit $sha1 && ! is_merge_commit $sha1\n-\t\t\tthen\n-\t\t\t\tcomment_out=\"$comment_char \"\n-\t\t\telse\n-\t\t\t\tcomment_out=\n-\t\t\tfi\n-\n-\t\t\tif test -z \"$rebase_root\"\n-\t\t\tthen\n-\t\t\t\tpreserve=t\n-\t\t\t\tfor p in $(git rev-list --parents -1 $sha1 | cut -d' ' -s -f2-)\n-\t\t\t\tdo\n-\t\t\t\t\tif test -f \"$rewritten\"/$p\n-\t\t\t\t\tthen\n-\t\t\t\t\t\tpreserve=f\n-\t\t\t\t\tfi\n-\t\t\t\tdone\n-\t\t\telse\n-\t\t\t\tpreserve=f\n-\t\t\tfi\n-\t\t\tif test f = \"$preserve\"\n-\t\t\tthen\n-\t\t\t\ttouch \"$rewritten\"/$sha1\n-\t\t\t\tprintf '%s\\n' \"${comment_out}pick $sha1 $rest\" >>\"$todo\"\n-\t\t\tfi\n-\t\tdone\n-\tfi\n-\n-\t# Watch for commits that been dropped by --cherry-pick\n-\tif test t = \"$preserve_merges\"\n-\tthen\n-\t\tmkdir \"$dropped\"\n-\t\t# Save all non-cherry-picked changes\n-\t\tgit rev-list $revisions --left-right --cherry-pick | \\\n-\t\t\tsed -n \"s/^>//p\" > \"$state_dir\"/not-cherry-picks\n-\t\t# Now all commits and note which ones are missing in\n-\t\t# not-cherry-picks and hence being dropped\n-\t\tgit rev-list $revisions |\n-\t\twhile read rev\n-\t\tdo\n-\t\t\tif test -f \"$rewritten\"/$rev &&\n-\t\t\t   ! sane_grep \"$rev\" \"$state_dir\"/not-cherry-picks >/dev/null\n-\t\t\tthen\n-\t\t\t\t# Use -f2 because if rev-list is telling us this commit is\n-\t\t\t\t# not worthwhile, we don't want to track its multiple heads,\n-\t\t\t\t# just the history of its first-parent for others that will\n-\t\t\t\t# be rebasing on top of it\n-\t\t\t\tgit rev-list --parents -1 $rev | cut -d' ' -s -f2 > \"$dropped\"/$rev\n-\t\t\t\tsha1=$(git rev-list -1 $rev)\n-\t\t\t\tsane_grep -v \"^[a-z][a-z]* $sha1\" <\"$todo\" > \"${todo}2\" ; mv \"${todo}2\" \"$todo\"\n-\t\t\t\trm \"$rewritten\"/$rev\n-\t\t\tfi\n-\t\tdone\n-\tfi\n+\tgit rebase--helper --make-script ${keep_empty:+--keep-empty} \\\n+\t\t$revisions ${restrict_revision+^$restrict_revision} >\"$todo\" ||\n+\tdie \"$(gettext \"Could not generate todo list\")\"\n \n \tcomplete_action\n }\n-- \n2.16.2\n\n"},{"id":"342534","messageId":"xmqqpo3wklt2.fsf@gitster-ct.c.googlers.com","threadId":"48117","inReplyTo":"47b224af1c22900a69d0e292935f37a719b6a9f3.1521690197.git.wink@saville.com","subject":"Re: [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-22T18:27:37Z","receivedAt":"2018-03-22T18:27:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wink Saville <wink@saville.com> writes:\n\n> Instead of indirectly invoking git_rebase__interactive this invokes\n> it directly after sourcing.\n>\n> Signed-off-by: Wink Saville <wink@saville.com>\n> ---\n>  git-rebase--interactive.sh | 11 -----------\n>  git-rebase.sh              | 11 +++++++++--\n>  2 files changed, 9 insertions(+), 13 deletions(-)\n>\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 561e2660e..213d75f43 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -740,15 +740,6 @@ get_missing_commit_check_level () {\n>  \tprintf '%s' \"$check_level\" | tr 'A-Z' 'a-z'\n>  }\n>  \n> -# The whole contents of this file is run by dot-sourcing it from\n> -# inside a shell function.  It used to be that \"return\"s we see\n> -# below were not inside any function, and expected to return\n> -# to the function that dot-sourced us.\n> -#\n> -# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a\n> -# construct and continue to run the statements that follow such a \"return\".\n> -# As a work-around, we introduce an extra layer of a function\n> -# here, and immediately call it after defining it.\n\nWe still enclose the whole thing (including the returns that are\nproblematic for older FreeBSD shells) in a shell function, so it's\nnot like we are dropping the workaround for these systems.  It's\njust the caller of the function moved.\n\nI think the removal of this large comment is justifiable, but the\nstructure still needs a bit of explanation, especially given that\nthe caller in git-rebase.sh needs to treat this scriptlet a bit\ndifferently from others.\n\nIf we were not in the (longer term) process of getting rid of\ngit-rebase.sh, it might even make sense to port the same\n\"dot-sourced scriptlet defines a shell function to be called, and\nthe caller calls it after dot-sourcing it\" pattern to other rebase\nbackends, so that the calling side can be unified again to become\nsomething like:\n\n\t. git-rebase--$type\n\tgit_rebase__$type\n\tret=$?\n\n\n\n\n>  git_rebase__interactive () {\n>  \n>  case \"$action\" in\n> @@ -1029,5 +1020,3 @@ fi\n>  do_rest\n>  \n>  }\n> -# ... and then we call the whole thing.\n> -git_rebase__interactive\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index a1f6e5de6..c4ec7c21b 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -196,8 +196,15 @@ run_specific_rebase () {\n>  \t\texport GIT_EDITOR\n>  \t\tautosquash=\n>  \tfi\n> -\t. git-rebase--$type\n> -\tret=$?\n> +\tif test \"$type\" = interactive\n> +\tthen\n> +\t\t. git-rebase--interactive\n> +\t\tgit_rebase__interactive\n> +\t\tret=$?\n> +\telse\n> +\t\t. git-rebase--$type\n> +\t\tret=$?\n> +\tfi\n>  \tif test $ret -eq 0\n>  \tthen\n>  \t\tfinish_rebase\n"},{"id":"342538","messageId":"CAKk8isoL--f1gt_p5XZ99ybN+xts=0_AKBo52Ej=n3SMPoMq3A@mail.gmail.com","threadId":"48117","inReplyTo":"xmqqpo3wklt2.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH v3 2/9] Call git_rebase__interactive from run_specific_rebase","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T19:28:58Z","receivedAt":"2018-03-22T19:29:25Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"On Thu, Mar 22, 2018 at 11:27 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Wink Saville <wink@saville.com> writes:\n>\n>> Instead of indirectly invoking git_rebase__interactive this invokes\n>> it directly after sourcing.\n>>\n>> Signed-off-by: Wink Saville <wink@saville.com>\n>> ---\n>>  git-rebase--interactive.sh | 11 -----------\n>>  git-rebase.sh              | 11 +++++++++--\n>>  2 files changed, 9 insertions(+), 13 deletions(-)\n>>\n>> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n>> index 561e2660e..213d75f43 100644\n>> --- a/git-rebase--interactive.sh\n>> +++ b/git-rebase--interactive.sh\n>> @@ -740,15 +740,6 @@ get_missing_commit_check_level () {\n>>       printf '%s' \"$check_level\" | tr 'A-Z' 'a-z'\n>>  }\n>>\n>> -# The whole contents of this file is run by dot-sourcing it from\n>> -# inside a shell function.  It used to be that \"return\"s we see\n>> -# below were not inside any function, and expected to return\n>> -# to the function that dot-sourced us.\n>> -#\n>> -# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a\n>> -# construct and continue to run the statements that follow such a \"return\".\n>> -# As a work-around, we introduce an extra layer of a function\n>> -# here, and immediately call it after defining it.\n>\n> We still enclose the whole thing (including the returns that are\n> problematic for older FreeBSD shells) in a shell function, so it's\n> not like we are dropping the workaround for these systems.  It's\n> just the caller of the function moved.\n>\n> I think the removal of this large comment is justifiable, but the\n> structure still needs a bit of explanation, especially given that\n> the caller in git-rebase.sh needs to treat this scriptlet a bit\n> differently from others.\n>\n> If we were not in the (longer term) process of getting rid of\n> git-rebase.sh, it might even make sense to port the same\n> \"dot-sourced scriptlet defines a shell function to be called, and\n> the caller calls it after dot-sourcing it\" pattern to other rebase\n> backends, so that the calling side can be unified again to become\n> something like:\n>\n>         . git-rebase--$type\n>         git_rebase__$type\n>         ret=$?\n\nI've gone with the above suggestion and added back some\nof the header.\n"},{"id":"342542","messageId":"7ce3cfef9ff3ab97ac8292fae94a0024a1d85505.1521748846.git.wink@saville.com","threadId":"48117","inReplyTo":"CAKk8isoL--f1gt_p5XZ99ybN+xts=0_AKBo52Ej=n3SMPoMq3A@mail.gmail.com","subject":"[RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T20:03:24Z","receivedAt":"2018-03-22T20:03:37Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"At Junio's suggestion have git-rebase--am and git-rebase--merge work the\nsame way as git-rebase--interactive. This makes the code more consistent.\n\nSigned-off-by: Wink Saville <wink@saville.com>\n---\n git-rebase--am.sh          | 17 ++++++-----------\n git-rebase--interactive.sh |  8 +++++++-\n git-rebase--merge.sh       | 17 ++++++-----------\n git-rebase.sh              | 13 ++++---------\n 4 files changed, 23 insertions(+), 32 deletions(-)\n\ndiff --git a/git-rebase--am.sh b/git-rebase--am.sh\nindex be3f06892..47dc69ed9 100644\n--- a/git-rebase--am.sh\n+++ b/git-rebase--am.sh\n@@ -4,17 +4,14 @@\n # Copyright (c) 2010 Junio C Hamano.\n #\n \n-# The whole contents of this file is run by dot-sourcing it from\n-# inside a shell function.  It used to be that \"return\"s we see\n-# below were not inside any function, and expected to return\n-# to the function that dot-sourced us.\n+# The whole contents of this file is loaded by dot-sourcing it from\n+# inside another shell function, hence no shebang on the first line\n+# and then the caller invokes git_rebase__am.\n #\n-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a\n-# construct and continue to run the statements that follow such a \"return\".\n-# As a work-around, we introduce an extra layer of a function\n-# here, and immediately call it after defining it.\n+# Previously this file was sourced and it called itself to get this\n+# was to get around a bug in older (9.x) versions of FreeBSD.\n git_rebase__am () {\n-\n+echo \"git_rebase_am:+\" 1>&5\n case \"$action\" in\n continue)\n \tgit am --resolved --resolvemsg=\"$resolvemsg\" \\\n@@ -105,5 +102,3 @@ fi\n move_to_original_branch\n \n }\n-# ... and then we call the whole thing.\n-git_rebase__am\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 213d75f43..48f358333 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -740,8 +740,14 @@ get_missing_commit_check_level () {\n \tprintf '%s' \"$check_level\" | tr 'A-Z' 'a-z'\n }\n \n+# The whole contents of this file is loaded by dot-sourcing it from\n+# inside another shell function, hence no shebang on the first line\n+# and then the caller invokes git_rebase__interactive.\n+#\n+# Previously this file was sourced and it called itself to get this\n+# was to get around a bug in older (9.x) versions of FreeBSD.\n git_rebase__interactive () {\n-\n+echo \"git_rebase_interactive:+\" 1>&5\n case \"$action\" in\n continue)\n \tif test ! -d \"$rewritten\"\ndiff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\nindex ceb715453..71de80788 100644\n--- a/git-rebase--merge.sh\n+++ b/git-rebase--merge.sh\n@@ -104,17 +104,14 @@ finish_rb_merge () {\n \tsay All done.\n }\n \n-# The whole contents of this file is run by dot-sourcing it from\n-# inside a shell function.  It used to be that \"return\"s we see\n-# below were not inside any function, and expected to return\n-# to the function that dot-sourced us.\n+# The whole contents of this file is loaded by dot-sourcing it from\n+# inside another shell function, hence no shebang on the first line\n+# and then the caller invokes git_rebase__merge.\n #\n-# However, older (9.x) versions of FreeBSD /bin/sh misbehave on such a\n-# construct and continue to run the statements that follow such a \"return\".\n-# As a work-around, we introduce an extra layer of a function\n-# here, and immediately call it after defining it.\n+# Previously this file was sourced and it called itself to get this\n+# was to get around a bug in older (9.x) versions of FreeBSD.\n git_rebase__merge () {\n-\n+echo \"git_rebase_merge:+\" 1>&5\n case \"$action\" in\n continue)\n \tread_state\n@@ -171,5 +168,3 @@ done\n finish_rb_merge\n \n }\n-# ... and then we call the whole thing.\n-git_rebase__merge\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex c4ec7c21b..4595a316a 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -196,15 +196,10 @@ run_specific_rebase () {\n \t\texport GIT_EDITOR\n \t\tautosquash=\n \tfi\n-\tif test \"$type\" = interactive\n-\tthen\n-\t\t. git-rebase--interactive\n-\t\tgit_rebase__interactive\n-\t\tret=$?\n-\telse\n-\t\t. git-rebase--$type\n-\t\tret=$?\n-\tfi\n+\t# Source the code and invoke it\n+\t. git-rebase--$type\n+\tgit_rebase__$type\n+\tret=$?\n \tif test $ret -eq 0\n \tthen\n \t\tfinish_rebase\n-- \n2.16.2\n\n"},{"id":"342543","messageId":"xmqqefkbltxv.fsf@gitster-ct.c.googlers.com","threadId":"48117","inReplyTo":"7ce3cfef9ff3ab97ac8292fae94a0024a1d85505.1521748846.git.wink@saville.com","subject":"Re: [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-22T20:46:36Z","receivedAt":"2018-03-22T20:46:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wink Saville <wink@saville.com> writes:\n\n> At Junio's suggestion have git-rebase--am and git-rebase--merge work the\n> same way as git-rebase--interactive. This makes the code more consistent.\n\nI mumbled about making git_rebase__$type functions for all $type in\nmy previous response, but that was done without even looking at\ngit-rebase--$type.sh scriptlets.  It seems that they all shared the\nsame structure (i.e. define git_rebase__$type function and then at\nthe end clla it) and were consistent already.  It was the v3 that\nchanged the calling convention only for interactive, which made it\ninconsistent.  If you are making git-rebase.sh call the helper shell\nfunction for all backend $type, you are keeping the existing\nconsistency.\n\nThis is no longer about \"interactive\" alone, though, and need to be\nretitled ;-)\n\n> Signed-off-by: Wink Saville <wink@saville.com>\n> ---\n>  git-rebase--am.sh          | 17 ++++++-----------\n>  git-rebase--interactive.sh |  8 +++++++-\n>  git-rebase--merge.sh       | 17 ++++++-----------\n>  git-rebase.sh              | 13 ++++---------\n>  4 files changed, 23 insertions(+), 32 deletions(-)\n>\n> diff --git a/git-rebase--am.sh b/git-rebase--am.sh\n> index be3f06892..47dc69ed9 100644\n> --- a/git-rebase--am.sh\n> +++ b/git-rebase--am.sh\n> @@ -4,17 +4,14 @@\n>  # Copyright (c) 2010 Junio C Hamano.\n>  #\n>  \n> +# The whole contents of this file is loaded by dot-sourcing it from\n> +# inside another shell function, hence no shebang on the first line\n> +# and then the caller invokes git_rebase__am.\n\nIs this comment necessary?\n\n> +# Previously this file was sourced and it called itself to get this\n> +# was to get around a bug in older (9.x) versions of FreeBSD.\n\nECANTPARSE.  But this probably is no longer needed here, even though\nit may make sense to explain why this comment is no longer relevant\nin the log message.  E.g.\n\n\tThe backend scriptlets for \"git rebase\" are structured in a\n\tbit unusual way for historical reasons.  Originally, it was\n\tdesigned in such a way that dot-sourcing them from \"git\n\trebase\" would be sufficient to invoke the specific backend.\n\tWhen it was discovered that some shell implementations\n\t(e.g. FreeBSD 9.x) misbehaved by exiting when \"return\" is\n\texecuted at the top level of a dot-sourced script (the\n\toriginal was expecting that the control returns to the next\n\tcommand in \"git rebase\" after dot-sourcing the scriptlet),\n\tthe whole body of git-rebase--$backend.sh was made into a\n\tshell function git_rebase__$backend and then the scriptlet\n        was made to call this function at the end as a workaround.\n\n\tMove the call to \"git rebase\" side, instead of at the end of\n\teach scriptlet.  This would give us a more normal\n\tarrangement where a function library lives in a scriptlet\n\tthat is dot-sourced, and then these helper functions are\n\tcalled by the script that dot-sourced the scriptlet.\n\n\tWhile at it, remove the large comment that explains why this\n\trather unusual structure was used from these scriptlets.\n\nor something like that in the log message, and then we can get rid\nof these in-code comments, I would think.\n\n>  git_rebase__am () {\n> -\n> +echo \"git_rebase_am:+\" 1>&5\n\ndebuggin'?  I see similar stuff left in other parts (snipped) of\nthis patch.\n"},{"id":"342548","messageId":"CAKk8isrLrE+sC0Rfv6WUzjziSqGzRgk2xTOh-OKCeEpdxsPfkA@mail.gmail.com","threadId":"48117","inReplyTo":"xmqqefkbltxv.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-22T22:45:48Z","receivedAt":"2018-03-22T22:46:17Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"On Thu, Mar 22, 2018 at 1:46 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Wink Saville <wink@saville.com> writes:\n>\n>> At Junio's suggestion have git-rebase--am and git-rebase--merge work the\n>> same way as git-rebase--interactive. This makes the code more consistent.\n>\n> I mumbled about making git_rebase__$type functions for all $type in\n> my previous response, but that was done without even looking at\n> git-rebase--$type.sh scriptlets.  It seems that they all shared the\n> same structure (i.e. define git_rebase__$type function and then at\n> the end clla it) and were consistent already.  It was the v3 that\n> changed the calling convention only for interactive, which made it\n> inconsistent.  If you are making git-rebase.sh call the helper shell\n> function for all backend $type, you are keeping the existing\n> consistency.\n>\n> This is no longer about \"interactive\" alone, though, and need to be\n> retitled ;-)\n>\n>> Signed-off-by: Wink Saville <wink@saville.com>\n>> ---\n>>  git-rebase--am.sh          | 17 ++++++-----------\n>>  git-rebase--interactive.sh |  8 +++++++-\n>>  git-rebase--merge.sh       | 17 ++++++-----------\n>>  git-rebase.sh              | 13 ++++---------\n>>  4 files changed, 23 insertions(+), 32 deletions(-)\n>>\n>> diff --git a/git-rebase--am.sh b/git-rebase--am.sh\n>> index be3f06892..47dc69ed9 100644\n>> --- a/git-rebase--am.sh\n>> +++ b/git-rebase--am.sh\n>> @@ -4,17 +4,14 @@\n>>  # Copyright (c) 2010 Junio C Hamano.\n>>  #\n>>\n>> +# The whole contents of this file is loaded by dot-sourcing it from\n>> +# inside another shell function, hence no shebang on the first line\n>> +# and then the caller invokes git_rebase__am.\n>\n> Is this comment necessary?\n\nRemoved\n\n>\n>> +# Previously this file was sourced and it called itself to get this\n>> +# was to get around a bug in older (9.x) versions of FreeBSD.\n>\n> ECANTPARSE.  But this probably is no longer needed here, even though\n> it may make sense to explain why this comment is no longer relevant\n> in the log message.  E.g.\n>\n>         The backend scriptlets for \"git rebase\" are structured in a\n>         bit unusual way for historical reasons.  Originally, it was\n>         designed in such a way that dot-sourcing them from \"git\n>         rebase\" would be sufficient to invoke the specific backend.\n>         When it was discovered that some shell implementations\n>         (e.g. FreeBSD 9.x) misbehaved by exiting when \"return\" is\n>         executed at the top level of a dot-sourced script (the\n>         original was expecting that the control returns to the next\n>         command in \"git rebase\" after dot-sourcing the scriptlet),\n>         the whole body of git-rebase--$backend.sh was made into a\n>         shell function git_rebase__$backend and then the scriptlet\n>         was made to call this function at the end as a workaround.\n>\n>         Move the call to \"git rebase\" side, instead of at the end of\n>         each scriptlet.  This would give us a more normal\n>         arrangement where a function library lives in a scriptlet\n>         that is dot-sourced, and then these helper functions are\n>         called by the script that dot-sourced the scriptlet.\n>\n>         While at it, remove the large comment that explains why this\n>         rather unusual structure was used from these scriptlets.\n>\n> or something like that in the log message, and then we can get rid\n> of these in-code comments, I would think.\n\nUpdated commit message\n\n>>  git_rebase__am () {\n>> -\n>> +echo \"git_rebase_am:+\" 1>&5\n>\n> debuggin'?  I see similar stuff left in other parts (snipped) of\n> this patch.\n\nRemoved debugging :(\n\nCurrently I'm not rebasing the other commits (3..9)\nto reduce the amount of work I have to do in each\nreview cycle, is that OK?\n\nAlso, will you merge commits 1 and 2 before the other\ncommits or is the procedure to merge the complete set\nat once?\n"},{"id":"342552","messageId":"xmqqzi2zk0l2.fsf@gitster-ct.c.googlers.com","threadId":"48117","inReplyTo":"CAKk8isrLrE+sC0Rfv6WUzjziSqGzRgk2xTOh-OKCeEpdxsPfkA@mail.gmail.com","subject":"Re: [RFC PATCH v3.1 2/9 2/2] rebase-interactive: Do not automatically run code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T02:06:01Z","receivedAt":"2018-03-23T02:06:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wink Saville <wink@saville.com> writes:\n\n> Currently I'm not rebasing the other commits (3..9)\n> to reduce the amount of work I have to do in each\n> review cycle, is that OK?\n\nYeah, I want to see others more heavily involved in this part of the\nsystem to comment on your patches.  As to the organization of the\nchanges, I have some more opinions of my own, primarily regarding to\nreviewability, but they are of secondary importance than reviews by\narea experts.  I think it would be helpful to give them a target\nthat is not moving too rapidly ;-)\n\n> Also, will you merge commits 1 and 2 before the other\n> commits or is the procedure to merge the complete set\n> at once?\n\nI am inclined to take the early preliminary clean-up steps before\nthe remainder.\n"},{"id":"342639","messageId":"nycvar.QRO.7.76.6.1803231838020.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48117","inReplyTo":"387b3dc01562fd30f5894c91109d1a9d00d07720.1521690197.git.wink@saville.com","subject":"Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-23T17:42:13Z","receivedAt":"2018-03-23T17:42:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Wink,\n\nOn Thu, 22 Mar 2018, Wink Saville wrote:\n\n> Use initiate_action, setup_reflog_action, init_basic_state,\n> init_revisions_and_shortrevisions and complete_action.\n> \n> Signed-off-by: Wink Saville <wink@saville.com>\n> ---\n>  git-rebase--interactive.sh | 187 ++-------------------------------------------\n\nIf you fold this into the previous patch, I am sure that after your 3/9\nindenting the function properly, the splitting into functions will look\nmore or less like this:\n\n-git_rebase__interactive () {\n+initiate_action () {\n+\taction=\"$1\"\n \n \t[... unchanged code ...]\n+}\n+\n+<next function> () {\n+\t[... setting up variables ...]\n \n \t[.. unchanged code ...]\n+}\n[... more of the same ...]\n+\n+git_rebase__interactive () {\n+\tinitiate_action \"$action\" &&\n+ \t<next function> <arguments> &&\n+\t...\n+}\n\nIn other words, it would be easier to review and to verify that the\nprevious code is left unchanged (although that would have to be verified\nmanually by applying 3/9 and then looking at the diff with the -w option,\nanyway).\n\nCiao,\nJohannes\n\n"},{"id":"342640","messageId":"nycvar.QRO.7.76.6.1803231842240.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48117","inReplyTo":"34f2b9350324772dcce9cead650188601a35ac15.1521690197.git.wink@saville.com","subject":"Re: [RFC PATCH v3 3/9] Indent function git_rebase__interactive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-23T17:43:12Z","receivedAt":"2018-03-23T17:43:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Wink,\n\nOn Thu, 22 Mar 2018, Wink Saville wrote:\n\n> Signed-off-by: Wink Saville <wink@saville.com>\n> ---\n>  git-rebase--interactive.sh | 432 ++++++++++++++++++++++-----------------------\n\nIt cannot be helped (at least for now) that this indent change produces\nsuch a large diff.\n\nI am fine with it, of course.\n\nThanks,\nJohannes\n"},{"id":"342658","messageId":"xmqqzi2yiraj.fsf@gitster-ct.c.googlers.com","threadId":"48117","inReplyTo":"nycvar.QRO.7.76.6.1803231838020.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T18:24:20Z","receivedAt":"2018-03-23T18:24:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> If you fold this into the previous patch, I am sure that after your 3/9\n> indenting the function properly, the splitting into functions will look\n> more or less like this:\n>\n> -git_rebase__interactive () {\n> +initiate_action () {\n> +\taction=\"$1\"\n>  \n>  \t[... unchanged code ...]\n> +}\n> +\n> +<next function> () {\n> +\t[... setting up variables ...]\n>  \n>  \t[.. unchanged code ...]\n> +}\n> [... more of the same ...]\n> +\n> +git_rebase__interactive () {\n> +\tinitiate_action \"$action\" &&\n> + \t<next function> <arguments> &&\n> +\t...\n> +}\n>\n> In other words, it would be easier to review and to verify that the\n> previous code is left unchanged (although that would have to be verified\n> manually by applying 3/9 and then looking at the diff with the -w option,\n> anyway).\n\nWe are on the same page on this.  A series structured to have a step\nthat looks exactly like the above would be very pleasant to review.\n\nThanks for a great suggestion.\n"},{"id":"342673","messageId":"CAKk8isqYW5-f8LUQQ5fAybV89gE_pDMWRZe=p7YoCn8ta235YA@mail.gmail.com","threadId":"48117","inReplyTo":"xmqqzi2yiraj.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH v3 5/9] Use new functions in git_rebase__interactive","fromName":"Wink Saville","fromEmail":"wink@saville.com","sentAt":"2018-03-23T20:09:34Z","receivedAt":"2018-03-23T20:10:02Z","isPatch":true,"sender":{"key":"wink@saville.com","avatar":"https://avatars.githubusercontent.com/u/1024284?v=4"},"body":"On Fri, Mar 23, 2018 at 11:24 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n>> If you fold this into the previous patch, I am sure that after your 3/9\n>> indenting the function properly, the splitting into functions will look\n>> more or less like this:\n>>\n>> -git_rebase__interactive () {\n>> +initiate_action () {\n>> +     action=\"$1\"\n>>\n>>       [... unchanged code ...]\n>> +}\n>> +\n>> +<next function> () {\n>> +     [... setting up variables ...]\n>>\n>>       [.. unchanged code ...]\n>> +}\n>> [... more of the same ...]\n>> +\n>> +git_rebase__interactive () {\n>> +     initiate_action \"$action\" &&\n>> +     <next function> <arguments> &&\n>> +     ...\n>> +}\n>>\n>> In other words, it would be easier to review and to verify that the\n>> previous code is left unchanged (although that would have to be verified\n>> manually by applying 3/9 and then looking at the diff with the -w option,\n>> anyway).\n>\n> We are on the same page on this.  A series structured to have a step\n> that looks exactly like the above would be very pleasant to review.\n>\n> Thanks for a great suggestion.\n\nSG and I'll rebase on top of my v5 for the first two commits which\nI rebased on master.\n"}]}