Fabian Ruch <bafain@xxxxxxxxx> writes: > There are two kinds of to-do list commands available. One kind > replays a commit (`pick`, `reword`, `edit`, `squash` and `fixup` that > is) and the other executes a shell command (`exec`). We will call the > first kind replay commands. > > The two kinds of tasks are scheduled using different line formats. > Replay commands expect a commit hash argument following the command > name and exec concatenates all arguments to assemble a command line. > > Adhere to the distinction of formats by not trying to parse the > `sha1` field unless we are dealing with a replay command. Move the > replay command handling code to a new function `do_replay` which > assumes the first argument to be a commit hash and make no more such > assumptions in `do_next`. In do_next(), we used read the first line of the insn sheet into three variables, assuming the common case of handling one of the replay commands, and (somewhat wastefully) re-read the same line into two variables when we realize that the command was "exec". After you split do_replay() out of do_next() with this patch, we seem to do exactly the same thing. What exactly is the problem this change wants to fix? Puzzled... > > Signed-off-by: Fabian Ruch <bafain@xxxxxxxxx> > --- > git-rebase--interactive.sh | 42 ++++++++++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 14 deletions(-) > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 008f3a0..9de7441 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -585,13 +585,12 @@ do_pick () { > fi > } > > -do_next () { > - rm -f "$msg" "$author_script" "$amend" || exit > - read -r command sha1 rest < "$todo" > +do_replay () { > + command=$1 > + sha1=$2 > + rest=$3 > + > case "$command" in > - "$comment_char"*|''|noop) > - mark_action_done > - ;; > pick|p) > comment_for_reflog pick > > @@ -656,6 +655,28 @@ do_next () { > esac > record_in_rewritten $sha1 > ;; > + *) > + read -r command <"$todo" > + warn "Unknown command: $command" > + fixtodo="Please fix this using 'git rebase --edit-todo'." > + if git rev-parse --verify -q "$sha1" >/dev/null > + then > + die_with_patch $sha1 "$fixtodo" > + else > + die "$fixtodo" > + fi > + ;; > + esac > +} > + > +do_next () { > + rm -f "$msg" "$author_script" "$amend" || exit > + read -r command sha1 rest <"$todo" > + > + case "$command" in > + "$comment_char"*|''|noop) > + mark_action_done > + ;; > x|"exec") > read -r command rest < "$todo" > mark_action_done > @@ -695,14 +716,7 @@ do_next () { > fi > ;; > *) > - warn "Unknown command: $command $sha1 $rest" > - fixtodo="Please fix this using 'git rebase --edit-todo'." > - if git rev-parse --verify -q "$sha1" >/dev/null > - then > - die_with_patch $sha1 "$fixtodo" > - else > - die "$fixtodo" > - fi > + do_replay $command $sha1 "$rest" > ;; > esac > test -s "$todo" && return -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html