Re: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Kevin Ballard wrote:

> [Subject: [PATCHv3] git-rebase--interactive.sh: extend "edit" command to be more useful

Maybe something like

	rebase-i: treat "edit" without sha1 as a request to amend previous commit

would make the meaning more obvious in a shortlog.

> Extend the "edit" command to simply stop for editing if no sha1 is
> given or if the sha1 is equal to "-". This behaves the same as "x false"
> but is a bit friendlier for the user.

Nice.  I like the semantics.

> * Picked up the extended explanation suggested by Jonathan Nieder.
>   I left off the last line about "noop" as that doesn't seem related.

Right, please feel free to remind me if I forget to pick that up again.

> * If the line given is "edit - some comments", emit "some comments" when
>   stopped. This is undocumented

I think that's okay for now (though of course it would be best to explain
some example uses in Documentation/git-rebase.txt in the form of examples).

> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -469,12 +469,29 @@ do_next () {
> +			comment="$rest"
> +			line=$(git rev-list --pretty=oneline -1 --abbrev-commit --abbrev=7 HEAD)

Hmm, the script seems to assume rev-list will not fail throughout.  :/
Ok.

> +			sha1="${line%% *}"
> +			rest="${line#* }"
> +			echo "$sha1" > "$DOTEST"/stopped-sha

Maybe this can be done without relying on details of --pretty=oneline
format?

			sha1=$(git rev-parse --short HEAD)
			rest=$(git show -s --format=%s HEAD)

(Yes, elsewhere the script uses

	git rev-list --no-merges --pretty=oneline --abbrev-commit \
		--abbrev=7 --reverse --left-right --topo-order "$@" |
	sed -n "s/^>//p" |
	while read -r shortsha1 rest

but in that loop, avoiding an extra exec seems more important.)

> +		fi
>  		git rev-parse --verify HEAD > "$AMEND"
>  		warn "Stopped at $sha1... $rest"
> +		if test -n "$comment"; then
> +			warn
> +			warn "	$comment"
> +			warn

Thanks, looks good to me.

Ideas for tests?  (see t3404 for inspiration)