> > + 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).
> > + 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
--
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