Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

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

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> +apply_autostash () {
> +	if test -f "$state_dir/autostash"
> +	then
> +		stash_sha1=$(cat "$state_dir/autostash")
> +		git stash apply $stash_sha1 2>&1 >/dev/null ||
> +		die "
> +$(eval_gettext 'Applying autostash resulted in conflicts.
> +Either fix the conflicts now, or run
> +	git reset --hard
> +and apply the stash on your desired branch:
> +	git stash apply $stash_sha1
> +at any time.')" &&
> +		echo "Applied autostash"

That && looks funny.  What does it even mean for die to succeed and
give control back to you for a chance to say "Applied"?

	stash apply || die
	echo applied

would be far more logical.

> +	fi
> +	git gc --auto &&
> +	rm -rf "$state_dir"
> +}

This is somewhat worrysome.  After getting the "oops, we cannot
apply" message, the "$stash_sha1" in the message on the terminal is
the ONLY indication of where the (presumably precious) local change
the user used to have can be recovered from.

You do not "rm -fr $state_dir" in such a case, so perhaps telling
the user the location of that "autostash" file may help her
somewhat.

For that matter, wouldn't it be a lot simpler to put the autostash
ref somewhere in refs/ hierarchy, instead of storing an object name
of the stash that keeps (presumably precious) local change of the
user in a plain-text file that is not at all known by "gc"?

> +
>  run_specific_rebase () {
>  	if [ "$interactive_rebase" = implied ]; then
>  		GIT_EDITOR=:
> @@ -153,8 +173,7 @@ run_specific_rebase () {
>  	ret=$?
>  	if test $ret = 0
>  	then
> -		git gc --auto &&
> -		rm -rf "$state_dir"
> +		apply_autostash
>  	fi
>  	exit $ret
>  }
> @@ -248,6 +267,9 @@ do
>  	--stat)
>  		diffstat=t
>  		;;
> +	--autostash)
> +		autostash=true
> +		;;
>  	-v)
>  		verbose=t
>  		diffstat=t
> @@ -348,7 +370,7 @@ abort)
>  		;;
>  	esac
>  	output git reset --hard $orig_head
> -	rm -r "$state_dir"
> +	apply_autostash
>  	exit
>  	;;
>  edit-todo)
> @@ -487,6 +509,16 @@ case "$#" in
>  	;;
>  esac
>  
> +if test "$autostash" = true && ! require_clean_work_tree --quiet
> +then
> +	stash_sha1=$(git stash create) || die "$(gettext "Cannot autostash")" &&
> +	mkdir -p "$state_dir" &&
> +	echo $stash_sha1 >"$state_dir/autostash" &&
> +	stash_abbrev=$(git rev-parse --short $stash_sha1) &&
> +	echo "$(gettext "Created autostash: $stash_abbrev")" &&
> +	git reset --hard
> +fi
> +
>  require_clean_work_tree "rebase" "$(gettext "Please commit or stash them.")"
>  
>  # Now we are rebasing commits $upstream..$orig_head (or with --root,