[PATCH] Makes 'git-stash show' stay quiet when there are no stashes.

[PATCH] Makes 'git-stash show' stay quiet when there are no stashes.

[Jing Xue]

(I tried to send this trivial patch for a couple of times using
git-send-email, but somehow it never turned up.)

Currently when there are no stashes, 'git stash show' basically aborts with an
error message from rev-parse: "fatal: Needed a single revision", which can be
confusing. This patch makes git-stash keep quiet and exit gracefully in that
case.

---
 git-stash.sh |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..dbdaeaf 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -119,6 +119,10 @@ show_stash () {
 		flags=--stat
 	fi
 	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
+	if test -z "$s"
+	then
+		exit 0
+	fi
 
 	w_commit=$(git rev-parse --verify "$s") &&
 	b_commit=$(git rev-parse --verify "$s^") &&
-- 
1.5.4.rc0.1.g3696

Re: [PATCH] Makes 'git-stash show' stay quiet when there are no stashes.

[Junio C Hamano]

Jing Xue <jingxue@digizenstudio.com> writes:

> (I tried to send this trivial patch for a couple of times using
> git-send-email, but somehow it never turned up.)
>
> Currently when there are no stashes, 'git stash show' basically aborts with an
> error message from rev-parse: "fatal: Needed a single revision", which can be
> confusing. This patch makes git-stash keep quiet and exit gracefully in that
> case.

I agree "git stash show" should not give cryptic error message, but I
think you should do this only when the user did not explicitly say which
stash to show (that is, we should still give error message if the user
said "git stash show garbage").

[PATCH] Replace the cryptic messages from "git stash show".

[Jing Xue]

On Thu, Dec 13, 2007 at 05:34:40PM -0800, Junio C Hamano wrote:
> 
> I agree "git stash show" should not give cryptic error message, but I
> think you should do this only when the user did not explicitly say which
> stash to show (that is, we should still give error message if the user
> said "git stash show garbage").

Good point. Actually I found out that if there _are_ some stashes and an
invalid name is given, the current behavior is still printing
refs/stash, which I think is not quite right. So I also try to fix that
while I'm at it.

Now "git stash show" will keep quiet and just exit if there are no
stashes at all. "git stash show some-non-existent-stash" will always
print a clear message indicating the case.

---
 git-stash.sh |   23 ++++++++++++++++++++---
 1 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..40e93dd 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -116,13 +116,30 @@ show_stash () {
 	flags=$(git rev-parse --no-revs --flags "$@")
 	if test -z "$flags"
 	then
-		flags=--stat
+		diff_flags=--stat
+	else
+		diff_flags=$flags
+	fi
+	s=$(git rev-parse --revs-only --no-flags "$@")
+	if test -z "$s"
+	then
+		arguments=$@
+		if test "${flags}" = "${arguments}"
+		then
+			s=$(git rev-parse --revs-only --no-flags $ref_stash)
+			if test -z "$s"
+			then
+				return 0
+			fi
+		else
+			eval stash_name=\$$#
+			die "Can't find any stash with name $stash_name"
+		fi
 	fi
-	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
 
 	w_commit=$(git rev-parse --verify "$s") &&
 	b_commit=$(git rev-parse --verify "$s^") &&
-	git diff $flags $b_commit $w_commit
+	git diff $diff_flags $b_commit $w_commit
 }
 
 apply_stash () {
-- 
1.5.4.rc0.8.gd381b

Re: [PATCH] Replace the cryptic messages from "git stash show".

[Junio C Hamano]

Jing Xue <jingxue@digizenstudio.com> writes:

> On Thu, Dec 13, 2007 at 05:34:40PM -0800, Junio C Hamano wrote:
>> 
>> I agree "git stash show" should not give cryptic error message, but I
>> think you should do this only when the user did not explicitly say which
>> stash to show (that is, we should still give error message if the user
>> said "git stash show garbage").
>
> Good point. Actually I found out that if there _are_ some stashes and an
> invalid name is given, the current behavior is still printing
> refs/stash, which I think is not quite right. So I also try to fix that
> while I'm at it.
>
> Now "git stash show" will keep quiet and just exit if there are no
> stashes at all. "git stash show some-non-existent-stash" will always
> print a clear message indicating the case.
>
> ---
>  git-stash.sh |   23 ++++++++++++++++++++---
>  1 files changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index f16fd9c..40e93dd 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -116,13 +116,30 @@ show_stash () {
>  	flags=$(git rev-parse --no-revs --flags "$@")
>  	if test -z "$flags"
>  	then
> -		flags=--stat
> +		diff_flags=--stat
> +	else
> +		diff_flags=$flags
> +	fi
> +	s=$(git rev-parse --revs-only --no-flags "$@")
> +	if test -z "$s"
> +	then
> +		arguments=$@
> +		if test "${flags}" = "${arguments}"
> +		then
> +			s=$(git rev-parse --revs-only --no-flags $ref_stash)
> +			if test -z "$s"
> +			then
> +				return 0
> +			fi
> +		else
> +			eval stash_name=\$$#
> +			die "Can't find any stash with name $stash_name"
> +		fi
>  	fi

Is it just me who feels that the added code is much worse than the
disease?

Re: [PATCH] Replace the cryptic messages from "git stash show".

[Jing Xue]

On Fri, Dec 14, 2007 at 10:12:32PM -0800, Junio C Hamano wrote:
> 
> Is it just me who feels that the added code is much worse than the
> disease?

I wouldn't be surprised. Shell scripting is by no means my long suit -
in fact, I am not even sure it's "a suit" of mine at all. I didn't like
what I saw and basically had two options:

1. Send a "this is not user friendly" rant and get beaten up by "why
don't you even try?"

2. Try and hack together something, submit it, and keep fixing it until
it makes it - and learn something in the process, maybe at the price of
being mocked at, but I don't care.

In particular, as far as I _could_ see, there are probably at least one
place I'm doing something potentially absurd, but couldn't figure out
anything better:

I'm not sure how to test if there is a stash name specified, so I
tried:

test "$flags" = "$@"

but then it breaks when $@ has a space in it. Hence the pointless
assignment to $arguments first.

The reason I removed the --default option to rev-parse is that it
doesn't distinguish between no stash name or an invalid one.

I'm sure there are other things people don't like - some because my
shell scripting sucks, some others maybe because of style differences.
At any rate, I'm open to criticism. So bring it on. 8-)
-- 
Jing Xue