threads / patch / 20531

patchgit stash: Give friendlier error when there is nothing to apply

Subject: [PATCH] git stash: Give friendlier error when there is nothing to apply

## tl;dr

7 messages between Aug 11, 2009 and Aug 15, 2009. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Ori Avtalion· Aug 11, 2009, 11:12 UTC · lore
The old message was scary and included a 'fatal' error from rev-parse:
     fatal: Needed a single revision
     : no valid stashed state found

The new message is identical to the one given by 'git stash branch', and given *before* the check for a dirty working tree.

Previously, the command prompted the user to clean a dirty working tree when there is nothing to apply.

Signed-off-by: Ori Avtalion <ori@avtalion.name>
---
 git-stash.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to git-stash.sh +2 −0
diff --git a/git-stash.sh b/git-stash.sh
index 03e589f..aa84144 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -162,6 +162,8 @@ show_stash () {
 }
 
 apply_stash () {
+	have_stash || die 'Nothing to apply'
+
 	git update-index -q --refresh &&
 	git diff-files --quiet --ignore-submodules ||
 		die 'Cannot apply to a dirty working tree, please stage your changes'

-- 
1.6.4.73.gc144.dirty
Thomas Rast· Aug 11, 2009, 12:09 UTC · re: Ori Avtalion · lore

Re: [PATCH] git stash: Give friendlier error when there is nothing to apply

Ori Avtalion wrote:
Show 7 quoted lines
>  apply_stash () {
> +	have_stash || die 'Nothing to apply'
> +
>  	git update-index -q --refresh &&
>  	git diff-files --quiet --ignore-submodules ||
>  		die 'Cannot apply to a dirty working tree, please stage your changes'
> 
This needs a guard against the case where the user says
  git stash apply $some_sha1

but his refs/stash is empty. This could be the case, e.g., after mistakenly blowing away the reflog with 'git stash clear' and then going on a recovery hunt through the unreferenced commits.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Ori Avtalion· Aug 11, 2009, 11:12 UTC · re: Thomas Rast · lore

[PATCH] git stash: Give friendlier errors when there is nothing to apply

The change makes sure a stash (given or default) exists before checking if the working tree is dirty.

If the default stash is requested, the old message was scary and
included a 'fatal' error from rev-parse:
     fatal: Needed a single revision
     : no valid stashed state found

It is replaced with a friendlier 'Nothing to apply' error, similar to 'git stash branch'.

If a specific stash is specified, the 'Needed a single revision' errors from rev-parse are suppressed.

Signed-off-by: Ori Avtalion <ori@avtalion.name>
---

Thomas, I added handling for the 'git stash apply <stash>' case based on your reminder, and also changed the error messages related to it.

All of the stash tests pass, as before.
 git-stash.sh |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)
Show changes to git-stash.sh +16 −11
diff --git a/git-stash.sh b/git-stash.sh
index 03e589f..d61c9d0 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -162,10 +162,6 @@ show_stash () {
 }
 
 apply_stash () {
-	git update-index -q --refresh &&
-	git diff-files --quiet --ignore-submodules ||
-		die 'Cannot apply to a dirty working tree, please stage your changes'
-
 	unstash_index=
 
 	while test $# != 0
@@ -184,18 +180,27 @@ apply_stash () {
 		shift
 	done
 
-	# current index state
-	c_tree=$(git write-tree) ||
-		die 'Cannot apply a stash in the middle of a merge'
+	if test $# = 0
+	then
+		have_stash || die 'Nothing to apply'
+	fi
 
 	# stash records the work tree, and is a merge between the
 	# base commit (first parent) and the index tree (second parent).
-	s=$(git rev-parse --verify --default $ref_stash "$@") &&
-	w_tree=$(git rev-parse --verify "$s:") &&
-	b_tree=$(git rev-parse --verify "$s^1:") &&
-	i_tree=$(git rev-parse --verify "$s^2:") ||
+	s=$(git rev-parse --quiet --verify --default $ref_stash "$@") &&
+	w_tree=$(git rev-parse --quiet --verify "$s:") &&
+	b_tree=$(git rev-parse --quiet --verify "$s^1:") &&
+	i_tree=$(git rev-parse --quiet --verify "$s^2:") ||
 		die "$*: no valid stashed state found"
 
+	git update-index -q --refresh &&
+	git diff-files --quiet --ignore-submodules ||
+		die 'Cannot apply to a dirty working tree, please stage your changes'
+
+	# current index state
+	c_tree=$(git write-tree) ||
+		die 'Cannot apply a stash in the middle of a merge'
+
 	unstashed_index_tree=
 	if test -n "$unstash_index" && test "$b_tree" != "$i_tree" &&
 			test "$c_tree" != "$i_tree"

-- 
1.6.4.115.g82cf7
Thomas Rast· Aug 13, 2009, 07:35 UTC · re: Ori Avtalion · lore

Re: [PATCH] git stash: Give friendlier errors when there is nothing to apply

Ori Avtalion wrote:
Show 21 quoted lines
> The change makes sure a stash (given or default) exists before
> checking if the working tree is dirty.
> 
> If the default stash is requested, the old message was scary and
> included a 'fatal' error from rev-parse:
>      fatal: Needed a single revision
>      : no valid stashed state found
> 
> It is replaced with a friendlier 'Nothing to apply' error, similar to
> 'git stash branch'.
> 
> If a specific stash is specified, the 'Needed a single revision' errors
> from rev-parse are suppressed.
> 
> Signed-off-by: Ori Avtalion <ori@avtalion.name>
> ---
> 
> Thomas, I added handling for the 'git stash apply <stash>' case based
> on your reminder, and also changed the error messages related to it.
> 
> All of the stash tests pass, as before.
Acked-by: Thomas Rast <trast@student.ethz.ch>
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Junio C Hamano· Aug 14, 2009, 19:52 UTC · re: Ori Avtalion · lore

Re: [PATCH] git stash: Give friendlier errors when there is nothing to apply

Ori Avtalion <ori@avtalion.name> writes:
Show 16 quoted lines
> The change makes sure a stash (given or default) exists before
> checking if the working tree is dirty.
>
> If the default stash is requested, the old message was scary and
> included a 'fatal' error from rev-parse:
>      fatal: Needed a single revision
>      : no valid stashed state found
>
> It is replaced with a friendlier 'Nothing to apply' error, similar to
> 'git stash branch'.
>
> If a specific stash is specified, the 'Needed a single revision' errors
> from rev-parse are suppressed.
>
> Signed-off-by: Ori Avtalion <ori@avtalion.name>
> Acked-by: Thomas Rast <trast@student.ethz.ch>

I do not see anything that might break existing usage of the command. Comments?

A tangent; we might want an analogue to "shortlog -s -n" but based on "blame".

Nanako Shiraishi· Aug 14, 2009, 21:39 UTC · re: Junio C Hamano · lore

Re: [PATCH] git stash: Give friendlier errors when there is nothing to apply

Quoting Junio C Hamano <gitster@pobox.com>
Show 21 quoted lines
> Ori Avtalion <ori@avtalion.name> writes:
>
>> The change makes sure a stash (given or default) exists before
>> checking if the working tree is dirty.
>>
>> If the default stash is requested, the old message was scary and
>> included a 'fatal' error from rev-parse:
>>      fatal: Needed a single revision
>>      : no valid stashed state found
>>
>> It is replaced with a friendlier 'Nothing to apply' error, similar to
>> 'git stash branch'.
>>
>> If a specific stash is specified, the 'Needed a single revision' errors
>> from rev-parse are suppressed.
>>
>> Signed-off-by: Ori Avtalion <ori@avtalion.name>
>> Acked-by: Thomas Rast <trast@student.ethz.ch>
>
> I do not see anything that might break existing usage of the command.
> Comments?

The patch looks good to me. I think it was my fault but you can avoid adding two extra --quiet by inspecting i_tree (in other words, "$s^2:") before w_tree and b_tree. Because a commit that is a stash exactly has two parents, it may be good to also make sure 'git rev-parse "$s^3"' fails.

	s=$(git rev-parse --verify --default $ref_stash "$@") &&
	! git rev-parse --quiet --verify "$s^3" >/dev/null &&
	i_tree=$(git rev-parse --quiet --verify "$s^2:") &&
	b_tree=$(git rev-parse --verify "$s^1:") &&
	w_tree=$(git rev-parse --verify "$s:") ||
		die "$*: no valid stashed state found"
> A tangent; we might want an analogue to "shortlog -s -n" but based on
> "blame".
I'm sorry, I don't understand what you mean.
-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
Junio C Hamano· Aug 15, 2009, 01:50 UTC · re: Nanako Shiraishi · lore

Re: [PATCH] git stash: Give friendlier errors when there is nothing to apply

Nanako Shiraishi <nanako3@lavabit.com> writes:
Show 6 quoted lines
> Quoting Junio C Hamano <gitster@pobox.com>
> ...
>> A tangent; we might want an analogue to "shortlog -s -n" but based on
>> "blame".
>
> I'm sorry, I don't understand what you mean.

I often use shortlog to find whom to CC to, but sometimes blame output gives us a much better picture.

$ git shortlog -s -n git-stash.sh | head -n 6
    22	Junio C Hamano
     4	Brandon Casey
     2	Jeff King
     2	Johannes Schindelin
     2	Nanako Shiraishi
     2	SZEDER Gábor
$ git blame -w git-stash.sh |
  sed -e 's/^[0-9a-f]* (\(.*\) *200[7-9]-..-.. .*/\1/' |
  sort | uniq -c | sort -n -r | head -n 6
    110 Nanako Shiraishi    
     86 Junio C Hamano      
     55 Stephen Boyd        
     33 Brandon Casey       
     20 Abhijit Menon-Sen   
     16 Johannes Schindelin

← back to recent threads