[PATCH v3 2/2] rebase-i: loosen over-eager check_bad_cmd check
- From
Matthieu Moy <matthieu.moy@imag.fr>
- Date
- Oct 1, 2015, 08:18 UTC
- Message-ID
- <1443687522-14311-2-git-send-email-Matthieu.Moy@imag.fr>
- In-Reply-To
- <1443687522-14311-1-git-send-email-Matthieu.Moy@imag.fr>
804098bb (git rebase -i: add static check for commands and SHA-1, 2015-06-29) tried to check all insns before running any in the todo list, but it did so by implementing its own parser that is a lot stricter than necessary. We used to allow lines that are indented (including comment lines), and we used to allow a whitespace between the insn and the commit object name to be HT, among other things, that are flagged as an invalid line by mistake.
Fix this by using the same tokenizer that is used to parse the todo list file in the new check.
Whether it's a good thing to accept indented comments is debatable (other commands like "git commit" do not accept them), but we already accepted them in the past, and some people and scripts rely on this behavior. Also, a line starting with space followed by a '#' cannot have any meaning other than being a comment, hence it doesn't harm to accept them as comments.
Largely based on patch by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> --- I went back to Junio's version, plus [ ] (space-tab) instead of just space to split the line.
git-rebase--interactive.sh | 62 ++++++++++++++++++++----------------------- t/t3404-rebase-interactive.sh | 15 +++++++++++ 2 files changed, 44 insertions(+), 33 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 0d77429..d65c06e 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -857,7 +857,8 @@ add_exec_commands () { # Check if the SHA-1 passed as an argument is a # correct one, if not then print $2 in "$todo".badsha # $1: the SHA-1 to test -# $2: the line to display if incorrect SHA-1 +# $2: the line number of the input +# $3: the input filename check_commit_sha () { badsha=0 if test -z $1 @@ -873,9 +874,10 @@ check_commit_sha () { if test $badsha -ne 0 then + line="$(sed -n -e "${2}p" "$3")" warn "Warning: the SHA-1 is missing or isn't" \ "a commit in the following line:" - warn " - $2" + warn " - $line" warn fi @@ -886,37 +888,31 @@ check_commit_sha () { # from the todolist in stdin check_bad_cmd_and_sha () { retval=0 - git stripspace --strip-comments | - ( - while read -r line - do - IFS=' ' - set -- $line - command=$1 - sha1=$2 - - case $command in - ''|noop|x|"exec") - # Doesn't expect a SHA-1 - ;; - pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f) - if ! check_commit_sha $sha1 "$line" - then - retval=1 - fi - ;; - *) - warn "Warning: the command isn't recognized" \ - "in the following line:" - warn " - $line" - warn + lineno=0 + while read -r command rest + do + lineno=$(( $lineno + 1 )) + case $command in + "$comment_char"*|''|noop|x|exec) + # Doesn't expect a SHA-1 + ;; + pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f) + if ! check_commit_sha "${rest%%[ ]*}" "$lineno" "$1" + then retval=1 - ;; - esac - done - - return $retval - ) + fi + ;; + *) + line="$(sed -n -e "${lineno}p" "$1")" + warn "Warning: the command isn't recognized" \ + "in the following line:" + warn " - $line" + warn + retval=1 + ;; + esac + done <"$1" + return $retval } # Print the list of the SHA-1 of the commits @@ -1010,7 +1006,7 @@ check_todo_list () { ;; esac - if ! check_bad_cmd_and_sha <"$todo" + if ! check_bad_cmd_and_sha "$todo" then raise_error=t fi diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index d26e3f5..3da3ba3 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1227,6 +1227,21 @@ test_expect_success 'static check of bad command' ' test C = $(git cat-file commit HEAD^ | sed -ne \$p) ' +test_expect_success 'tabs and spaces are accepted in the todolist' ' + rebase_setup_and_clean indented-comment && + write_script add-indent.sh <<-\EOF && + ( + # Turn single spaces into space/tab mix + sed "1s/ /\t/g; 2s/ / /g; 3s/ / \t/g" "$1" + printf "\n\t# comment\n #more\n\t # comment\n" + ) >$1.new + mv "$1.new" "$1" + EOF + test_set_editor "$(pwd)/add-indent.sh" && + git rebase -i HEAD^^^ && + test E = $(git cat-file commit HEAD | sed -ne \$p) +' + cat >expect <<EOF Warning: the SHA-1 is missing or isn't a commit in the following line: - edit XXXXXXX False commit
-- 2.5.0.402.g8854c44