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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ramkumar Ramachandra <artagnon@xxxxxxxxx> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]