Re: [PATCH RFC v2 15/19] rebase -i: Explicitly distinguish replay commands and exec tasks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 10, 2014, 20:03 UTC
- Message-ID
- <xmqq1ttt7zib.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <07e5f295c0042e78ba83ffb66925e3f09988ae3c.1404323078.git.bafain@gmail.com>
Fabian Ruch <bafain@gmail.com> writes:
Show 14 quoted lines
> 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...
Show 74 quoted lines
>
> Signed-off-by: Fabian Ruch <bafain@gmail.com>
> ---
> 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