Re: [PATCH/RFCv5 3/3] git rebase -i: add static check for commands and SHA-1

quoted

+        git stripspace --strip-comments |
+        while read -r command sha1 rest
+        do
+                case $command in
+                ''|noop|x|"exec")
+                        ;;
+                pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)
+                        if test -z $sha1
+                        then
+                                echo "$command $rest" >>"$todo".badsha
+                        else
+                                sha1_verif="$(git rev-parse --verify --quiet $sha1^{commit})"
+                                if test -z $sha1_verif
+                                then
+                                        echo "$command $sha1 $rest" \
+                                                >>"$todo".badsha

When you reach the right end of your screen because of indentation,
cutting lines with \ is rarely the best option. Having 5 levels of
indentation is a sign that you should make more functions.

Yeah, I wasn't overly happy with that either, I will try to add some
functions (your example seems like a good way of refactoring).

quoted

+                                commit="$(git rev-list --oneline -1 --ignore-missing $sha1 2>/dev/null)"
+                                if test -z "$commit"
+                                then
+                                        echo "$command $sha1 $rest" \
+                                                >>"$todo".badcmd
+                                else
+                                        echo "$command $commit" >>"$todo".badcmd
+                                fi
+                        fi

What are you trying to do here? It seems that you are trying to recover
the line, but the line is your input, you shouldn't have to recompute
it.

Why isn't printf '%s %s %s' "$command" "$sha1" "$rest" sufficient in all
cases?

It is mainly because here the SHA-1 is a long one (40 chars), however
I agree that computing short_sha1 and then printf '%s %s %s'
"$command" "$short_sha1" "$rest" should be good in this case.

Maybe it would be better to read line by line (to avoid loosing
whitespace information for example), like

        while read -r line
        do
                printf '%s' "$line" | read -r cmd sha1 rest
                case $sha1 in
                        ...

or maybe it's overkill.

Could be a good idea, though I am not completely convinced about it
yet.

Noted for the other points.

Thanks,
Rémi