Subject: Re: [PATCH] stash: allow ref of a stash by index

From: Junio C Hamano <[email-masked]>
Date: Wed, 07 Sep 2016 10:50:09 -0700

Øystein Walle <[email-masked]> writes:

> diff --git a/git-stash.sh b/git-stash.sh
> index 826af18..b026288 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -384,7 +384,7 @@ parse_flags_and_rev()
>      i_tree=
>      u_tree=
>
> -    REV=$(git rev-parse --no-flags --symbolic --sq "$@") || exit 1
> +    REV=$(git rev-parse --no-flags --symbolic --sq "$@" 2>/dev/null)
>
>      FLAGS=
>      for opt
> @@ -422,6 +422,15 @@ parse_flags_and_rev()
>          ;;
>      esac
>
> +    case "$1" in
> +        *[!0-9]*)
> +            :

OK, so you ignore anything that has a non-digit here, to ensure that...

> +        ;;
> +        *)
> +            set -- "${ref_stash}@{$1}"

... this one triggers only for a string $1 that consists solely of
digits.

> +        ;;
> +    esac

Makes sense.  I notice that both of the two existing case/esac
statements in this function indent case arms and their bodies one
level too deep, which you follwed with the above addition.  That may
be something we would want to fix in a follow-up patch.

>      REV=$(git rev-parse --symbolic --verify --quiet "$1") || {
>          reference="$1"
>          die "$(eval_gettext "\$reference is not a valid reference")"

I agree with Peff that the change in error message he noticed is
probably an improvement ;-)

Want to do a final log message to make it a real patch?

Thanks.