{"thread":{"id":"20654","subject":"[RFC PATCH] stash: accept options also when subcommand 'save' is omitted","startedAt":"2009-08-18T12:46:07Z","lastAt":"2009-08-19T09:57:16Z","messageCount":22,"participants":["Matthieu Moy","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"121094","messageId":"1250599567-31428-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"20654","inReplyTo":null,"subject":"[RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-18T12:46:07Z","receivedAt":"2009-08-18T12:46:07Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This allows in particular 'git stash --keep-index' which is shorter than\n'git stash save --keep-index', and not ambiguous.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nDoes this sound right?\n\nI'm so used to 'git stash' (without saying 'save') that I keep typing\n'git stash --keep-index', and get a usage string as an error message.\n\n Documentation/git-stash.txt |    2 +-\n git-stash.sh                |    8 +++++++-\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 2f5ca7b..6f251e7 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n 'git stash' drop [-q|--quiet] [<stash>]\n 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n 'git stash' branch <branchname> [<stash>]\n-'git stash' [save [--keep-index] [-q|--quiet] [<message>]]\n+'git stash' [save] [--keep-index] [-q|--quiet] [<message>]\n 'git stash' clear\n 'git stash' create\n \ndiff --git a/git-stash.sh b/git-stash.sh\nindex 03e589f..2599410 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -7,7 +7,7 @@ USAGE=\"list [<options>]\n    or: $dashless drop [-q|--quiet] [<stash>]\n    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n    or: $dashless branch <branchname> [<stash>]\n-   or: $dashless [save [--keep-index] [-q|--quiet] [<message>]]\n+   or: $dashless [save] [--keep-index] [-q|--quiet] [<message>]\n    or: $dashless clear\"\n \n SUBDIRECTORY_OK=Yes\n@@ -302,6 +302,12 @@ apply_to_branch () {\n \tdrop_stash $stash\n }\n \n+case \"$1\" in\n+    -*)\n+\tset \"save\" \"$@\"\n+\t;;\n+esac\n+\n # Main command set\n case \"$1\" in\n list)\n-- \n1.6.4.173.g0591\n"},{"id":"121101","messageId":"vpqws51l1hb.fsf@bauges.imag.fr","threadId":"20654","inReplyTo":"1250599567-31428-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-18T13:01:52Z","receivedAt":"2009-08-18T13:01:52Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> This allows in particular 'git stash --keep-index' which is shorter than\n> 'git stash save --keep-index', and not ambiguous.\n\nHmm, googling a bit, I just noticed that there's already something in\npu:\nea41cfc4f (Make 'git stash -k' a short form for 'git stash save --keep-index')\nwhich also does the trick, while adding a -k alias for --keep-index.\n\nNot sure which hack is best between my\n\n> +case \"$1\" in\n> +    -*)\n> +\tset \"save\" \"$@\"\n> +\t;;\n> +esac\n> +\n\nAnd the proposed\n\n *)\n-\tif test $# -eq 0\n-\tthen\n-\t\tsave_stash &&\n+\tcase $#,\"$1\" in\n+\t0,|1,-k|1,--keep-index)\n+\t\tsave_stash \"$@\" &&\n \t\tsay '(To restore them type \"git stash apply\")'\n-\telse\n+\t\t;;\n+\t*)\n \t\tusage\n-\tfi\n+\tesac\n \t;;\n esac\n\nMine has at least two advantages:\n\n* It won't require changing the code again when new options are added\n  to 'git stash save'.\n\n* It works with 'git stash -k -q' for example, while the other\n  proposal checks that $# == 1, which won't work if there are more\n  than one option.\n\nBut I may have missed its drawbacks ;-)\n\n--\nMatthieu\n"},{"id":"121134","messageId":"20090818174509.GA27518@coredump.intra.peff.net","threadId":"20654","inReplyTo":"vpqws51l1hb.fsf@bauges.imag.fr","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-18T17:45:09Z","receivedAt":"2009-08-18T17:45:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 18, 2009 at 03:01:52PM +0200, Matthieu Moy wrote:\n\n> Hmm, googling a bit, I just noticed that there's already something in\n> pu:\n> ea41cfc4f (Make 'git stash -k' a short form for 'git stash save --keep-index')\n> which also does the trick, while adding a -k alias for --keep-index.\n>\n> [...]\n> \n> Mine has at least two advantages:\n> \n> * It won't require changing the code again when new options are added\n>   to 'git stash save'.\n> \n> * It works with 'git stash -k -q' for example, while the other\n>   proposal checks that $# == 1, which won't work if there are more\n>   than one option.\n\nI think yours is nicer, especially as we have just added the\n'-p|--patch' option, as well. With what is there now, you can do \"git\nstash -p\", but not \"git stash -p -k\".\n\n> But I may have missed its drawbacks ;-)\n\nThe only I can think of is that bogus input will provoke 'save'. So\nsomething like:\n\n  git stash --apply\n\nwill invoke \"git stash save --apply\", which doesn't even complain. It\njust tries to make a stash with message --apply. Now of course this\ninput is obviously bogus, but probably the right thing to do is\ncomplain.\n\nOTOH, I think it is the fault of \"git stash save --apply\" for not doing\nthe complaining, so your patch really isn't making it worse. Probably it\nshould barf on anything unrecognized starting with a '-', and allow '--'\nto separate the message from the rest of the options (in the rare case\nthat you want a message starting with '-').\n\n-Peff\n"},{"id":"121156","messageId":"7vbpmcc1sc.fsf@alter.siamese.dyndns.org","threadId":"20654","inReplyTo":"20090818174509.GA27518@coredump.intra.peff.net","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-18T20:20:03Z","receivedAt":"2009-08-18T20:20:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Aug 18, 2009 at 03:01:52PM +0200, Matthieu Moy wrote:\n> ...\n> I think yours is nicer, especially as we have just added the\n> '-p|--patch' option, as well. With what is there now, you can do \"git\n> stash -p\", but not \"git stash -p -k\".\n>\n>> But I may have missed its drawbacks ;-)\n>\n> The only I can think of is that bogus input will provoke 'save'. So\n> something like:\n>\n>   git stash --apply\n>\n> will invoke \"git stash save --apply\", which doesn't even complain. It\n> just tries to make a stash with message --apply. Now of course this\n> input is obviously bogus, but probably the right thing to do is\n> complain.\n>\n> OTOH, I think it is the fault of \"git stash save --apply\" for not doing\n> the complaining, so your patch really isn't making it worse. Probably it\n> should barf on anything unrecognized starting with a '-', and allow '--'\n> to separate the message from the rest of the options (in the rare case\n> that you want a message starting with '-').\n\nSounds like a sane approach.\n\nI am Ok with the idea of queuing a patch to update js/stash-dwim ea41cfc\n(Make 'git stash -k' a short form for 'git stash save --keep-index',\n2009-07-27) with an implementation along this line; the \"stash -p\" one\nf300fab (DWIM 'git stash save -p' for 'git stash -p', 2009-08-13) also\nmay need to adjusted (it could become no-op, I suspect).\n"},{"id":"121171","messageId":"1250631523-10524-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"20654","inReplyTo":"7vbpmcc1sc.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/3] short syntaxes for 'git stash'","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-18T21:38:40Z","receivedAt":"2009-08-18T21:38:40Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This small patch serie is based on the following commit in pu:\n\ndda1f2a Implement 'git stash save --patch'\n\nIt is meant to replace two commits already there:\n\nea41cfc Make 'git stash -k' a short form for 'git stash save --keep-index'\nf300fab DWIM 'git stash save -p' for 'git stash -p'\n\nThe first (git stash -k) has the drawback of forcing one to use one\nand only one option when the 'save' command is ommited. My approach is\nmostly based on the simple hack:\n\n+case \"$1\" in\n+    -*)\n+       set \"save\" \"$@\"\n+       ;;\n+esac\n+\n\nand accepts an arbitrary number of options.\n\nThe second (DWIM git stash save -p) becomes unnecessary afterwards.\n"},{"id":"121168","messageId":"1250631523-10524-2-git-send-email-Matthieu.Moy@imag.fr","threadId":"20654","inReplyTo":"1250631523-10524-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 1/3] stash: accept -k as a shortcut for --keep-index","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-18T21:38:41Z","receivedAt":"2009-08-18T21:38:41Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Documentation/git-stash.txt |    4 ++--\n git-stash.sh                |    4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 4b15459..1b5392a 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n 'git stash' drop [-q|--quiet] [<stash>]\n 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n 'git stash' branch <branchname> [<stash>]\n-'git stash' [save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]]\n+'git stash' [save [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]]\n 'git stash' clear\n 'git stash' create\n \n@@ -42,7 +42,7 @@ is also possible).\n OPTIONS\n -------\n \n-save [--patch] [--[no-]keep-index] [-q|--quiet] [<message>]::\n+save [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]::\n \n \tSave your local modifications to a new 'stash', and run `git reset\n \t--hard` to revert them.  This is the default action when no\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 567aa5d..856a2d5 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -7,7 +7,7 @@ USAGE=\"list [<options>]\n    or: $dashless drop [-q|--quiet] [<stash>]\n    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n    or: $dashless branch <branchname> [<stash>]\n-   or: $dashless [save [--keep-index] [-q|--quiet] [<message>]]\n+   or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]\n    or: $dashless clear\"\n \n SUBDIRECTORY_OK=Yes\n@@ -132,7 +132,7 @@ save_stash () {\n \twhile test $# != 0\n \tdo\n \t\tcase \"$1\" in\n-\t\t--keep-index)\n+\t\t-k|--keep-index)\n \t\t\tkeep_index=t\n \t\t\t;;\n \t\t--no-keep-index)\n-- \n1.6.4.rc2.31.g2d7d7\n"},{"id":"121169","messageId":"1250631523-10524-3-git-send-email-Matthieu.Moy@imag.fr","threadId":"20654","inReplyTo":"1250631523-10524-2-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 2/3] stash: accept options also when subcommand 'save' is omitted","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-18T21:38:42Z","receivedAt":"2009-08-18T21:38:42Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This allows in particular 'git stash -k which is shorter than\n'git stash save -k', and not ambiguous.\n\nTestcase taken from Johannes Schindelin <johannes.schindelin@gmx.de>.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n Documentation/git-stash.txt |    2 +-\n git-stash.sh                |    8 +++++++-\n t/t3903-stash.sh            |    8 ++++++++\n 3 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 1b5392a..7e515ce 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n 'git stash' drop [-q|--quiet] [<stash>]\n 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n 'git stash' branch <branchname> [<stash>]\n-'git stash' [save [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]]\n+'git stash' [save] [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]\n 'git stash' clear\n 'git stash' create\n \ndiff --git a/git-stash.sh b/git-stash.sh\nindex 856a2d5..bb36bc7 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -7,7 +7,7 @@ USAGE=\"list [<options>]\n    or: $dashless drop [-q|--quiet] [<stash>]\n    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n    or: $dashless branch <branchname> [<stash>]\n-   or: $dashless [save [-k|--keep-index] [-q|--quiet] [<message>]]\n+   or: $dashless [save] [-k|--keep-index] [-q|--quiet] [<message>]\n    or: $dashless clear\"\n \n SUBDIRECTORY_OK=Yes\n@@ -354,6 +354,12 @@ apply_to_branch () {\n \tdrop_stash $stash\n }\n \n+case \"$1\" in\n+    -*)\n+\tset \"save\" \"$@\"\n+\t;;\n+esac\n+\n # Main command set\n case \"$1\" in\n list)\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 7a3fb67..0e831e0 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -200,4 +200,12 @@ test_expect_success 'drop -q is quiet' '\n \ttest ! -s output.out\n '\n \n+test_expect_success 'stash -k' '\n+       echo bar3 > file &&\n+       echo bar4 > file2 &&\n+       git add file2 &&\n+       git stash -k &&\n+       test bar,bar4 = $(cat file),$(cat file2)\n+'\n+\n test_done\n-- \n1.6.4.rc2.31.g2d7d7\n"},{"id":"121173","messageId":"1250631523-10524-4-git-send-email-Matthieu.Moy@imag.fr","threadId":"20654","inReplyTo":"1250631523-10524-3-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH 3/3] stash: reject stash name starting with a dash.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-18T21:38:43Z","receivedAt":"2009-08-18T21:38:43Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"This avoids typos like 'git stash save --invalid-option', particularly\nnasty since one can omit the 'save' subcommand. The syntax\n'git stash save -- \"-name starting with dash\" still allows such stash name.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nJeff King <peff@peff.net> writes:\n\n>> But I may have missed its drawbacks ;-)\n>\n> The only I can think of is that bogus input will provoke 'save'. So\n> something like:\n>\n>   git stash --apply\n>\n> will invoke \"git stash save --apply\", which doesn't even complain. It\n> just tries to make a stash with message --apply. Now of course this\n> input is obviously bogus, but probably the right thing to do is\n> complain.\n>\n> OTOH, I think it is the fault of \"git stash save --apply\" for not doing\n> the complaining, so your patch really isn't making it worse. Probably it\n> should barf on anything unrecognized starting with a '-', and allow '--'\n> to separate the message from the rest of the options (in the rare case\n> that you want a message starting with '-').\n>\n> -Peff\n\nSo, here it is!\n\n Documentation/git-stash.txt |    2 +-\n git-stash.sh                |   10 +++++++++-\n t/t3903-stash.sh            |   10 ++++++++++\n 3 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt\nindex 7e515ce..ded62e0 100644\n--- a/Documentation/git-stash.txt\n+++ b/Documentation/git-stash.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n 'git stash' drop [-q|--quiet] [<stash>]\n 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n 'git stash' branch <branchname> [<stash>]\n-'git stash' [save] [--patch] [--[no-]keep-index|-k] [-q|--quiet] [<message>]\n+'git stash' [save] [--patch] [--[no-]keep-index|-k] [-q|--quiet] [--] [<message>]\n 'git stash' clear\n 'git stash' create\n \ndiff --git a/git-stash.sh b/git-stash.sh\nindex bb36bc7..642e265 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -7,7 +7,7 @@ USAGE=\"list [<options>]\n    or: $dashless drop [-q|--quiet] [<stash>]\n    or: $dashless ( pop | apply ) [--index] [-q|--quiet] [<stash>]\n    or: $dashless branch <branchname> [<stash>]\n-   or: $dashless [save] [-k|--keep-index] [-q|--quiet] [<message>]\n+   or: $dashless [save] [-k|--keep-index] [-q|--quiet] [--] [<message>]\n    or: $dashless clear\"\n \n SUBDIRECTORY_OK=Yes\n@@ -145,6 +145,14 @@ save_stash () {\n \t\t-q|--quiet)\n \t\t\tGIT_QUIET=t\n \t\t\t;;\n+\t\t--)\n+\t\t\tshift\n+\t\t\tbreak\n+\t\t\t;;\n+\t\t-*)\n+\t\t\techo \"error: unknown option for 'stash save': $1\"\n+\t\t\tusage\n+\t\t\t;;\n \t\t*)\n \t\t\tbreak\n \t\t\t;;\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 0e831e0..87e5a14 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -208,4 +208,14 @@ test_expect_success 'stash -k' '\n        test bar,bar4 = $(cat file),$(cat file2)\n '\n \n+test_expect_success 'stash --invalid-option' '\n+       echo bar5 > file &&\n+       echo bar6 > file2 &&\n+       git add file2 &&\n+       ! git stash --invalid-option &&\n+       test bar5,bar6 = $(cat file),$(cat file2) &&\n+       git stash -- -message-starting-with-dash &&\n+       test bar,bar2 = $(cat file),$(cat file2)\n+'\n+\n test_done\n-- \n1.6.4.rc2.31.g2d7d7\n"},{"id":"121172","messageId":"alpine.DEB.1.00.0908182337200.8306@pacific.mpi-cbg.de","threadId":"20654","inReplyTo":"20090818174509.GA27518@coredump.intra.peff.net","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-18T21:42:58Z","receivedAt":"2009-08-18T21:42:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 18 Aug 2009, Jeff King wrote:\n\n> On Tue, Aug 18, 2009 at 03:01:52PM +0200, Matthieu Moy wrote:\n> \n> > Hmm, googling a bit, I just noticed that there's already something in\n> > pu:\n> > ea41cfc4f (Make 'git stash -k' a short form for 'git stash save --keep-index')\n> > which also does the trick, while adding a -k alias for --keep-index.\n> >\n> > [...]\n> > \n> > Mine has at least two advantages:\n> > \n> > * It won't require changing the code again when new options are added\n> >   to 'git stash save'.\n> > \n> > * It works with 'git stash -k -q' for example, while the other\n> >   proposal checks that $# == 1, which won't work if there are more\n> >   than one option.\n> \n> I think yours is nicer, especially as we have just added the\n> '-p|--patch' option, as well. With what is there now, you can do \"git\n> stash -p\", but not \"git stash -p -k\".\n\nBut it is sloppy, in that it blindly accepts options that might be valid \nfor several subcommands, not just \"save\".\n\nThat was the reason I did not implement it this way.\n\nBut we do not have such ambiguous options yet.\n\nOr do we?  Look at what \"list\" accepts!\n\nSo please register my objection.\n\nCiao,\nDscho\n"},{"id":"121175","messageId":"vpqtz0423jk.fsf@bauges.imag.fr","threadId":"20654","inReplyTo":"alpine.DEB.1.00.0908182337200.8306@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-18T21:52:15Z","receivedAt":"2009-08-18T21:52:15Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> But it is sloppy, in that it blindly accepts options that might be valid\n> for several subcommands, not just \"save\".\n\nI wouldn't call that sloppy. 'save' is the default command, if you\ndon't provide any command, then 'save' will be used.\n\nSo, \"git stash -p\" means \"git stash save -p\" regardless of the fact\nthat there exists somewhere else a \"git stash list -p\". Actually, in\nthe current form of pu, it is already the case since\n\nf300fab (Thomas Rast, DWIM 'git stash save -p' for 'git stash -p')\n\nwhich came right after your patch.\n\n> So please register my objection.\n\nI will if you register my objection to yours ;-).\n\nJokes aside, if you insist in rejecting 'git stash -p', then my patch\ncan be slightly modified to say something like\n\n+case \"$1\" in\n+    -k|--keep-index|--patch)\n+       set \"save\" \"$@\"\n+       ;;\n+esac\n\ninstead, which also allows multiple arguments (unlike your initial\nproposal), and can control more precisely the list of options for\nwhich 'save' is a default.\n\n--\nMatthieu\n"},{"id":"121180","messageId":"20090818223028.GB31176@coredump.intra.peff.net","threadId":"20654","inReplyTo":"alpine.DEB.1.00.0908182337200.8306@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-18T22:30:28Z","receivedAt":"2009-08-18T22:30:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 18, 2009 at 11:42:58PM +0200, Johannes Schindelin wrote:\n\n> > I think yours is nicer, especially as we have just added the\n> > '-p|--patch' option, as well. With what is there now, you can do \"git\n> > stash -p\", but not \"git stash -p -k\".\n> \n> But it is sloppy, in that it blindly accepts options that might be valid \n> for several subcommands, not just \"save\".\n> \n> That was the reason I did not implement it this way.\n> \n> But we do not have such ambiguous options yet.\n> \n> Or do we?  Look at what \"list\" accepts!\n> \n> So please register my objection.\n\nI don't see the problem. Either the option works for \"stash save\" or it\ndoes not.  If I say \"git stash --quiet\", then it _must_ be \"git stash\nsave --quiet\", and not \"git stash pop --quiet\", because \"save\" is the\nonly default command.  If I say \"git stash --foobar\", it is translated\nto \"git stash save --foobar\", which should generate an error (it doesn't\nright now, but that is a separate issue). I don't see any confusion:\n\"save\" is always the default command, unless one is given as the first\nargument.\n\nThe place where I would see a potential problem is if stash grew any\n\"global\" options (e.g., in the same way that \"git\" can take options\nbefore its subcommand name).\n\n-Peff\n"},{"id":"121181","messageId":"alpine.DEB.1.00.0908190035140.8306@pacific.mpi-cbg.de","threadId":"20654","inReplyTo":"vpqtz0423jk.fsf@bauges.imag.fr","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-18T22:37:20Z","receivedAt":"2009-08-18T22:37:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 18 Aug 2009, Matthieu Moy wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > But it is sloppy, in that it blindly accepts options that might be \n> > valid for several subcommands, not just \"save\".\n> \n> I wouldn't call that sloppy. 'save' is the default command, if you don't \n> provide any command, then 'save' will be used.\n\n'save' might be the default command, but we don't trigger it with _any_ \ncrap, for a very good reason.  Read the commit log for git-stash.sh to \nknow why.\n\n> > So please register my objection.\n> \n> I will if you register my objection to yours ;-).\n> \n> Jokes aside,\n\nIf you think that's funny, I don't.  I actually put a lot of thought into \nthe issue whether to allow _any_ parameter with looks like an option to \ntrigger \"save\".  And I very much had to come to the decision that no, that \nis too dangerous.\n\nSo I cannot take your objection seriously.\n\nCiao,\nDscho\n"},{"id":"121183","messageId":"alpine.DEB.1.00.0908190041200.8306@pacific.mpi-cbg.de","threadId":"20654","inReplyTo":"20090818223028.GB31176@coredump.intra.peff.net","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-08-18T22:46:27Z","receivedAt":"2009-08-18T22:46:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 18 Aug 2009, Jeff King wrote:\n\n> On Tue, Aug 18, 2009 at 11:42:58PM +0200, Johannes Schindelin wrote:\n> \n> > > I think yours is nicer, especially as we have just added the \n> > > '-p|--patch' option, as well. With what is there now, you can do \n> > > \"git stash -p\", but not \"git stash -p -k\".\n> > \n> > But it is sloppy, in that it blindly accepts options that might be \n> > valid for several subcommands, not just \"save\".\n> > \n> > That was the reason I did not implement it this way.\n> > \n> > But we do not have such ambiguous options yet.\n> > \n> > Or do we?  Look at what \"list\" accepts!\n> > \n> > So please register my objection.\n> \n> I don't see the problem. Either the option works for \"stash save\" or it\n> does not.  If I say \"git stash --quiet\", then it _must_ be \"git stash\n> save --quiet\", and not \"git stash pop --quiet\", because \"save\" is the\n> only default command.\n\nSo if I say \"git stash -q\" by mistake, but wanted to say \"git stash drop \n-q\", then I am borked?\n\nBah!  I say: bah!\n\nYou're basically reintroducing at least part of the DWIMery that was \nreverted in 9488e875 and I have the distinct feeling that some people in \nthis thread do not think hard enough about what would adher to the \nprinciple of least surprise even in the future (or even for people \nintroducing other stash save options).\n\nWell, you just go ahead and push through your patch, and I do what I \npromised on my blog.\n"},{"id":"121186","messageId":"20090818230632.GA4436@sigill.intra.peff.net","threadId":"20654","inReplyTo":"alpine.DEB.1.00.0908190041200.8306@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-18T23:06:32Z","receivedAt":"2009-08-18T23:06:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 19, 2009 at 12:46:27AM +0200, Johannes Schindelin wrote:\n\n> So if I say \"git stash -q\" by mistake, but wanted to say \"git stash drop \n> -q\", then I am borked?\n> \n> Bah!  I say: bah!\n\nYep, though it is only one of many borkings that currently exist. Try:\n\n  # oops, what was the name of that option?\n  git stash save --index\n\n  # does apply take --patch, too?\n  git stash apply --patch\n\nStill, other parts of the option parsing being awful aren't an excuse to\nmess this one up. So I see your point.\n\n> You're basically reintroducing at least part of the DWIMery that was \n> reverted in 9488e875 and I have the distinct feeling that some people in \n> this thread do not think hard enough about what would adher to the \n> principle of least surprise even in the future (or even for people \n> introducing other stash save options).\n\nI don't think it is quite as bad as that, as arbitrary crap will not get\npassed through, only crap that looks like options. Which is perhaps a\nstep up, but it is debatable how much.\n\nI think Matthieu's proposal to be strict about matching the options, but\nto take multiple options is probably the best bet for now. As it is now,\naccepting \"git stash -k\" or \"git stash -p\" but not \"git stash -k -p\" is\npretty counterintuitive. But with explicit matching it should be no less\nsafe than it is now.\n\n> Well, you just go ahead and push through your patch, and I do what I \n> promised on my blog.\n\nI don't know why you need to be so confrontational. I don't even have a\npatch to \"push through\". I just said \"I don't see the problem\", and I'm\nglad you brought up the previous stash behavior, because I had forgotten\nthe pain it caused. And that is why discussing on a public forum is\nnice; it lets people contribute things that others might have missed.\n\n-Peff\n"},{"id":"121187","messageId":"20090818233153.GA6304@sigill.intra.peff.net","threadId":"20654","inReplyTo":"1250631523-10524-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 0/3] short syntaxes for 'git stash'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-18T23:31:53Z","receivedAt":"2009-08-18T23:31:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 18, 2009 at 11:38:40PM +0200, Matthieu Moy wrote:\n\n> This small patch serie is based on the following commit in pu:\n> \n> dda1f2a Implement 'git stash save --patch'\n> \n> It is meant to replace two commits already there:\n> \n> ea41cfc Make 'git stash -k' a short form for 'git stash save --keep-index'\n> f300fab DWIM 'git stash save -p' for 'git stash -p'\n\nActually, these are already in 'next', so they can't be simply replaced.\nSo you would need to re-roll patches 1 and 2 at the very least.\n\nHowever, thinking more on it, I think we can address Dscho's concern\nwith your proposal to accept only a limited set of options. And looking\nat what's in f300fab, it actually does make an attempt to allow multiple\noptions, but it doesn't cover all cases (e.g., I can use \"-p\n--no-keep-index\" but not \"--no-keep-index -p\". Nor can I do \"-p -k\";\neven though \"-k\" is implied by \"-p\", you will get a very strange usage\nmention instead of it being a silent no-op).\n\nSo there are two issues:\n\n  - refactoring to allow arbitrary combinations of -k/-p and variants.\n\n  - allowing other options; I believe \"-q\" is the only one. That seems\n    to be specific to Dscho's objection, as it is ambiguous with other\n    subcommands. Though \"-p\" may also become ambiguous, if we get \"stash\n    apply -p\" soon.\n\nI think the first one should be fairly uncontroversial. I'm not sure\nabout the second.\n\n-Peff\n"},{"id":"121188","messageId":"20090818233535.GB6304@sigill.intra.peff.net","threadId":"20654","inReplyTo":"1250631523-10524-4-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH 3/3] stash: reject stash name starting with a dash.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-18T23:35:35Z","receivedAt":"2009-08-18T23:35:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 18, 2009 at 11:38:43PM +0200, Matthieu Moy wrote:\n\n> This avoids typos like 'git stash save --invalid-option', particularly\n> nasty since one can omit the 'save' subcommand. The syntax\n> 'git stash save -- \"-name starting with dash\" still allows such stash name.\n\nAside from the documentation and usage lines, this one is actually\nindependent of the other two, and I think makes sense regardless of what\nhappens.\n\n> +test_expect_success 'stash --invalid-option' '\n> +       echo bar5 > file &&\n> +       echo bar6 > file2 &&\n> +       git add file2 &&\n> +       ! git stash --invalid-option &&\n> +       test bar5,bar6 = $(cat file),$(cat file2) &&\n> +       git stash -- -message-starting-with-dash &&\n> +       test bar,bar2 = $(cat file),$(cat file2)\n> +'\n\nShould this actually be \"git stash save --invalid-option\", since it is\nreally testing the actual save option parsing, and not the behavior to\nautomatically push options to \"git stash save\"?\n\nOther than that, patch looks good.\n\n-Peff\n"},{"id":"121190","messageId":"7vocqc3cuz.fsf@alter.siamese.dyndns.org","threadId":"20654","inReplyTo":"20090818233535.GB6304@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] stash: reject stash name starting with a dash.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-18T23:45:40Z","receivedAt":"2009-08-18T23:45:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Aug 18, 2009 at 11:38:43PM +0200, Matthieu Moy wrote:\n>\n>> This avoids typos like 'git stash save --invalid-option', particularly\n>> nasty since one can omit the 'save' subcommand. The syntax\n>> 'git stash save -- \"-name starting with dash\" still allows such stash name.\n>\n> Aside from the documentation and usage lines, this one is actually\n> independent of the other two, and I think makes sense regardless of what\n> happens.\n\nYes, and I'd rather see it near 'maint' even if we were to (mind you, we\nwon't) reject all changes in 'next' on this program.\n\n>> +test_expect_success 'stash --invalid-option' '\n>> +       echo bar5 > file &&\n>> +       echo bar6 > file2 &&\n>> +       git add file2 &&\n>> +       ! git stash --invalid-option &&\n>> +       test bar5,bar6 = $(cat file),$(cat file2) &&\n>> +       git stash -- -message-starting-with-dash &&\n>> +       test bar,bar2 = $(cat file),$(cat file2)\n>> +'\n>\n> Should this actually be \"git stash save --invalid-option\", since it is\n> really testing the actual save option parsing, and not the behavior to\n> automatically push options to \"git stash save\"?\n\nIt would be ideal if\n\n\tgit stash save --invalid-option\n\nfailed, while\n\n\tgit stash --invalid-option should be trapped\n\nand/or\n\n\tgit stash \"--invalid-option should be trapped\"\n\nare accepted as valid quickie ways to name a WIP stash before attending to\nan unrelated emergency.\n"},{"id":"121193","messageId":"20090818235446.GA11153@sigill.intra.peff.net","threadId":"20654","inReplyTo":"7vocqc3cuz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] stash: reject stash name starting with a dash.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-18T23:54:46Z","receivedAt":"2009-08-18T23:54:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 18, 2009 at 04:45:40PM -0700, Junio C Hamano wrote:\n\n> > Should this actually be \"git stash save --invalid-option\", since it is\n> > really testing the actual save option parsing, and not the behavior to\n> > automatically push options to \"git stash save\"?\n> \n> It would be ideal if\n> \n> \tgit stash save --invalid-option\n> \n> failed, while\n> \n> \tgit stash --invalid-option should be trapped\n> \n> and/or\n> \n> \tgit stash \"--invalid-option should be trapped\"\n> \n> are accepted as valid quickie ways to name a WIP stash before attending to\n> an unrelated emergency.\n\nI thought that was the exact DWIM that was rejected previously so that\nyou don't accidentally make commits with \"git stash llist\" or \"git stash\n--almost-a-valid-optio\".\n\n-Peff\n"},{"id":"121194","messageId":"7vfxbo3cen.fsf@alter.siamese.dyndns.org","threadId":"20654","inReplyTo":"7vocqc3cuz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] stash: reject stash name starting with a dash.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-18T23:55:28Z","receivedAt":"2009-08-18T23:55:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> It would be ideal if\n>\n> \tgit stash save --invalid-option\n>\n> failed, while\n>\n> \tgit stash --invalid-option should be trapped\n>\n> and/or\n>\n> \tgit stash \"--invalid-option should be trapped\"\n>\n> are accepted as valid quickie ways to name a WIP stash before attending to\n> an unrelated emergency.\n\nMeh, let me take it back.  I somehow was living in 13 months ago for a\nshort while when I wrote the above.\n\n9488e87 (git-stash: require \"save\" to be explicit and update\ndocumentation, 2007-07-01) made the latter two invalid.\n"},{"id":"121214","messageId":"vpq8whg1ebi.fsf@bauges.imag.fr","threadId":"20654","inReplyTo":"20090818233535.GB6304@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] stash: reject stash name starting with a dash.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-19T06:57:05Z","receivedAt":"2009-08-19T06:57:05Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Aug 18, 2009 at 11:38:43PM +0200, Matthieu Moy wrote:\n>\n>> This avoids typos like 'git stash save --invalid-option', particularly\n>> nasty since one can omit the 'save' subcommand. The syntax\n>> 'git stash save -- \"-name starting with dash\" still allows such stash name.\n>\n> Aside from the documentation and usage lines, this one is actually\n> independent of the other two, and I think makes sense regardless of what\n> happens.\n\nI also do (but I'm not sure this is important enough to be in maint,\nespecially since it'll introduce silly textual conflicts in docs when\nmerging).\n\n>> +test_expect_success 'stash --invalid-option' '\n>> +       echo bar5 > file &&\n>> +       echo bar6 > file2 &&\n>> +       git add file2 &&\n>> +       ! git stash --invalid-option &&\n>> +       test bar5,bar6 = $(cat file),$(cat file2) &&\n>> +       git stash -- -message-starting-with-dash &&\n>> +       test bar,bar2 = $(cat file),$(cat file2)\n>> +'\n>\n> Should this actually be \"git stash save --invalid-option\", since it is\n> really testing the actual save option parsing, and not the behavior to\n> automatically push options to \"git stash save\"?\n\nIt could be, but the most annoying DWIM would be the \"implicit save\"\ncase, so that's the one I'm testing. One could test both, but that'd\nprobably be a bit overkill.\n\n-- \nMatthieu\n"},{"id":"121215","messageId":"vpqzl9wz35u.fsf@bauges.imag.fr","threadId":"20654","inReplyTo":"alpine.DEB.1.00.0908190035140.8306@pacific.mpi-cbg.de","subject":"Re: [RFC PATCH] stash: accept options also when subcommand 'save' is omitted","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2009-08-19T07:14:05Z","receivedAt":"2009-08-19T07:14:05Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Tue, 18 Aug 2009, Matthieu Moy wrote:\n>\n>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>> \n>> > But it is sloppy, in that it blindly accepts options that might be \n>> > valid for several subcommands, not just \"save\".\n>> \n>> I wouldn't call that sloppy. 'save' is the default command, if you don't \n>> provide any command, then 'save' will be used.\n>\n> 'save' might be the default command, but we don't trigger it with _any_ \n> crap, for a very good reason.  Read the commit log for git-stash.sh to \n> know why.\n\nThe good reason was people doing a typo when typing a command, like\n'git stash aply' or so. And yes, I did find this annoying. The\nquestion of someone omitting the subcommand is very different to me.\nOn can hardly type 'git stash -q' and claim he explicitely wanted to\nrun 'pop'.\n\nYour proposal will almost certainly trigger complains from users:\n\ngit stash -k => works\ngit stash -k -q => doesn't work\ngit stash -k \"name of stash\" => doesn't work\ngit stash save -k \"name of stash\" => works\ngit stash -p => works with another patch merged in next\ngit stash -q => doesn't work\ngit stash --patch --no-keep-index => works\ngit stash --no-keep-index --patch => doesn't work\n\nYou'll have a hard time explaining that to bare mortals.\n\nLook at what's the code's becomming:\n\n+       0,,|1,-k,|1,--keep-index,|1,-p,|1,--patch,|2,-p,--no-keep-index|2,--patch,--no-keep-index)\n\nwhen the code starts having to enumerate so many possibilities instead\nof having a simple logic, something's going wrong.\n\n-- \nMatthieu\n"},{"id":"121234","messageId":"20090819095716.GA28920@sigill.intra.peff.net","threadId":"20654","inReplyTo":"vpq8whg1ebi.fsf@bauges.imag.fr","subject":"Re: [PATCH 3/3] stash: reject stash name starting with a dash.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-19T09:57:16Z","receivedAt":"2009-08-19T09:57:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 19, 2009 at 08:57:05AM +0200, Matthieu Moy wrote:\n\n> >> +test_expect_success 'stash --invalid-option' '\n> >> +       echo bar5 > file &&\n> >> +       echo bar6 > file2 &&\n> >> +       git add file2 &&\n> >> +       ! git stash --invalid-option &&\n> >> +       test bar5,bar6 = $(cat file),$(cat file2) &&\n> >> +       git stash -- -message-starting-with-dash &&\n> >> +       test bar,bar2 = $(cat file),$(cat file2)\n> >> +'\n> >\n> > Should this actually be \"git stash save --invalid-option\", since it is\n> > really testing the actual save option parsing, and not the behavior to\n> > automatically push options to \"git stash save\"?\n> \n> It could be, but the most annoying DWIM would be the \"implicit save\"\n> case, so that's the one I'm testing. One could test both, but that'd\n> probably be a bit overkill.\n\nBut if your proposal to accept any \"-*' is not accepted, then it is not\ntesting your added code at all, is it? Even without the rest of the\npatch, the test would pass.\n\n-Peff\n"}]}