Junio C Hamano <gitster@xxxxxxxxx> writes:
> I am actually tempted to say that we should revert 804098b, which is
> the simplest fix.
>
> If we want "check everything before doing a single thing" mode, the
> right way to do it would be to base the check on the same loop as
> transform_todo_ids (one way to do so would be to give a third mode
> to that helper function, but I do not think we mind a small code
> duplication).
> ...
So here is a reroll, which is now minimally tested.
-- >8 --
Subject: [PATCH] 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. A new test is by Matthieu Moy.
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
git-rebase--interactive.sh | 62 ++++++++++++++++++++-----------------------
t/t3404-rebase-interactive.sh | 10 +++++++
2 files changed, 39 insertions(+), 33 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dcc3401..ae1806a 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ebdab4b..2437a3c 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1206,6 +1206,16 @@ test_expect_success 'static check of bad command' '
test C = $(git cat-file commit HEAD^ | sed -ne \$p)
'
+test_expect_success 'indented comments are accepted' '
+ rebase_setup_and_clean indented-comment &&
+ write_script add-indent.sh <<-\EOF &&
+ printf "\n \t # comment\n" >>$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.6.0-273-g484a0d0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html