Re: [PATCH] rebase -i: fixup fixup! fixup!

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

Andrew Pimlott <andrew@pimlott.net> writes:

> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index c84854a..6b2e1c8 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -389,7 +389,9 @@ squash/fixup series.
>  	the same ..., automatically modify the todo list of rebase -i
>  	so that the commit marked for squashing comes right after the
>  	commit to be modified, and change the action of the moved
> -	commit from `pick` to `squash` (or `fixup`).
> +	commit from `pick` to `squash` (or `fixup`).  Ignores subsequent
> +	"fixup! " or "squash! " after the first, in case you referred to an
> +	earlier fixup/squash with `git commit --fixup/--squash`.
>  +
>  This option is only valid when the '--interactive' option is used.
>  +
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index f953d8d..54ed4c3 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -689,7 +689,18 @@ rearrange_squash () {
>  		case "$message" in
>  		"squash! "*|"fixup! "*)
>  			action="${message%%!*}"
> -			rest="${message#*! }"
> +			rest=$message
> +			# ignore any squash! or fixup! after the first
> +			while : ; do

Style:

	while :
        do

> +				case "$rest" in
> +				"squash! "*|"fixup! "*)
> +					rest="${rest#*! }"
> +					;;
> +				*)
> +					break
> +					;;
> +				esac
> +			done
>  			echo "$sha1 $action $rest"
>  			# if it's a single word, try to resolve to a full sha1 and
>  			# emit a second copy. This allows us to match on both message
> diff --git a/t/t3415-rebase-autosquash.sh b/t/t3415-rebase-autosquash.sh
> index a1e86c4..1a3f40a 100755
> --- a/t/t3415-rebase-autosquash.sh
> +++ b/t/t3415-rebase-autosquash.sh
> @@ -193,4 +193,53 @@ test_expect_success 'use commit --squash' '
>  	test_auto_commit_flags squash 2
>  '
>  
> +test_auto_fixup_fixup () {
> +	git reset --hard base &&
> +	echo 1 >file1 &&
> +	git add -u &&
> +	test_tick &&
> +	git commit -m "$1! first" &&
> +	echo 2 >file1 &&
> +	git add -u &&
> +	test_tick &&
> +	git commit -m "$1! $2! first" &&
> +	git tag "final-$1-$2" &&
> +	test_tick &&
> +	git rebase --autosquash -i HEAD^^^^ &&
> +	git log --oneline >actual &&
> +	test_pause &&

This patch obviously hasn't been tested.  It breaks without -v.

> +	if [ "$1" = "fixup" ]; then
> +		test_line_count = 3 actual
> +	elif [ "$1" = "squash" ]; then
> +		test_line_count = 4 actual
> +	else
> +		false
> +	fi &&

Style

	if test "$1" = "fixup"
	then
        	...
	elif test "$1" = "squash"
	then
		...

(you got the idea).

> +	git diff --exit-code "final-$1-$2" &&
> +	test 2 = "$(git cat-file blob HEAD^:file1)" &&
> +	if [ "$1" = "fixup" ]; then
> +		test 1 = $(git cat-file commit HEAD^ | grep first | wc -l)
> +	elif [ "$1" = "squash" ]; then
> +		test 3 = $(git cat-file commit HEAD^ | grep first | wc -l)
> +	else
> +		false
> +	fi
> +}
> +
> +test_expect_success 'fixup! fixup!' '
> +	test_auto_fixup_fixup fixup fixup
> +'
> +
> +test_expect_success 'fixup! squash!' '
> +	test_auto_fixup_fixup fixup squash
> +'
> +
> +test_expect_success 'squash! squash!' '
> +	test_auto_fixup_fixup squash squash
> +'

This does not seem to pass for me.

> +test_expect_success 'squash! fixup!' '
> +	test_auto_fixup_fixup squash fixup
> +'
> +
>  test_done