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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 3, 2015, 18:02 UTC
Message-ID: <xmqq8uc0y8jw.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1433331859-24832-1-git-send-email-remi.galan-alfonso@ensimag.grenoble-inp.fr>

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

Show 21 quoted lines

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

Show 10 quoted lines

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