{"thread":{"id":"39385","subject":"[BUG, RFC] git stash drop --help","startedAt":"2015-05-20T17:14:14Z","lastAt":"2015-11-01T08:17:52Z","messageCount":5,"participants":["Vincent Legoll","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"261672","messageId":"CAEwRq=r=iMmnnzS2F_2rr9Tjem9khn1d=os3krjEjOg5iK5bww@mail.gmail.com","threadId":"39385","inReplyTo":null,"subject":"[BUG, RFC] git stash drop --help","fromName":"Vincent Legoll","fromEmail":"vincent.legoll@gmail.com","sentAt":"2015-05-20T17:14:14Z","receivedAt":"2015-05-20T17:14:14Z","isPatch":false,"sender":{"key":"vincent.legoll@gmail.com","avatar":null},"body":"Hello,\n\nI stumbled upon something that annoyed me a bit, as I was working with\n\ngit stash to commit some big pile of modifications in small commits...\n\nI wanted to get help wrt \"git stash drop\" and did it the following way :\n\n[steps to reproduce]\n\nmkdir tmp\ncd tmp\ngit init\ntouch test.txt\ngit add test.txt\ngit commit -a -m \"initial version\"\necho zorglub > test.txt\ngit stash\ngit stash drop --help\nrefs/stash@{0} supprimé (ff100a8c2f1b7b00b9100b32d2a5dc19a8b0092a)\n\nAnd that was definitely not what I intended. Fortunately for me I had a\nbackup of that stashed diff somewhere else, but I was still surprised,\n\nbecause I was used to:\n\ngit $(SOMETHING) --help\n\nto do what I want.\n\nThis is probably because \"drop\" is a subcommand of \"stash\",\nas evidenced by:\n\ngit stash --help drop\n\nworking as intended (even if as as side effect of --help ignoring\nfurther parameters)\n\n-- \nVincent Legoll\n"},{"id":"261678","messageId":"20150520180132.GB14561@peff.net","threadId":"39385","inReplyTo":"CAEwRq=r=iMmnnzS2F_2rr9Tjem9khn1d=os3krjEjOg5iK5bww@mail.gmail.com","subject":"[PATCH] stash: complain about unknown flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-05-20T18:01:32Z","receivedAt":"2015-05-20T18:01:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The option parser for git-stash stuffs unknown flags into\nthe $FLAGS variable, where they can be accessed by the\nindividual commands. However, most commands do not even look\nat these extra flags, leading to unexpected results like\nthis:\n\n  $ git stash drop --help\n  Dropped refs/stash@{0} (e6cf6d80faf92bb7828f7b60c47fc61c03bd30a1)\n\nWe should notice the extra flags and bail. Rather than\nannotate each command to reject a non-empty $FLAGS variable,\nwe can notice that \"stash show\" is the only command that\nactually _wants_ arbitrary flags. So we switch the default\nmode to reject unknown flags, and let stash_show() opt into\nthe feature.\n\nReported-by: Vincent Legoll <vincent.legoll@gmail.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis takes away the immediate pain. We may also want to\nteach \"--help\" to the option. I guess we cannot do better\nthan just having it run \"git help stash\" in all cases (i.e.,\nwe have no way to get the help for a specific subcommand).\n\n git-stash.sh     | 6 +++++-\n t/t3903-stash.sh | 4 ++++\n 2 files changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 7911f30..c6f492c 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -301,6 +301,7 @@ list_stash () {\n }\n \n show_stash () {\n+\tALLOW_UNKNOWN_FLAGS=t\n \tassert_stash_like \"$@\"\n \n \tgit diff ${FLAGS:---stat} $b_commit $w_commit\n@@ -332,13 +333,14 @@ show_stash () {\n #\n #   GIT_QUIET is set to t if -q is specified\n #   INDEX_OPTION is set to --index if --index is specified.\n-#   FLAGS is set to the remaining flags\n+#   FLAGS is set to the remaining flags (if allowed)\n #\n # dies if:\n #   * too many revisions specified\n #   * no revision is specified and there is no stash stack\n #   * a revision is specified which cannot be resolve to a SHA1\n #   * a non-existent stash reference is specified\n+#   * unknown flags were set and ALLOW_UNKNOWN_FLAGS is not \"t\"\n #\n \n parse_flags_and_rev()\n@@ -372,6 +374,8 @@ parse_flags_and_rev()\n \t\t\t\tINDEX_OPTION=--index\n \t\t\t;;\n \t\t\t-*)\n+\t\t\t\ttest \"$ALLOW_UNKNOWN_FLAGS\" = t ||\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\tesac\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 0746eee..7396ca9 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -100,6 +100,10 @@ test_expect_success 'unstashing in a subdirectory' '\n \t)\n '\n \n+test_expect_success 'stash drop complains of extra options' '\n+\ttest_must_fail git stash drop --foo\n+'\n+\n test_expect_success 'drop top stash' '\n \tgit reset --hard &&\n \tgit stash list > stashlist1 &&\n-- \n2.4.1.396.g7ba6d7b\n"},{"id":"261680","messageId":"20150520181746.GC14561@peff.net","threadId":"39385","inReplyTo":"20150520180132.GB14561@peff.net","subject":"[PATCH 2/1] stash: recognize \"--help\" for subcommands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-05-20T18:17:46Z","receivedAt":"2015-05-20T18:17:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 20, 2015 at 02:01:32PM -0400, Jeff King wrote:\n\n> This takes away the immediate pain. We may also want to\n> teach \"--help\" to the option. I guess we cannot do better\n> than just having it run \"git help stash\" in all cases (i.e.,\n> we have no way to get the help for a specific subcommand).\n\nThat actually turns out to be pretty painless...\n\n-- >* --\nSubject: stash: recognize \"--help\" for subcommands\n\nIf you run \"git stash --help\", you get the help for stash\n(this magic is done by the git wrapper itself). But if you\nrun \"git stash drop --help\", you get an error. We\ncannot show help specific to \"stash drop\", of course, but we\ncan at least give the user the normal stash manpage.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-stash.sh | 11 +++++++++++\n 1 file changed, 11 insertions(+)\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex c6f492c..1f5ea87 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -219,6 +219,9 @@ save_stash () {\n \t\t-a|--all)\n \t\t\tuntracked=all\n \t\t\t;;\n+\t\t--help)\n+\t\t\tshow_help\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -307,6 +310,11 @@ show_stash () {\n \tgit diff ${FLAGS:---stat} $b_commit $w_commit\n }\n \n+show_help () {\n+\texec git help stash\n+\texit 1\n+}\n+\n #\n # Parses the remaining options looking for flags and\n # at most one revision defaulting to ${ref_stash}@{0}\n@@ -373,6 +381,9 @@ parse_flags_and_rev()\n \t\t\t--index)\n \t\t\t\tINDEX_OPTION=--index\n \t\t\t;;\n+\t\t\t--help)\n+\t\t\t\tshow_help\n+\t\t\t;;\n \t\t\t-*)\n \t\t\t\ttest \"$ALLOW_UNKNOWN_FLAGS\" = t ||\n \t\t\t\t\tdie \"$(eval_gettext \"unknown option: \\$opt\")\"\n-- \n2.4.1.396.g7ba6d7b\n"},{"id":"272648","messageId":"CAEwRq=p5AGE4ingzp8QFyUPetkiY7bgCyx6ZeaetzO5uQayXpg@mail.gmail.com","threadId":"39385","inReplyTo":"20150520180132.GB14561@peff.net","subject":"Re: [PATCH] stash: complain about unknown flags","fromName":"Vincent Legoll","fromEmail":"vincent.legoll@gmail.com","sentAt":"2015-11-01T08:16:40Z","receivedAt":"2015-11-01T08:16:40Z","isPatch":true,"sender":{"key":"vincent.legoll@gmail.com","avatar":null},"body":"Hello,\n\nOn Wed, May 20, 2015 at 8:01 PM, Jeff King <peff@peff.net> wrote:\n> The option parser for git-stash stuffs unknown flags into\n> the $FLAGS variable, where they can be accessed by the\n> individual commands. However, most commands do not even look\n> at these extra flags, leading to unexpected results like\n> this:\n>\n>   $ git stash drop --help\n>   Dropped refs/stash@{0} (e6cf6d80faf92bb7828f7b60c47fc61c03bd30a1)\n>\n> We should notice the extra flags and bail. Rather than\n> annotate each command to reject a non-empty $FLAGS variable,\n> we can notice that \"stash show\" is the only command that\n> actually _wants_ arbitrary flags. So we switch the default\n> mode to reject unknown flags, and let stash_show() opt into\n> the feature.\n\nBetter late than never, that does look good...\n\n-- \nVincent Legoll\n"},{"id":"272649","messageId":"CAEwRq=oXEkMpMHKdWHrg6uY-TTCP_H+TyX2++AKyq0BG3MU1gg@mail.gmail.com","threadId":"39385","inReplyTo":"20150520181746.GC14561@peff.net","subject":"Re: [PATCH 2/1] stash: recognize \"--help\" for subcommands","fromName":"Vincent Legoll","fromEmail":"vincent.legoll@gmail.com","sentAt":"2015-11-01T08:17:52Z","receivedAt":"2015-11-01T08:17:52Z","isPatch":true,"sender":{"key":"vincent.legoll@gmail.com","avatar":null},"body":"On Wed, May 20, 2015 at 8:17 PM, Jeff King <peff@peff.net> wrote:\n> On Wed, May 20, 2015 at 02:01:32PM -0400, Jeff King wrote:\n>\n>> This takes away the immediate pain. We may also want to\n>> teach \"--help\" to the option. I guess we cannot do better\n>> than just having it run \"git help stash\" in all cases (i.e.,\n>> we have no way to get the help for a specific subcommand).\n>\n> That actually turns out to be pretty painless...\n\nLooks OK, but this \"[PATCH 2/1]\" is fishy...\n\n-- \nVincent Legoll\n"}]}