Re: [PATCH] rebase -i: auto-squash commits

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 18, 2009, 05:21 UTC
Message-ID: <7vvdmu15j0.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20090618063348.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

Show 16 quoted lines

>       pick 1/3 Clean up ...
>       pick 2/3 Lay the groundwork
>       pick 3/3 Implement
>       pick 4/3 squash to "clean up"
>   
>   that I'll change to 
>   
>       pick 1/3 Clean up ...
>       squash 4/3 squash to "clean up"
>       pick 2/3 Lay the groundwork
>       pick 3/3 Implement
>   
>   and then I'll need to edit the commit message for the first two combined.
>
> How about this patch?  It does not let you say 'squash to "clean up"'
> but other people who are more skillfull than me can enhance such details.

I have to admit that I wished to see something like this for more than once. It would have been nicer if the patch went one step further and did "squash the patch, but use the log message from the commit that is squashed into, without even asking for a consolidated message", but I think it is a reasonable start.

But as Dscho already objected to, this is a new feature that is potentially dangerous --- there is a risk of matching a commit that was not intended for squashing, albeit small. We may want an explicit option to enable it. On the other hand, you may be able to argue that use of "interactive" rebase is already a sign that the user is likely to want such a convenience, though.

Show 31 quoted lines

>  git-rebase--interactive.sh   |   31 +++++++++++++++++++++++++++++++
>  t/t3414-rebase-autosquash.sh |   36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+), 0 deletions(-)
>  create mode 100755 t/t3414-rebase-autosquash.sh
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index f96d887..0832164 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -482,6 +482,35 @@ get_saved_options () {
>  	test -f "$DOTEST"/rebase-root && REBASE_ROOT=t
>  }
>  
> +# Rearrange the todo list that has both "pick sha1 msg" and
> +# "pick sha1 squash to msg" in it, so that the latter comes
> +# immediately after the former, and change "pick" to "squash".
> +rearrange_squash () {
> +	sed -n -e 's/^pick \([0-9a-f]*\) squash to /\1 /p' "$1" >"$1.sq"
> +	test -s "$1.sq" || return
> +
> +	used=
> +	while read pick sha1 message
> +	do
> +		case " $used" in
> +		*" $sha1 "*) continue ;;
> +		esac
> +		echo "$pick $sha1 $message"
> +		while read squash msg
> +		do
> +			case "$message" in
> +			"$msg"*)

I guess we could even loosen this "must match the leading substring exactly" restriction if we can expose Dscho's Levenstein to Porcelain writers.

> +				echo "squash $squash to $msg"
> +				used="$used$squash "
> +				break
> +				;;

Do you really want to break here? What happens if I have more than one fixup patches to the same commit?

Show 45 quoted lines

> +			esac
> +		done <"$1.sq"
> +	done <"$1" >"$1.rearranged"
> +
> +	cat "$1.rearranged" >"$1"
> +}
> +
>  while test $# != 0
>  do
>  	case "$1" in
> @@ -746,6 +776,7 @@ first and then run 'git rebase --continue' again."
>  		fi
>  
>  		test -s "$TODO" || echo noop >> "$TODO"
> +		rearrange_squash "$TODO"
>  		cat >> "$TODO" << EOF
>  
>  # Rebase $SHORTREVISIONS onto $SHORTONTO
> diff --git a/t/t3414-rebase-autosquash.sh b/t/t3414-rebase-autosquash.sh
> new file mode 100755
> index 0000000..ddb0daf
> --- /dev/null
> +++ b/t/t3414-rebase-autosquash.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +test_description='auto squash'
> +
> +. ./test-lib.sh
> +
> +test_expect_success setup '
> +	echo 0 > file0
> +	git add .
> +	test_tick
> +	git commit -m "initial commit"
> +	echo 0 > file1
> +	echo 2 > file2
> +	git add .
> +	test_tick
> +	git commit -m "first commit"
> +	echo 3 > file3
> +	git add .
> +	test_tick
> +	git commit -m "second commit"
> +'

These tests want to be stringed together with && to catch possible breakages during the setup. The same for the real test below.

Show 10 quoted lines

> +test_expect_success 'auto squash' '
> +	echo 1 > file1
> +	git add -u
> +	test_tick
> +	git commit -m "squash to first"
> +	git tag final
> +	test_tick
> +	git rebase -i HEAD^^^
> +	git log --oneline >actual
> +	test 3 = $(wc -l <actual) &&

Not just count, but you would want to make sure that the rewritten "first commit" now has the desired tree ("1" instead of "0" in file1, if I am reading the test correctly).

> +	git diff --exit-code final
> +'
> +
> +test_done