{"thread":{"id":"44045","subject":"[PATCH] Allow stashes to be referenced by index only","startedAt":"2016-09-08T23:47:18Z","lastAt":"2016-10-26T00:12:25Z","messageCount":8,"participants":["Aaron M Watson","Jeff King","Aaron and Ashley Watson","Junio C Hamano","Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"301462","messageId":"1473378397-22453-1-git-send-email-watsona4@gmail.com","threadId":"44045","inReplyTo":null,"subject":"[PATCH] Allow stashes to be referenced by index only","fromName":"Aaron M Watson","fromEmail":"watsona4@gmail.com","sentAt":"2016-09-08T23:46:37Z","receivedAt":"2016-09-08T23:47:18Z","isPatch":true,"sender":{"key":"watsona4@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5954119?v=4"},"body":"Instead of referencing \"stash@{n}\" explicitly, it can simply be\nreferenced as \"n\".  Most users only reference stashes by their position\nin the stash stask (what I refer to as the \"index\"). The syntax for the\ntypical stash (stash@{n}) is slightly annoying and easy to forget, and\nsometimes difficult to escape properly in a script. Because of this the\ncapability to do things with the stash by simply referencing the index\nis desirable.\n\nThis patch includes the superior implementation provided by Øsse Walle\n(thanks for that), with a slight change to fix a broken test in the test\nsuite. I also merged the test scripts as suggested by Jeff King, and\nun-wrapped the documentation as suggested by Junio Hamano.\n\nSigned-off-by: Aaron M Watson <watsona4@gmail.com>\n---\n Documentation/git-stash.txt |  3 ++-\n git-stash.sh                | 17 +++++++++++++++--\n t/t3903-stash.sh            | 35 +++++++++++++++++++++++++++++++++++\n 3 files changed, 52 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 92df596..2e9cef0 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -39,7 +39,8 @@ The latest stash you created is stored in `refs/stash`; older\n stashes are found in the reflog of this reference and can be named using\n the usual reflog syntax (e.g. `stash@{0}` is the most recently\n created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`\n-is also possible).\n+is also possible). Stashes may also be referenced by specifying just the\n+stash index (e.g. the integer `n` is equivalent to `stash@{n}`).\n \n OPTIONS\n -------\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 826af18..d8d3b8d 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -384,9 +384,10 @@ parse_flags_and_rev()\n \ti_tree=\n \tu_tree=\n \n-\tREV=$(git rev-parse --no-flags --symbolic --sq \"$@\") || exit 1\n+\tREV=$(git rev-parse --no-flags --symbolic --sq \"$@\" 2> /dev/null)\n \n \tFLAGS=\n+\tARGV=\n \tfor opt\n \tdo\n \t\tcase \"$opt\" in\n@@ -404,10 +405,13 @@ parse_flags_and_rev()\n \t\t\t\t\tdie \"$(eval_gettext \"unknown option: \\$opt\")\"\n \t\t\t\tFLAGS=\"${FLAGS}${FLAGS:+ }$opt\"\n \t\t\t;;\n+\t\t\t*)\n+\t\t\t\tARGV=\"${ARGV}${ARGV:+ }'$opt'\"\n+\t\t\t;;\n \t\tesac\n \tdone\n \n-\teval set -- $REV\n+\teval set -- $ARGV\n \n \tcase $# in\n \t\t0)\n@@ -422,6 +426,15 @@ parse_flags_and_rev()\n \t\t;;\n \tesac\n \n+\tcase \"$1\" in\n+\t\t*[!0-9]*)\n+\t\t\t:\n+\t\t;;\n+\t\t*)\n+\t\t\tset -- \"${ref_stash}@{$1}\"\n+\t\t;;\n+\tesac\n+\n \tREV=$(git rev-parse --symbolic --verify --quiet \"$1\") || {\n \t\treference=\"$1\"\n \t\tdie \"$(eval_gettext \"\\$reference is not a valid reference\")\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 2142c1f..f82a8c4 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -131,6 +131,26 @@ test_expect_success 'drop middle stash' '\n \ttest 1 = $(git show HEAD:file)\n '\n \n+test_expect_success 'drop middle stash by index' '\n+\tgit reset --hard &&\n+\techo 8 > file &&\n+\tgit stash &&\n+\techo 9 > file &&\n+\tgit stash &&\n+\tgit stash drop 1 &&\n+\ttest 2 = $(git stash list | wc -l) &&\n+\tgit stash apply &&\n+\ttest 9 = $(cat file) &&\n+\ttest 1 = $(git show :file) &&\n+\ttest 1 = $(git show HEAD:file) &&\n+\tgit reset --hard &&\n+\tgit stash drop &&\n+\tgit stash apply &&\n+\ttest 3 = $(cat file) &&\n+\ttest 1 = $(git show :file) &&\n+\ttest 1 = $(git show HEAD:file)\n+'\n+\n test_expect_success 'stash pop' '\n \tgit reset --hard &&\n \tgit stash pop &&\n@@ -604,6 +624,21 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '\n \tgit stash drop\n '\n \n+test_expect_success 'invalid ref of the form \"n\", n >= N' '\n+\tgit stash clear &&\n+\ttest_must_fail git stash drop 0 &&\n+\techo bar5 > file &&\n+\techo bar6 > file2 &&\n+\tgit add file2 &&\n+\tgit stash &&\n+\ttest_must_fail git stash drop 1 &&\n+\ttest_must_fail git stash pop 1 &&\n+\ttest_must_fail git stash apply 1 &&\n+\ttest_must_fail git stash show 1 &&\n+\ttest_must_fail git stash branch tmp 1 &&\n+\tgit stash drop\n+'\n+\n test_expect_success 'stash branch should not drop the stash if the branch exists' '\n \tgit stash clear &&\n \techo foo >file &&\n-- \n2.7.4\n\n"},{"id":"304727","messageId":"20161023090207.sg52l4mijqtfvip6@sigill.intra.peff.net","threadId":"44045","inReplyTo":"1473378397-22453-1-git-send-email-watsona4@gmail.com","subject":"Re: [PATCH] Allow stashes to be referenced by index only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-23T09:02:07Z","receivedAt":"2016-10-23T09:02:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 08, 2016 at 07:46:37PM -0400, Aaron M Watson wrote:\n\n> Instead of referencing \"stash@{n}\" explicitly, it can simply be\n> referenced as \"n\".  Most users only reference stashes by their position\n> in the stash stask (what I refer to as the \"index\"). The syntax for the\n> typical stash (stash@{n}) is slightly annoying and easy to forget, and\n> sometimes difficult to escape properly in a script. Because of this the\n> capability to do things with the stash by simply referencing the index\n> is desirable.\n> \n> This patch includes the superior implementation provided by Øsse Walle\n> (thanks for that), with a slight change to fix a broken test in the test\n> suite. I also merged the test scripts as suggested by Jeff King, and\n> un-wrapped the documentation as suggested by Junio Hamano.\n\nJust cleaning out my list of leftover bits, and it looks like this patch\nfell through the cracks.\n\nThe last thing before this was generally people agreeing with the\napproach that Øsse showed:\n\n  http://public-inbox.org/git/CAFaJEqu-JUcwLjrQBk_huSa3DZfCf8O4eAZ=UgcXHzN=CLgtpw@mail.gmail.com/\n\n  http://public-inbox.org/git/xmqqbmzzoazy.fsf@gitster.mtv.corp.google.com/\n\nand we just needed to roll that up into a patch. Which this _mostly_ is,\nbut there are a few funny things still:\n\n> diff --git a/git-stash.sh b/git-stash.sh\n> index 826af18..d8d3b8d 100755\n> --- a/git-stash.sh\n> +++ b/git-stash.sh\n> @@ -384,9 +384,10 @@ parse_flags_and_rev()\n>  \ti_tree=\n>  \tu_tree=\n>  \n> -\tREV=$(git rev-parse --no-flags --symbolic --sq \"$@\") || exit 1\n> +\tREV=$(git rev-parse --no-flags --symbolic --sq \"$@\" 2> /dev/null)\n\nStyle: we don't put a space between \">\" and the filename.\n\nSo here we assign REV, and we no longer exit (to handle something like\n\"1\" by itself, which is good).\n\n>  \tFLAGS=\n> +\tARGV=\n>  \tfor opt\n>  \tdo\n>  \t\tcase \"$opt\" in\n> @@ -404,10 +405,13 @@ parse_flags_and_rev()\n>  \t\t\t\t\tdie \"$(eval_gettext \"unknown option: \\$opt\")\"\n>  \t\t\t\tFLAGS=\"${FLAGS}${FLAGS:+ }$opt\"\n>  \t\t\t;;\n> +\t\t\t*)\n> +\t\t\t\tARGV=\"${ARGV}${ARGV:+ }'$opt'\"\n> +\t\t\t;;\n>  \t\tesac\n>  \tdone\n>  \n> -\teval set -- $REV\n> +\teval set -- $ARGV\n\nBut what's going on here? Why did we bother running rev-parse earlier if\nwe don't actually use the value of REV?\n\nYou mentioned tweaking it to fix a broken test, and indeed, just using\n$REV here breaks a few tests in t3903.\n\nOffhand, I do not see anything wrong with pulling the non-option values\nout in the loop. But in that case I think the assignment of REV can just\ngo away completely.\n\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index 2142c1f..f82a8c4 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -131,6 +131,26 @@ test_expect_success 'drop middle stash' '\n>  \ttest 1 = $(git show HEAD:file)\n>  '\n>  \n> +test_expect_success 'drop middle stash by index' '\n> +\tgit reset --hard &&\n> +\techo 8 > file &&\n> +\tgit stash &&\n> +\techo 9 > file &&\n\nDitto on the \">\" style here and elsewhere.\n\nThe tests otherwise look good to me.\n\n-Peff\n"},{"id":"304771","messageId":"20161024125452.o3xvl3q2lmmwr5qu@sigill.intra.peff.net","threadId":"44045","inReplyTo":"CAB0+k9JrX7Ax26HfTEgoSyj02szFyHLayqTyW6KPuxVvXBOEOw@mail.gmail.com","subject":"Re: [PATCH] Allow stashes to be referenced by index only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-24T12:54:52Z","receivedAt":"2016-10-24T12:55:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Oct 23, 2016 at 01:41:25PM -0400, Aaron and Ashley Watson wrote:\n\n> > But what's going on here? Why did we bother running rev-parse earlier if\n> > we don't actually use the value of REV?\n> >\n> > You mentioned tweaking it to fix a broken test, and indeed, just using\n> > $REV here breaks a few tests in t3903.\n> >\n> > Offhand, I do not see anything wrong with pulling the non-option values\n> > out in the loop. But in that case I think the assignment of REV can just\n> > go away completely.\n> >\n> \n> The only reason for REV to remain is to preserve the error message seen with\n> the previous behavior. Perhaps it would be better to instead move the\n> assignment\n> of REV to the only place it is still used: the error message when multiple\n> arguments were detected.\n\nAh, thanks, I missed that use. We suppress stderr, so we're literally\njust getting the set of revs there. But that should match what we have\nin ARGV anyway (after all, $ARGV is where we decided we had too many\nrevs, and what we'll feed to rev-parse to get the sha1).\n\nSo I wonder if:\n\n  Too many revisions specified: $ARGV\n\nwould be more appropriate (at which point you can probably just continue\nto call it $REV).\n\n> I'm not sure of the next steps in the process of submitting a patch.\n> Should I submit a new patch by replying to this email, or is using git\n> send-email to create a new mail thread better?\n\nGenerally re-rolls of a patch are done as replies to the original. Gmail\nis bad about corrupting whitespace, though, so you can't just reply and\npaste the patch in there. You can use ask git-send-email to continue the\nthread, though:\n\n  mid=1473378397-22453-1-git-send-email-watsona4@gmail.com\n  git send-email --in-reply-to=$mid ...other options...\n\nYou might also want to cc the people involved in the earlier discussion.\nThe public-inbox archive gives a customized full send-email command for\neach message to make it easy.\n\n  http://public-inbox.org/git/1473378397-22453-1-git-send-email-watsona4@gmail.com/\n\n-Peff\n"},{"id":"304814","messageId":"1477352413-4628-1-git-send-email-watsona4@gmail.com","threadId":"44045","inReplyTo":"1473378397-22453-1-git-send-email-watsona4@gmail.com","subject":"[PATCH] Allow stashes to be referenced by index only","fromName":"Aaron M Watson","fromEmail":"watsona4@gmail.com","sentAt":"2016-10-24T23:40:13Z","receivedAt":"2016-10-24T23:40:52Z","isPatch":true,"sender":{"key":"watsona4@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5954119?v=4"},"body":"Instead of referencing \"stash@{n}\" explicitly, it can simply be\nreferenced as \"n\".  Most users only reference stashes by their position\nin the stash stask (what I refer to as the \"index\"). The syntax for the\ntypical stash (stash@{n}) is slightly annoying and easy to forget, and\nsometimes difficult to escape properly in a script. Because of this the\ncapability to do things with the stash by simply referencing the index\nis desirable.\n\nThis patch includes the superior implementation provided by Øsse Walle\n(thanks for that), with a slight change to fix a broken test in the test\nsuite. I also merged the test scripts as suggested by Jeff King, and\nun-wrapped the documentation as suggested by Junio Hamano.\n\nSigned-off-by: Aaron M Watson <watsona4@gmail.com>\n---\n Documentation/git-stash.txt |  3 ++-\n git-stash.sh                | 15 +++++++++++++--\n t/t3903-stash.sh            | 35 +++++++++++++++++++++++++++++++++++\n 3 files changed, 50 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 92df596..2e9cef0 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -39,7 +39,8 @@ The latest stash you created is stored in `refs/stash`; older\n stashes are found in the reflog of this reference and can be named using\n the usual reflog syntax (e.g. `stash@{0}` is the most recently\n created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`\n-is also possible).\n+is also possible). Stashes may also be referenced by specifying just the\n+stash index (e.g. the integer `n` is equivalent to `stash@{n}`).\n \n OPTIONS\n -------\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 826af18..d7072c8 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -384,9 +384,8 @@ parse_flags_and_rev()\n \ti_tree=\n \tu_tree=\n \n-\tREV=$(git rev-parse --no-flags --symbolic --sq \"$@\") || exit 1\n-\n \tFLAGS=\n+\tREV=\n \tfor opt\n \tdo\n \t\tcase \"$opt\" in\n@@ -404,6 +403,9 @@ parse_flags_and_rev()\n \t\t\t\t\tdie \"$(eval_gettext \"unknown option: \\$opt\")\"\n \t\t\t\tFLAGS=\"${FLAGS}${FLAGS:+ }$opt\"\n \t\t\t;;\n+\t\t\t*)\n+\t\t\t\tREV=\"${REV}${REV:+ }'$opt'\"\n+\t\t\t;;\n \t\tesac\n \tdone\n \n@@ -422,6 +424,15 @@ parse_flags_and_rev()\n \t\t;;\n \tesac\n \n+\tcase \"$1\" in\n+\t\t*[!0-9]*)\n+\t\t\t:\n+\t\t;;\n+\t\t*)\n+\t\t\tset -- \"${ref_stash}@{$1}\"\n+\t\t;;\n+\tesac\n+\n \tREV=$(git rev-parse --symbolic --verify --quiet \"$1\") || {\n \t\treference=\"$1\"\n \t\tdie \"$(eval_gettext \"\\$reference is not a valid reference\")\"\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 2142c1f..f82a8c4 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -131,6 +131,26 @@ test_expect_success 'drop middle stash' '\n \ttest 1 = $(git show HEAD:file)\n '\n \n+test_expect_success 'drop middle stash by index' '\n+\tgit reset --hard &&\n+\techo 8 >file &&\n+\tgit stash &&\n+\techo 9 >file &&\n+\tgit stash &&\n+\tgit stash drop 1 &&\n+\ttest 2 = $(git stash list | wc -l) &&\n+\tgit stash apply &&\n+\ttest 9 = $(cat file) &&\n+\ttest 1 = $(git show :file) &&\n+\ttest 1 = $(git show HEAD:file) &&\n+\tgit reset --hard &&\n+\tgit stash drop &&\n+\tgit stash apply &&\n+\ttest 3 = $(cat file) &&\n+\ttest 1 = $(git show :file) &&\n+\ttest 1 = $(git show HEAD:file)\n+'\n+\n test_expect_success 'stash pop' '\n \tgit reset --hard &&\n \tgit stash pop &&\n@@ -604,7 +624,21 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '\n \tgit stash drop\n '\n \n+test_expect_success 'invalid ref of the form \"n\", n >= N' '\n+\tgit stash clear &&\n+\ttest_must_fail git stash drop 0 &&\n+\techo bar5 >file &&\n+\techo bar6 >file2 &&\n+\tgit add file2 &&\n+\tgit stash &&\n+\ttest_must_fail git stash drop 1 &&\n+\ttest_must_fail git stash pop 1 &&\n+\ttest_must_fail git stash apply 1 &&\n+\ttest_must_fail git stash show 1 &&\n+\ttest_must_fail git stash branch tmp 1 &&\n+\tgit stash drop\n+'\n+\n test_expect_success 'stash branch should not drop the stash if the branch exists' '\n \tgit stash clear &&\n \techo foo >file &&\n-- \n2.7.4\n\n"},{"id":"304819","messageId":"20161025081149.x5l5zcupva546ssf@sigill.intra.peff.net","threadId":"44045","inReplyTo":"1477352413-4628-1-git-send-email-watsona4@gmail.com","subject":"Re: [PATCH] Allow stashes to be referenced by index only","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-25T08:11:49Z","receivedAt":"2016-10-25T08:12:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 24, 2016 at 07:40:13PM -0400, Aaron M Watson wrote:\n\n> Instead of referencing \"stash@{n}\" explicitly, it can simply be\n> referenced as \"n\".  Most users only reference stashes by their position\n> in the stash stask (what I refer to as the \"index\"). The syntax for the\n> typical stash (stash@{n}) is slightly annoying and easy to forget, and\n> sometimes difficult to escape properly in a script. Because of this the\n> capability to do things with the stash by simply referencing the index\n> is desirable.\n> \n> This patch includes the superior implementation provided by Øsse Walle\n> (thanks for that), with a slight change to fix a broken test in the test\n> suite. I also merged the test scripts as suggested by Jeff King, and\n> un-wrapped the documentation as suggested by Junio Hamano.\n> \n> Signed-off-by: Aaron M Watson <watsona4@gmail.com>\n> ---\n\nThanks, this version looks good to me.\n\nOddly, it does not seem to apply. I get:\n\n  $ git am -3 ~/patch\n  Applying: Allow stashes to be referenced by index only\n  Using index info to reconstruct a base tree...\n  M       git-stash.sh\n  error: patch failed: t/t3903-stash.sh:604\n  error: t/t3903-stash.sh: patch does not apply\n  error: Did you hand edit your patch?\n  It does not apply to blobs recorded in its index.\n  Patch failed at 0001 Allow stashes to be referenced by index only\n\nThe culprit seems to be the final hunk header:\n\n> @@ -604,7 +624,21 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '\n\nThis should be \"604,6\", as there are 6 context lines, and your patch\ndoes not remove any lines.\n\nI suspect the maintainer can fix it up while applying, but for my\ncuriosity: did you hand-edit it, or is there a potential bug in git's\ndiff code?\n\n-Peff\n"},{"id":"304820","messageId":"CAB0+k9KWLo-cBGceJ5fzU41ZKbbii5dbxy5TYUbOhybivKcWGg@mail.gmail.com","threadId":"44045","inReplyTo":"20161025081149.x5l5zcupva546ssf@sigill.intra.peff.net","subject":"Re: [PATCH] Allow stashes to be referenced by index only","fromName":"Aaron and Ashley Watson","fromEmail":"watsona4@gmail.com","sentAt":"2016-10-25T08:20:17Z","receivedAt":"2016-10-25T08:20:27Z","isPatch":true,"sender":{"key":"watsona4@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5954119?v=4"},"body":"On Tue, Oct 25, 2016 at 4:11 AM, Jeff King <peff@peff.net> wrote:\n> On Mon, Oct 24, 2016 at 07:40:13PM -0400, Aaron M Watson wrote:\n>\n>> Instead of referencing \"stash@{n}\" explicitly, it can simply be\n>> referenced as \"n\".  Most users only reference stashes by their position\n>> in the stash stask (what I refer to as the \"index\"). The syntax for the\n>> typical stash (stash@{n}) is slightly annoying and easy to forget, and\n>> sometimes difficult to escape properly in a script. Because of this the\n>> capability to do things with the stash by simply referencing the index\n>> is desirable.\n>>\n>> This patch includes the superior implementation provided by Øsse Walle\n>> (thanks for that), with a slight change to fix a broken test in the test\n>> suite. I also merged the test scripts as suggested by Jeff King, and\n>> un-wrapped the documentation as suggested by Junio Hamano.\n>>\n>> Signed-off-by: Aaron M Watson <watsona4@gmail.com>\n>> ---\n>\n> Thanks, this version looks good to me.\n>\n> Oddly, it does not seem to apply. I get:\n>\n>   $ git am -3 ~/patch\n>   Applying: Allow stashes to be referenced by index only\n>   Using index info to reconstruct a base tree...\n>   M       git-stash.sh\n>   error: patch failed: t/t3903-stash.sh:604\n>   error: t/t3903-stash.sh: patch does not apply\n>   error: Did you hand edit your patch?\n>   It does not apply to blobs recorded in its index.\n>   Patch failed at 0001 Allow stashes to be referenced by index only\n>\n> The culprit seems to be the final hunk header:\n>\n>> @@ -604,7 +624,21 @@ test_expect_success 'invalid ref of the form stash@{n}, n >= N' '\n>\n> This should be \"604,6\", as there are 6 context lines, and your patch\n> does not remove any lines.\n>\n> I suspect the maintainer can fix it up while applying, but for my\n> curiosity: did you hand-edit it, or is there a potential bug in git's\n> diff code?\n\nI did indeed edit the patch by hand (I forgot to remove the spaces\nafter the > in the test file), but\nthe bug appears to be in emacs's diff-mode, not in git.\n\n>\n> -Peff\n\n\n\n-- \nAaron and Ashley Watson\n"},{"id":"304870","messageId":"xmqqshrkqf79.fsf@gitster.mtv.corp.google.com","threadId":"44045","inReplyTo":"1477352413-4628-1-git-send-email-watsona4@gmail.com","subject":"Re: [PATCH] Allow stashes to be referenced by index only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-25T21:41:30Z","receivedAt":"2016-10-25T21:41:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Aaron M Watson <watsona4@gmail.com> writes:\n\nAaron M Watson <watsona4@gmail.com> writes:\n\n> Instead of referencing \"stash@{n}\" explicitly, it can simply be\n> referenced as \"n\".\n> Most users only reference stashes by their position\n> in the stash stask (what I refer to as the \"index\").\n\nIt is unclear if the first sentence is a statement of the fact, an\nexpression of desire, or something else.  With the current codebase,\nit cannot simply be referenced as \"n\", and you either \"wish it were\npossible\", or \"make it possible to do so\", or perhaps little bit of\nboth.\n\nThis is why we tend to use imperative mood to give an order to the\ncodebase to \"be like so\" to make it clear.\n\nPerhaps\n\n  Instead of referencing \"stash@{n}\" explicitly, make it possible to\n  simply reference as \"n\".  Most users only reference stashes by their\n  position in the stash stask (what I refer to as the \"index\" here).\n\nor something like that (which is what I tenatively rewritten this to\nwhile queuing).\n\n> @@ -404,6 +403,9 @@ parse_flags_and_rev()\n>  \t\t\t\t\tdie \"$(eval_gettext \"unknown option: \\$opt\")\"\n>  \t\t\t\tFLAGS=\"${FLAGS}${FLAGS:+ }$opt\"\n>  \t\t\t;;\n> +\t\t\t*)\n> +\t\t\t\tREV=\"${REV}${REV:+ }'$opt'\"\n> +\t\t\t;;\n>  \t\tesac\n>  \tdone\n>  \n> @@ -422,6 +424,15 @@ parse_flags_and_rev()\n>  \t\t;;\n>  \tesac\n>  \n> +\tcase \"$1\" in\n> +\t\t*[!0-9]*)\n> +\t\t\t:\n> +\t\t;;\n> +\t\t*)\n> +\t\t\tset -- \"${ref_stash}@{$1}\"\n> +\t\t;;\n> +\tesac\n\nI can see that you inherited the brokenness from an existing one in\nthe earlier hunk, but case arms in these two case statements are\nindented one level too deep.  It would be good to fix it in a\nfollow-up patch (not a reroll of this patch).\n\nThanks.  Will queue.\n"},{"id":"304877","messageId":"353cc9d2-ed54-040c-95f8-9038cfa265f0@ramsayjones.plus.com","threadId":"44045","inReplyTo":"xmqqshrkqf79.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Allow stashes to be referenced by index only","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-10-26T00:12:14Z","receivedAt":"2016-10-26T00:12:25Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 25/10/16 22:41, Junio C Hamano wrote:\n> Aaron M Watson <watsona4@gmail.com> writes:\n> \n> Aaron M Watson <watsona4@gmail.com> writes:\n> \n>> Instead of referencing \"stash@{n}\" explicitly, it can simply be\n>> referenced as \"n\".\n>> Most users only reference stashes by their position\n>> in the stash stask (what I refer to as the \"index\").\n> \n> It is unclear if the first sentence is a statement of the fact, an\n> expression of desire, or something else.  With the current codebase,\n> it cannot simply be referenced as \"n\", and you either \"wish it were\n> possible\", or \"make it possible to do so\", or perhaps little bit of\n> both.\n> \n> This is why we tend to use imperative mood to give an order to the\n> codebase to \"be like so\" to make it clear.\n> \n> Perhaps\n> \n>   Instead of referencing \"stash@{n}\" explicitly, make it possible to\n>   simply reference as \"n\".  Most users only reference stashes by their\n>   position in the stash stask (what I refer to as the \"index\" here).\n\ns/stask/stack/\n\nATB,\nRamsay Jones\n\n"}]}