Skip to content

Commit 1db168e

Browse files
moygitster
authored andcommitted
rebase-i: loosen over-eager check_bad_cmd check
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 <[email protected]> [jc: updated test with quickfix from Torsten Bögershausen] Signed-off-by: Matthieu Moy <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 31bff64 commit 1db168e

File tree

2 files changed

+44
-33
lines changed

2 files changed

+44
-33
lines changed

git-rebase--interactive.sh

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,8 @@ add_exec_commands () {
849849
# Check if the SHA-1 passed as an argument is a
850850
# correct one, if not then print $2 in "$todo".badsha
851851
# $1: the SHA-1 to test
852-
# $2: the line to display if incorrect SHA-1
852+
# $2: the line number of the input
853+
# $3: the input filename
853854
check_commit_sha () {
854855
badsha=0
855856
if test -z $1
@@ -865,9 +866,10 @@ check_commit_sha () {
865866

866867
if test $badsha -ne 0
867868
then
869+
line="$(sed -n -e "${2}p" "$3")"
868870
warn "Warning: the SHA-1 is missing or isn't" \
869871
"a commit in the following line:"
870-
warn " - $2"
872+
warn " - $line"
871873
warn
872874
fi
873875

@@ -878,37 +880,31 @@ check_commit_sha () {
878880
# from the todolist in stdin
879881
check_bad_cmd_and_sha () {
880882
retval=0
881-
git stripspace --strip-comments |
882-
(
883-
while read -r line
884-
do
885-
IFS=' '
886-
set -- $line
887-
command=$1
888-
sha1=$2
889-
890-
case $command in
891-
''|noop|x|"exec")
892-
# Doesn't expect a SHA-1
893-
;;
894-
pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
895-
if ! check_commit_sha $sha1 "$line"
896-
then
897-
retval=1
898-
fi
899-
;;
900-
*)
901-
warn "Warning: the command isn't recognized" \
902-
"in the following line:"
903-
warn " - $line"
904-
warn
883+
lineno=0
884+
while read -r command rest
885+
do
886+
lineno=$(( $lineno + 1 ))
887+
case $command in
888+
"$comment_char"*|''|noop|x|exec)
889+
# Doesn't expect a SHA-1
890+
;;
891+
pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
892+
if ! check_commit_sha "${rest%%[ ]*}" "$lineno" "$1"
893+
then
905894
retval=1
906-
;;
907-
esac
908-
done
909-
910-
return $retval
911-
)
895+
fi
896+
;;
897+
*)
898+
line="$(sed -n -e "${lineno}p" "$1")"
899+
warn "Warning: the command isn't recognized" \
900+
"in the following line:"
901+
warn " - $line"
902+
warn
903+
retval=1
904+
;;
905+
esac
906+
done <"$1"
907+
return $retval
912908
}
913909

914910
# Print the list of the SHA-1 of the commits
@@ -1002,7 +998,7 @@ check_todo_list () {
1002998
;;
1003999
esac
10041000

1005-
if ! check_bad_cmd_and_sha <"$todo"
1001+
if ! check_bad_cmd_and_sha "$todo"
10061002
then
10071003
raise_error=t
10081004
fi

t/t3404-rebase-interactive.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1206,6 +1206,21 @@ test_expect_success 'static check of bad command' '
12061206
test C = $(git cat-file commit HEAD^ | sed -ne \$p)
12071207
'
12081208

1209+
test_expect_success 'tabs and spaces are accepted in the todolist' '
1210+
rebase_setup_and_clean indented-comment &&
1211+
write_script add-indent.sh <<-\EOF &&
1212+
(
1213+
# Turn single spaces into space/tab mix
1214+
sed "1s/ / /g; 2s/ / /g; 3s/ / /g" "$1"
1215+
printf "\n\t# comment\n #more\n\t # comment\n"
1216+
) >$1.new
1217+
mv "$1.new" "$1"
1218+
EOF
1219+
test_set_editor "$(pwd)/add-indent.sh" &&
1220+
git rebase -i HEAD^^^ &&
1221+
test E = $(git cat-file commit HEAD | sed -ne \$p)
1222+
'
1223+
12091224
cat >expect <<EOF
12101225
Warning: the SHA-1 is missing or isn't a commit in the following line:
12111226
- edit XXXXXXX False commit

0 commit comments

Comments
 (0)