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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 7, 2016, 17:50 UTC
Message-ID: <xmqqbmzzoazy.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAFaJEqu-JUcwLjrQBk_huSa3DZfCf8O4eAZ=UgcXHzN=CLgtpw@mail.gmail.com>

Øystein Walle <oystwa@gmail.com> writes:

Show 20 quoted lines

> 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.