threads / patch / 11279

patchMakes 'git-stash show' stay quiet when there are no stashes.

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

## tl;dr

5 messages between Dec 14, 2007 and Dec 15, 2007. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Jing Xue· Dec 14, 2007, 01:28 UTC · lore

(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(-)

Show changes to git-stash.sh +4 −0

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

Junio C Hamano· Dec 14, 2007, 01:34 UTC · re: Jing Xue · lore

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

Jing Xue <jingxue@digizenstudio.com> writes:

Show 7 quoted lines

> (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").

Jing Xue· Dec 15, 2007, 05:14 UTC · re: Junio C Hamano · lore

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

On Thu, Dec 13, 2007 at 05:34:40PM -0800, Junio C Hamano wrote:

Show 5 quoted lines

> 
> 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(-)

Show changes to git-stash.sh +20 −3

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

Junio C Hamano· Dec 15, 2007, 06:12 UTC · re: Jing Xue · lore

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

Jing Xue <jingxue@digizenstudio.com> writes:

Show 49 quoted lines

> 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?

Jing Xue· Dec 15, 2007, 17:08 UTC · re: Junio C Hamano · lore

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

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