[PATCH] git-stash: add new 'drop' subcommand

[PATCH] git-stash: add new 'drop' subcommand

[Brandon Casey]

This allows a single stash entry to be deleted. It takes an
optional argument which is a stash reflog entry. If no
arguments are supplied, stash@{0} is used.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


Thus far I haven't been a big user of git stash, but I plan to
use it more and I expect to use 'drop' more often than
'clear'. I expect in the common case there will be a single
stash, and 'drop' will be sufficient. For the case where there
are many stashes and I want to remove one, 'drop' is required.
'git stash clear' will become a command that I give special
attention to just like 'rm -f *'.

I'm not sure if there is a proper way to get 'stash@{0}' from
'refs/stash' so I kept my usage of that former string outside
of the drop_stash() function.

Comments welcome, especially if there is a more appropriate
way to do this.

-brandon


 Documentation/git-stash.txt |    7 ++++++-
 git-stash.sh                |   29 ++++++++++++++++++++++++++++-
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index c0147b9..b89eadb 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -8,7 +8,7 @@ git-stash - Stash the changes in a dirty working directory away
 SYNOPSIS
 --------
 [verse]
-'git-stash' (list | show [<stash>] | apply [<stash>] | clear)
+'git-stash' (list | show [<stash>] | apply [<stash>] | clear | drop [<stash>])
 'git-stash' [save] [message...]
 
 DESCRIPTION
@@ -81,6 +81,11 @@ clear::
 	Remove all the stashed states. Note that those states will then
 	be subject to pruning, and may be difficult or impossible to recover.
 
+drop [<stash>]::
+
+	Remove a single stashed state from the stash list. When no `<stash>`
+	is given, it removes the latest one. i.e. `stash@\{0}`
+
 
 DISCUSSION
 ----------
diff --git a/git-stash.sh b/git-stash.sh
index 06cb177..a789a53 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # Copyright (c) 2007, Nanako Shiraishi
 
-USAGE='[  | save | list | show | apply | clear | create ]'
+USAGE='[  | save | list | show | apply | clear | create | drop ]'
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
@@ -192,6 +192,24 @@ apply_stash () {
 	fi
 }
 
+drop_stash () {
+	if ! have_stash
+	then
+		echo >&2 'No stash entries to drop'
+		exit 0
+	fi
+
+	# Verify supplied argument looks like a stash entry
+	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
+	git rev-parse --verify "$s:"   > /dev/null 2>&1 &&
+	git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
+	git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
+		die "$*: not a valid stashed state"
+
+	git reflog delete "$@" && echo "Dropped $@ ($s)" ||
+		die "$*: Could not drop stash entry"
+}
+
 # Main command set
 case "$1" in
 list)
@@ -225,6 +243,15 @@ create)
 	fi
 	create_stash "$*" && echo "$w_commit"
 	;;
+drop)
+	shift
+	if test $# = 0
+	then
+		set -- "stash@{0}"
+	fi
+	drop_stash "$@" &&
+	(git rev-parse --verify "stash@{0}" > /dev/null 2>&1 || clear_stash)
+	;;
 *)
 	if test $# -eq 0
 	then
-- 
1.5.4.rc2.1119.g6fdf-dirty

Re: [PATCH] git-stash: add new 'drop' subcommand

[Junio C Hamano]

Brandon Casey <casey@nrlssc.navy.mil> writes:

> I'm not sure if there is a proper way to get 'stash@{0}' from
> 'refs/stash' so I kept my usage of that former string outside
> of the drop_stash() function.

Doesn't "$refs_stash@{0}" (which would give refs/stash@{0} not
stash@{0}) work for you?

> diff --git a/git-stash.sh b/git-stash.sh
> index 06cb177..a789a53 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -1,7 +1,7 @@
>  #!/bin/sh
>  # Copyright (c) 2007, Nanako Shiraishi
>  
> -USAGE='[  | save | list | show | apply | clear | create ]'
> +USAGE='[  | save | list | show | apply | clear | create | drop ]'

Might want to put drop next to clear, but that is minor.

> +drop_stash () {
> +	if ! have_stash
> +	then
> +		echo >&2 'No stash entries to drop'
> +		exit 0
> +	fi
> +
> +	# Verify supplied argument looks like a stash entry
> +	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
> +	git rev-parse --verify "$s:"   > /dev/null 2>&1 &&
> +	git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
> +	git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
> +		die "$*: not a valid stashed state"
> +
> +	git reflog delete "$@" && echo "Dropped $@ ($s)" ||

The second $@ is inconsistent with the next line's use of $*; intentional?

> +		die "$*: Could not drop stash entry"
> +}

> +drop)
> +	shift
> +	if test $# = 0
> +	then
> +		set -- "stash@{0}"
> +	fi
> +	drop_stash "$@" &&
> +	(git rev-parse --verify "stash@{0}" > /dev/null 2>&1 || clear_stash)

Curious.

 (1) Why not do the clearing inside drop_stash?

 (2) Why is clearning necessary in the first place (iow,
     shouldn't "reflog delete" take care of that)?

Other than that, nicely done.

Re: [PATCH] git-stash: add new 'drop' subcommand

[Brandon Casey]

Junio C Hamano <gitster <at> pobox.com> writes:

> 
> Brandon Casey <casey <at> nrlssc.navy.mil> writes:
> 
> > I'm not sure if there is a proper way to get 'stash@{0}' from
> > 'refs/stash' so I kept my usage of that former string outside
> > of the drop_stash() function.
> 
> Doesn't "$refs_stash@{0}" (which would give refs/stash@{0} not
> stash@{0}) work for you?

yep that works. much nicer.

> > diff --git a/git-stash.sh b/git-stash.sh
> > -USAGE='[  | save | list | show | apply | clear | create ]'
> > +USAGE='[  | save | list | show | apply | clear | create | drop ]'
> 
> Might want to put drop next to clear, but that is minor.

no problem.


> > +	git reflog delete "$@" && echo "Dropped $@ ($s)" ||
> 
> The second $@ is inconsistent with the next line's use of $*; intentional?

not intentional.

> > +		set -- "stash@{0}"
> > +	fi
> > +	drop_stash "$@" &&
> > +	(git rev-parse --verify "stash@{0}" > /dev/null 2>&1 || clear_stash)
> 
> Curious.
> 
>  (1) Why not do the clearing inside drop_stash?

only because I didn't like stash@{0} notation and didn't want it
buried inside a function.

> 
>  (2) Why is clearning necessary in the first place (iow,
>      shouldn't "reflog delete" take care of that)?

clear_stash additionally deletes refs/stash and logs/refs/stash at least.

-brandon

Re: [PATCH] git-stash: add new 'drop' subcommand

[Jeff King]

On Fri, Jan 04, 2008 at 07:31:00PM -0600, Brandon Casey wrote:

> Thus far I haven't been a big user of git stash, but I plan to
> use it more and I expect to use 'drop' more often than
> 'clear'. I expect in the common case there will be a single

There was some discussion of a sensible name, but I don't recall seeing
a resolution on this: why not "clear stash@{0}" to clear one, and
"clear" to clear all? Otherwise, I foresee "git stash clear stash@{0}"
followed by "oops, I just deleted all of my stashes."

I guess you get "git stash drop" as a synonym for "git stash drop
stash@{0}" this way, but it just seems mean to users to make them
remember which of "drop" and "clear" does what they want.

-Peff

Re: [PATCH] git-stash: add new 'drop' subcommand

[JM Ibanez]

Jeff King <peff@peff.net> writes:

> On Fri, Jan 04, 2008 at 07:31:00PM -0600, Brandon Casey wrote:
>
>> Thus far I haven't been a big user of git stash, but I plan to
>> use it more and I expect to use 'drop' more often than
>> 'clear'. I expect in the common case there will be a single
>
> There was some discussion of a sensible name, but I don't recall seeing
> a resolution on this: why not "clear stash@{0}" to clear one, and
> "clear" to clear all? Otherwise, I foresee "git stash clear stash@{0}"
> followed by "oops, I just deleted all of my stashes."

I actually got hit by this. I didn't know that stash clear affected all
stashes and lost quite a bit of work that way (I use stash to store
various test database configs for a tree I work with, and so lost all of
them when trying to remove one particular stash).

> I guess you get "git stash drop" as a synonym for "git stash drop
> stash@{0}" this way, but it just seems mean to users to make them
> remember which of "drop" and "clear" does what they want.

I have to agree with this.

-- 
JM Ibanez
Software Architect
Orange & Bronze Software Labs, Ltd. Co.

jm@orangeandbronze.com
http://software.orangeandbronze.com/

[PATCH] git-stash clear: refuse to work with extra parameter for now

[Junio C Hamano]

Because it is so tempting to expect "git stash clear stash@{4}"
to remove the fourth element in the stash while leaving other
elements intact, we should not blindly throw away everything.

This may change when we start using "git reflog delete" to
selectively nuke a single (or multiple, for that matter) stash
entries when such a command is given.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

JM Ibanez <jm@orangeandbronze.com> writes:

> Jeff King <peff@peff.net> writes:
> ...
>> There was some discussion of a sensible name, but I don't recall seeing
>> a resolution on this: why not "clear stash@{0}" to clear one, and
>> "clear" to clear all? Otherwise, I foresee "git stash clear stash@{0}"
>> followed by "oops, I just deleted all of my stashes."
>
> I actually got hit by this. I didn't know that stash clear affected all
> stashes and lost quite a bit of work that way (I use stash to store
> various test database configs for a tree I work with, and so lost all of
> them when trying to remove one particular stash).

I think something along this line may be necessary to
futureproof our users. 

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

diff --git a/git-stash.sh b/git-stash.sh
index 06cb177..80036ef 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -20,6 +20,10 @@ no_changes () {
 }
 
 clear_stash () {
+	if test $# != 0
+	then
+		die "git stash clear with parameters unimplemented $@"
+	fi
 	if current=$(git rev-parse --verify $ref_stash 2>/dev/null)
 	then
 		git update-ref -d $ref_stash $current
@@ -216,7 +220,7 @@ apply)
 	apply_stash "$@"
 	;;
 clear)
-	clear_stash
+	clear_stash "$@"
 	;;
 create)
 	if test $# -gt 0 && test "$1" = create

Re: [PATCH] git-stash clear: refuse to work with extra parameter for now

[Jeff King]

On Sat, Jan 05, 2008 at 01:35:54AM -0800, Junio C Hamano wrote:

> I think something along this line may be necessary to
> futureproof our users. 

I think it not only futureproofs, but it helps those who misunderstand
(or don't read) the documentation from accidentally losing work.

Acked-by: Jeff King <peff@peff.net>

-Peff

Re: [PATCH] git-stash: add new 'drop' subcommand

[Marco Costalba]

On Jan 5, 2008 2:31 AM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
>
>
> +drop_stash () {
> +       if ! have_stash
> +       then
> +               echo >&2 'No stash entries to drop'
> +               exit 0
> +       fi

Please, or

> +               echo >&2 'No stash entries to drop'
> +               exit 1

or

> +               echo 'No stash entries to drop'
> +               exit 0

I would prefer the latter.

Thanks
Marco