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

2012-12-25 Thread Peter Collingbourne 
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.

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.

Signed-off-by: Peter Collingbourne pe...@pcc.me.uk
---
 Documentation/git-stash.txt |4 
 git-stash.sh|   15 ++-
 t/t3903-stash.sh|   18 +-
 3 files changed, 27 insertions(+), 10 deletions(-)

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.
+
 OPTIONS
 ---
 
diff --git a/git-stash.sh b/git-stash.sh
index bbefdf6..2232719 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -327,7 +327,20 @@ parse_flags_and_rev()
i_tree=
u_tree=
 
-   REV=$(git rev-parse --no-flags --symbolic $@) || exit 1
+   ARGS=
+   for arg
+   do
+   case $arg in
+   [0-9]|[0-9][0-9]|[0-9][0-9][0-9])
+   ARGS=${ARGS}${ARGS:+ }${ref_stash}@{$arg}
+   ;;
+   *)
+   ARGS=${ARGS}${ARGS:+ }$arg
+   ;;
+   esac
+   done
+
+   REV=$(git rev-parse --no-flags --symbolic $ARGS) || exit 1
 
FLAGS=
for opt
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 
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 
test 2 = $(git stash list | wc -l) 
git stash apply 
test 9 = $(cat file) 
@@ -570,16 +570,16 @@ test_expect_success 'ref with non-existent reflog' '
 
 test_expect_success 'invalid ref of the form stash@{n}, n = N' '
git stash clear 
-   test_must_fail git stash drop stash@{0} 
+   test_must_fail git stash drop 0 
echo bar5  file 
echo bar6  file2 
git add file2 
git stash 
-   test_must_fail git stash drop stash@{1} 
-   test_must_fail git stash pop stash@{1} 
-   test_must_fail git stash apply stash@{1} 
-   test_must_fail git stash show stash@{1} 
-   test_must_fail git stash branch tmp stash@{1} 
+   test_must_fail git stash drop 1 
+   test_must_fail git stash pop 1 
+   test_must_fail git stash apply 1 
+   test_must_fail git stash show 1 
+   test_must_fail git stash branch tmp 1 
git stash drop
 '
 
@@ -590,7 +590,7 @@ test_expect_success 'stash branch should not drop the stash 
if the branch exists
git commit -m initial 
echo bar file 
git stash 
-   test_must_fail git stash branch master stash@{0} 
+   test_must_fail git stash branch master 0 
git rev-parse stash@{0} --
 '
 
-- 
1.7.5.3

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-12-25 Thread Junio C Hamano 
Peter Collingbourne pe...@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 ?

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  (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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html