Re: [PATCH] stash: treat numerical arguments as shorthand for stash@{n}

[Junio C Hamano <gitster@pobox.com>]

Peter Collingbourne <peter@pcc.me.uk> writes:

> This patch causes git-stash to treat any argument consisting of
> between one and three numerical digits as if it were of the form
> `stash@{<n>}`, where `<n>` is the argument supplied.

Inperative mood.

>
> This is a significant usability improvement for people dealing with
> multiple stashes, as it avoids redundantly typing 'stash@{' ... '}'
> (which involves shifted characters on most keyboards) in the very
> common case that the stash was created using git-stash.

Be less subjective by dropping "significant"; do not shove the
judgement down the throat of reviewers.

> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 711ffe1..8ffcc97 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -40,6 +40,10 @@ the usual reflog syntax (e.g. `stash@{0}` is the most recently
>  created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`
>  is also possible).
>  
> +Any argument supplied to this command consisting of between one
> +and three numerical digits is treated as if it were of the
> +form `stash@{<n>}`, where `<n>` is the argument supplied.

Why up to 999, not 99 or 9999?

How about doing it this way instead:

	if commit=$(git rev-parse --verify --quiet $arg^0)
	then
		: that is a commit-ish, even though it is 0123
	elif test "$arg" = 0 || expr "$arg" : '[1-9][0-9]*$' >/dev/null &&
	     commit=$(git rev-parse --verify --quiet "stash@{$arg}^0")
	then
		: $arg is decimal integer and stash@{$arg} is a commit-ish
	else
        	BAD
	fi

> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 5dfbda7..5467acf 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -38,7 +38,7 @@ test_expect_success 'parents of stash' '
>  '
>  
>  test_expect_success 'applying bogus stash does nothing' '
> -	test_must_fail git stash apply stash@{1} &&
> +	test_must_fail git stash apply 1 &&

If you are _adding_ a feature, do not remove tests for existing one;
otherwise you will risk masking a breakage you may be introducing to
an existing feature.  Add tests to check that (1) your new feature
works when it should, and (2) your new feature does *not* kick in
when it should not.  For example, if you only accept up to 3-digit
decimal integer, make sure feeding 0000 (or something that is *not*
3-digit decimal integer) does not trigger your new feature.

>  	echo 1 >expect &&
>  	test_cmp expect file
>  '
> @@ -113,7 +113,7 @@ test_expect_success 'drop middle stash' '
>  	git stash &&
>  	echo 9 > file &&
>  	git stash &&
> -	git stash drop stash@{1} &&
> +	git stash drop 1 &&

Likewise throughout the patch.