rebase-i: loosen over-eager check_bad_cmd check

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>

[jc: updated test with quickfix from Torsten Bögershausen]

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Matthieu Moy
2015-10-01 10:18:42 +02:00
committed by Junio C Hamano
parent 31bff64100
commit 1db168ee97
2 changed files with 44 additions and 33 deletions

View File

@ -849,7 +849,8 @@ add_exec_commands () {
# Check if the SHA-1 passed as an argument is a # Check if the SHA-1 passed as an argument is a
# correct one, if not then print $2 in "$todo".badsha # correct one, if not then print $2 in "$todo".badsha
# $1: the SHA-1 to test # $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 () { check_commit_sha () {
badsha=0 badsha=0
if test -z $1 if test -z $1
@ -865,9 +866,10 @@ check_commit_sha () {
if test $badsha -ne 0 if test $badsha -ne 0
then then
line="$(sed -n -e "${2}p" "$3")"
warn "Warning: the SHA-1 is missing or isn't" \ warn "Warning: the SHA-1 is missing or isn't" \
"a commit in the following line:" "a commit in the following line:"
warn " - $2" warn " - $line"
warn warn
fi fi
@ -878,37 +880,31 @@ check_commit_sha () {
# from the todolist in stdin # from the todolist in stdin
check_bad_cmd_and_sha () { check_bad_cmd_and_sha () {
retval=0 retval=0
git stripspace --strip-comments | lineno=0
( while read -r command rest
while read -r line do
do lineno=$(( $lineno + 1 ))
IFS=' ' case $command in
set -- $line "$comment_char"*|''|noop|x|exec)
command=$1 # Doesn't expect a SHA-1
sha1=$2 ;;
pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
case $command in if ! check_commit_sha "${rest%%[ ]*}" "$lineno" "$1"
''|noop|x|"exec") then
# 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
retval=1 retval=1
;; fi
esac ;;
done *)
line="$(sed -n -e "${lineno}p" "$1")"
return $retval 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 # Print the list of the SHA-1 of the commits
@ -1002,7 +998,7 @@ check_todo_list () {
;; ;;
esac esac
if ! check_bad_cmd_and_sha <"$todo" if ! check_bad_cmd_and_sha "$todo"
then then
raise_error=t raise_error=t
fi fi

View File

@ -1206,6 +1206,21 @@ test_expect_success 'static check of bad command' '
test C = $(git cat-file commit HEAD^ | sed -ne \$p) 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/ / /g; 2s/ / /g; 3s/ / /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 cat >expect <<EOF
Warning: the SHA-1 is missing or isn't a commit in the following line: Warning: the SHA-1 is missing or isn't a commit in the following line:
- edit XXXXXXX False commit - edit XXXXXXX False commit