[PATCH 1/1] Improve error messages from 'git stash show'

[PATCH 1/1] Improve error messages from 'git stash show'

[Gustaf Hendeby]

The previous error message "fatal: Needed a single revision" is not
very informative.

Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---
 git-stash.sh |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index aa47e54..cf221c6 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -210,14 +210,18 @@ list_stash () {
 }
 
 show_stash () {
+	have_stash || die 'No stash found'
+
 	flags=$(git rev-parse --no-revs --flags "$@")
 	if test -z "$flags"
 	then
 		flags=--stat
 	fi
 
-	w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
-	b_commit=$(git rev-parse --verify "$w_commit^") &&
+	w_commit=$(git rev-parse --quiet --verify --default $ref_stash "$@") &&
+	b_commit=$(git rev-parse --quiet --verify "$w_commit^") ||
+		die 'Stash not found'
+
 	git diff $flags $b_commit $w_commit
 }
 
-- 
1.7.0.2.274.gc376e.dirty

Re: [PATCH 1/1] Improve error messages from 'git stash show'

[Junio C Hamano]

Gustaf Hendeby <hendeby@isy.liu.se> writes:

> The previous error message "fatal: Needed a single revision" is not
> very informative.
>
> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
> ---
>  git-stash.sh |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index aa47e54..cf221c6 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -210,14 +210,18 @@ list_stash () {
>  }
>  
>  show_stash () {
> +	have_stash || die 'No stash found'
> +
>  	flags=$(git rev-parse --no-revs --flags "$@")
>  	if test -z "$flags"
>  	then
>  		flags=--stat
>  	fi
>  
> -	w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
> -	b_commit=$(git rev-parse --verify "$w_commit^") &&
> +	w_commit=$(git rev-parse --quiet --verify --default $ref_stash "$@") &&
> +	b_commit=$(git rev-parse --quiet --verify "$w_commit^") ||
> +		die 'Stash not found'
> +

Because you checked have_stash upfront, an error detected here cannot be
"stash not found".  It is something else, isn't it?

>  	git diff $flags $b_commit $w_commit
>  }
>  
> -- 
> 1.7.0.2.274.gc376e.dirty

Re: [PATCH 1/1] Improve error messages from 'git stash show'

[Gustaf Hendeby]

On 03/16/2010 12:17 AM, Junio C Hamano wrote:
> Gustaf Hendeby <hendeby@isy.liu.se> writes:
> 
>> The previous error message "fatal: Needed a single revision" is not
>> very informative.
>>
>> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
>> ---
>>  git-stash.sh |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-stash.sh b/git-stash.sh
>> index aa47e54..cf221c6 100755
>> --- a/git-stash.sh
>> +++ b/git-stash.sh
>> @@ -210,14 +210,18 @@ list_stash () {
>>  }
>>  
>>  show_stash () {
>> +	have_stash || die 'No stash found'
>> +
>>  	flags=$(git rev-parse --no-revs --flags "$@")
>>  	if test -z "$flags"
>>  	then
>>  		flags=--stat
>>  	fi
>>  
>> -	w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
>> -	b_commit=$(git rev-parse --verify "$w_commit^") &&
>> +	w_commit=$(git rev-parse --quiet --verify --default $ref_stash "$@") &&
>> +	b_commit=$(git rev-parse --quiet --verify "$w_commit^") ||
>> +		die 'Stash not found'
>> +
> 
> Because you checked have_stash upfront, an error detected here cannot be
> "stash not found".  It is something else, isn't it?

This case occurs when you do something like git stash foo, where foo is
no appropriate stash to show.  Would something like this be more
informative in the case of git stash foo:

Stash entry 'foo' could not be found

/Gustaf

Re: [PATCH 1/1] Improve error messages from 'git stash show'

[Junio C Hamano]

Gustaf Hendeby <hendeby@isy.liu.se> writes:

> On 03/16/2010 12:17 AM, Junio C Hamano wrote:
>> Gustaf Hendeby <hendeby@isy.liu.se> writes:
>> 
>>> The previous error message "fatal: Needed a single revision" is not
>>> very informative.
>>>
>>> Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
>>> ---
>>>  git-stash.sh |    8 ++++++--
>>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/git-stash.sh b/git-stash.sh
>>> index aa47e54..cf221c6 100755
>>> --- a/git-stash.sh
>>> +++ b/git-stash.sh
>>> @@ -210,14 +210,18 @@ list_stash () {
>>>  }
>>>  
>>>  show_stash () {
>>> +	have_stash || die 'No stash found'
>>> +
>>>  	flags=$(git rev-parse --no-revs --flags "$@")
>>>  	if test -z "$flags"
>>>  	then
>>>  		flags=--stat
>>>  	fi
>>>  
>>> -	w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
>>> -	b_commit=$(git rev-parse --verify "$w_commit^") &&
>>> +	w_commit=$(git rev-parse --quiet --verify --default $ref_stash "$@") &&
>>> +	b_commit=$(git rev-parse --quiet --verify "$w_commit^") ||
>>> +		die 'Stash not found'
>>> +
>> 
>> Because you checked have_stash upfront, an error detected here cannot be
>> "stash not found".  It is something else, isn't it?
>
> This case occurs when you do something like git stash foo, where foo is
> no appropriate stash to show.  Would something like this be more
> informative in the case of git stash foo:
>
> Stash entry 'foo' could not be found

That is more like "'foo' is not a stash", isn't it?

[PATCH v2] Improve error messages from 'git stash show'

[Gustaf Hendeby]

The previous error message "fatal: Needed a single revision" is not
very informative.

Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---

On 03/16/2010 05:53 PM, Junio C Hamano wrote:
>>>> +	b_commit=$(git rev-parse --quiet --verify "$w_commit^") ||
>>>> >>>  +		die 'Stash not found'
>>>> >>>  +
>>> >>  
>>> >>  Because you checked have_stash upfront, an error detected here cannot be
>>> >>  "stash not found".  It is something else, isn't it?
>> >
>> >  This case occurs when you do something like git stash foo, where foo is
>> >  no appropriate stash to show.  Would something like this be more
>> >  informative in the case of git stash foo:
>> >
>> >  Stash entry 'foo' could not be found
> That is more like "'foo' is not a stash", isn't it?

I find your suggestion much clearer.  Thank you!

/Gustaf

 git-stash.sh |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index aa47e54..59db3dc 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -210,14 +210,18 @@ list_stash () {
 }
 
 show_stash () {
+	have_stash || die 'No stash found'
+
 	flags=$(git rev-parse --no-revs --flags "$@")
 	if test -z "$flags"
 	then
 		flags=--stat
 	fi
 
-	w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
-	b_commit=$(git rev-parse --verify "$w_commit^") &&
+	w_commit=$(git rev-parse --quiet --verify --default $ref_stash "$@") &&
+	b_commit=$(git rev-parse --quiet --verify "$w_commit^") ||
+		die "'$*' is not a stash"
+
 	git diff $flags $b_commit $w_commit
 }
 
-- 
1.7.0.2.274.gba5d.dirty