threads / patch / 11479

patchgit-stash: add new 'drop' subcommand

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

## tl;dr

8 messages between Jan 5, 2008 and Jan 5, 2008. Diffs are folded; open one to read it.

replies: 7people: 5as markdown or json

Brandon Casey· Jan 5, 2008, 01:31 UTC · lore

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(-)
Show changes to 2 files +34 −2

Documentation/git-stash.txt, git-stash.sh

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
Junio C Hamano· Jan 5, 2008, 01:47 UTC · re: Brandon Casey · lore

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

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?

Show 10 quoted lines
> 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.
Show 15 quoted lines
> +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"
> +}
Show 8 quoted lines
> +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.
Brandon Casey· Jan 5, 2008, 05:46 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster <at> pobox.com> writes:
Show 9 quoted lines
> 
> 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.
Show 5 quoted lines
> > 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.
Show 8 quoted lines
> > +		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
Jeff King· Jan 5, 2008, 03:51 UTC · re: Brandon Casey · lore

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

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
JM Ibanez· Jan 5, 2008, 09:26 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:
Show 10 quoted lines
> 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/
Junio C Hamano· Jan 5, 2008, 09:35 UTC · re: JM Ibanez · lore

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

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:
Show 11 quoted lines
> 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(-)
Show changes to git-stash.sh +5 −1
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
Jeff King· Jan 5, 2008, 10:00 UTC · re: Junio C Hamano · lore

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

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
Marco Costalba· Jan 5, 2008, 12:36 UTC · re: Brandon Casey · lore

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

On Jan 5, 2008 2:31 AM, Brandon Casey <casey@nrlssc.navy.mil> wrote:
Show 8 quoted lines
>
>
> +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

← back to recent threads