Re: [PATCH/RFCv4 1/2] git-rebase -i: add command "drop" to remove a commit

[Junio C Hamano <gitster@pobox.com>]

Galan Rémi  <remi.galan-alfonso@ensimag.grenoble-inp.fr> writes:

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index dc3133f..869cc60 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -152,6 +152,7 @@ Commands:
>   s, squash = use commit, but meld into previous commit
>   f, fixup = like "squash", but discard this commit's log message
>   x, exec = run command (the rest of the line) using shell
> + d, drop = remove commit
>  
>  These lines can be re-ordered; they are executed from top to bottom.
>  
> @@ -508,6 +509,23 @@ do_next () {
>  	"$comment_char"*|''|noop)
>  		mark_action_done
>  		;;
> +	drop|d)
> +		if test -z $sha1
> +		then
> +			warn "Missing SHA-1 in 'drop' command."
> +			die "Please fix this using 'git rebase --edit-todo'."

Is this a sensible piece of advice, though?  The user edited the
line away and it does not have the commit object name anymore.
How does she "fix" it in the editor?  The same puzzlement applies
to the other message.

> +		sha1_verif="$(git rev-parse --verify --quiet $sha1^{commit})"
> +		if test -z $sha1_verif

As you are not using the returned string at all, instead of wasting
a shell variable, it would be better to decide to come into this
block by using the exit status from rev-parse (it exits with
non-zero status upon failure).

> +		then
> +			warn "'$sha1' is not a SHA-1 or does not represent" \
> +				"a commit in 'drop' command."
> +			die "Please fix this using 'git rebase --edit-todo'."
> +		fi
> +
> +		mark_action_done
> +		;;
>  	pick|p)
>  		comment_for_reflog pick