From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 05 Jan 2008 01:47:43 GMT
Subject: Re: [PATCH] git-stash: add new 'drop' subcommand
Message-ID: <7vtzltf3gg.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <477EDDD4.5060509@nrlssc.navy.mil>

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.