From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 13 May 2013 06:28:10 GMT
Subject: Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash
Message-ID: <7vd2svqtqd.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1368359801-28121-8-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> +finish_rebase () {
> +	if test -f "$state_dir/autostash"
> +	then
> +		stash_sha1=$(cat "$state_dir/autostash")
> +		if git stash apply $stash_sha1 2>&1 >/dev/null
> +		then
> +			echo "Applied autostash"
> +		else
> +			ref_stash=refs/stash &&
> +			: >>"$GIT_DIR/logs/$ref_stash" &&
> +			git update-ref -m "autostash" $ref_stash $stash_sha1 \
> +				|| die "$(eval_gettext 'Cannot store $stash_sha1')"

Writing it like this:

			ref_stash=refs/stash &&
			: >>"$GIT_DIR/logs/$ref_stash" &&
			git update-ref -m "autostash" $ref_stash $stash_sha1 ||
			die "$(eval_gettext 'Cannot store $stash_sha1')"

with a blank line before the next "echo", it would be more readable.

As I said in a separate message, having a code that knows where
"stash" is and how it is organized outside the implementation of
"git stash" is less than ideal.  It probably makes more sense to
let programs like this to say:

			git stash store -m "autostash" $stash_sha1 || exit


> +			echo "
> +$(gettext 'Applying autostash resulted in conflicts.
> +Your changes are safe in the stash.
> +You can apply or drop it at any time.')"

This feels funny.  Why not

			gettext "$msg"

without an extra command substitution with an echo?