Re: [PATCH v2 2/3] stash: Allow git stash branch to process commits that look like stashes but are not stash references.

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 4, 2010, 23:51 UTC
Message-ID: <7vwrs6djie.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1280831775-30759-3-git-send-email-jon.seymour@gmail.com>

Jon Seymour <jon.seymour@gmail.com> writes:

Show 9 quoted lines

> This patch allows git stash branch to work with stash-like commits created by git stash create.
>
> Two changes were required:
>
> * relax the pre-condition so that a stash stack is required if and only if a stash argument is not specified
> * don't attempt to drop a stash argument that doesn't look like a stash reference.
>
>
> Signed-off-by: Jon Seymour <jon.seymour@gmail.com>

Please wrap very long lines.

Show 13 quoted lines

> diff --git a/git-stash.sh b/git-stash.sh
> index 1d95447..432ddae 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -225,6 +225,12 @@ show_stash () {
>  	git diff $flags $b_commit $w_commit
>  }
>  
> +if_stash_ref() {
> +	ref="$1"
> +	shift
> +	test "${ref#stash}" = "${ref}" -a "${ref#$ref_stash}" = "${ref}" || "$@"
> +}

The interface to this function looks a rather bad taste to me; wouldn't it look more natural if the callers can say:

	if stash_ref $it
        then
        	do this
	fi

Your criteria used here is that the given parameter does not begin with "stash" nor "refs/stash". If it begins with either of these two strings, the "test" fails and "$@" is run. Wouldn't this produce a false hit if you kept a handcrafted stash-looking commit with a tag "stash-42" or something?

It may make more sense to give "stash drop" an option to be silent if the given parameter is not on the list to begin with, perhaps?