Skip to content

Commit

Permalink
rebase-i: loosen over-eager check_bad_cmd check
Browse files Browse the repository at this point in the history
804098b (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>
  • Loading branch information
Matthieu Moy authored and Junio C Hamano committed Oct 6, 2015
1 parent 31bff64 commit 1db168e
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 33 deletions.
62 changes: 29 additions & 33 deletions git-rebase--interactive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,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
Expand All @@ -865,9 +866,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

Expand All @@ -878,37 +880,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
Expand Down Expand Up @@ -1002,7 +998,7 @@ check_todo_list () {
;;
esac

if ! check_bad_cmd_and_sha <"$todo"
if ! check_bad_cmd_and_sha "$todo"
then
raise_error=t
fi
Expand Down
15 changes: 15 additions & 0 deletions t/t3404-rebase-interactive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,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/ / /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
Warning: the SHA-1 is missing or isn't a commit in the following line:
- edit XXXXXXX False commit
Expand Down

0 comments on commit 1db168e

Please sign in to comment.