{"thread":{"id":"56424","subject":"[PATCH] stash: print the correct usage on \"git stash [push] --invalid-option\"","startedAt":"2021-09-01T11:20:16Z","lastAt":"2021-12-16T22:05:48Z","messageCount":5,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"434412","messageId":"patch-1.1-47c582f1218-20210901T111930Z-avarab@gmail.com","threadId":"56424","inReplyTo":null,"subject":"[PATCH] stash: print the correct usage on \"git stash [push] --invalid-option\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-01T11:19:54Z","receivedAt":"2021-09-01T11:20:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the usage message emitted by \"git stash --invalid-option\" to\nemit usage information for \"git stash\" in general, and not just for\nthe \"push\" command.\n\nThat we emitted the usage for just \"push\" in the case of the\nsubcommand not being explicitly specified was an unintentional\nside-effect of how it was implemented. When it was converted to C in\nd553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern\nof having per-subcommand usage information was rightly continued. The\nGit-stash.sh shellscript did not have that, and always printed the\nequivalent of \"git_stash_usage\".\n\nBut in doing so the case of push being implicit and explicit was\nconflated. A variable was added to track this in 8c3713cede7 (stash:\neliminate crude option parsing, 2020-02-17), but it did not update the\nusage output accordingly. Let's do that.\n\nThis still leaves e.g. \"git stash push -h\" emitting the\n\"git_stash_usage\" output, instead of \"git_stash_push_usage\". That\nshould be fixed, but is a much deeper misbehavior in parse_options()\nnot being aware of subcommands at all. I.e. in how\nPARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in\ncommands such as \"git stash\".\n\nPerhaps PARSE_OPT_KEEP_UNKNOWN should imply\nPARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be\nextended to fully handle these subcommand cases that we handle\nmanually in \"git stash\", \"git commit-graph\", \"git multi-pack-index\"\netc. All of those musings would be a much bigger change than this\nisolated fix though, so let's leave that for some other time.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/stash.c | 1 +\n t/t3903-stash.sh | 11 +++++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 8f42360ca91..cf62c3b3cd8 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1608,6 +1608,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \tif (argc) {\n \t\tforce_assume = !strcmp(argv[0], \"-p\");\n \t\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t\t push_assumed ? git_stash_usage :\n \t\t\t\t git_stash_push_usage,\n \t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n \t}\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 873aa56e359..ba9582bdee5 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -10,6 +10,17 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n \n+test_expect_success 'usage' '\n+\ttest_expect_code 129 git stash -h >out &&\n+\tgrep \"or: git stash\" out &&\n+\n+\ttest_expect_code 129 git stash --invalid-option >out 2>err &&\n+\tgrep \"or: git stash\" err &&\n+\n+\ttest_expect_code 129 git stash push --invalid-option >out 2>err &&\n+\t! grep \"or: git stash\" err\n+'\n+\n diff_cmp () {\n \tfor i in \"$1\" \"$2\"\n \tdo\n-- \n2.33.0.807.gf14ecf9c2e9\n\n"},{"id":"436614","messageId":"patch-v2-1.1-d1b9790904c-20210921T134436Z-avarab@gmail.com","threadId":"56424","inReplyTo":"patch-1.1-47c582f1218-20210901T111930Z-avarab@gmail.com","subject":"[PATCH v2] stash: print the correct usage on \"git stash [push] --invalid-option\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-21T13:46:07Z","receivedAt":"2021-09-21T13:46:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the usage message emitted by \"git stash --invalid-option\" to\nemit usage information for \"git stash\" in general, and not just for\nthe \"push\" command. I.e. before:\n\n $ git stash --blah\n error: unknown option `blah'\n usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] [-m|--message ]\n [--] [...]]\n [...]\n\nAfter:\n\n $ git stash --blah\n error: unknown option `blah'\n usage: git stash list []\n or: git stash show [] []\n or: git stash drop [-q|--quiet] []\n or: git stash ( pop | apply ) [--index] [-q|--quiet] []\n or: git stash branch []\n or: git stash clear\n or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] [-m|--message ]\n [--pathspec-from-file= [--pathspec-file-nul]]\n [--] [...]]\n or: git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] []\n [...]\n\nWe'll only emit the former on the likes of \"git stash push --blah\"\nnow.\n\nThat we emitted the usage for just \"push\" in the case of the\nsubcommand not being explicitly specified was an unintentional\nside-effect of how it was implemented. When it was converted to C in\nd553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern\nof having per-subcommand usage information was rightly continued. The\nGit-stash.sh shellscript did not have that, and always printed the\nequivalent of \"git_stash_usage\".\n\nBut in doing so the case of push being implicit and explicit was\nconflated. A variable was added to track this in 8c3713cede7 (stash:\neliminate crude option parsing, 2020-02-17), but it did not update the\nusage output accordingly.\n\nThis still leaves e.g. \"git stash push -h\" emitting the\n\"git_stash_usage\" output, instead of \"git_stash_push_usage\". That\nshould be fixed, but is a much deeper misbehavior in parse_options()\nnot being aware of subcommands at all. I.e. in how\nPARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in\ncommands such as \"git stash\".\n\nPerhaps PARSE_OPT_KEEP_UNKNOWN should imply\nPARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be\nextended to fully handle these subcommand cases that we handle\nmanually in \"git stash\", \"git commit-graph\", \"git multi-pack-index\"\netc. All of those musings would be a much bigger change than this\nisolated fix though, so let's leave that for some other time.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nA trivial patch for a small issue in \"git stash\" usage, unrelated and\nnon-conflicting with my other series to improve usage output alignment\nat\nhttps://lore.kernel.org/git/cover-v5-0.4-00000000000-20210921T132350Z-avarab@gmail.com/\n\nRange-diff against v1:\n1: 47c582f1218 ! 1: d1b9790904c stash: print the correct usage on \"git stash [push] --invalid-option\"\n @@ Commit message\n \n Change the usage message emitted by \"git stash --invalid-option\" to\n emit usage information for \"git stash\" in general, and not just for\n - the \"push\" command.\n + the \"push\" command. I.e. before:\n +\n + $ git stash --blah\n + error: unknown option `blah'\n + usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n + [-u|--include-untracked] [-a|--all] [-m|--message ]\n + [--] [...]]\n + [...]\n +\n + After:\n +\n + $ git stash --blah\n + error: unknown option `blah'\n + usage: git stash list []\n + or: git stash show [] []\n + or: git stash drop [-q|--quiet] []\n + or: git stash ( pop | apply ) [--index] [-q|--quiet] []\n + or: git stash branch []\n + or: git stash clear\n + or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n + [-u|--include-untracked] [-a|--all] [-m|--message ]\n + [--pathspec-from-file= [--pathspec-file-nul]]\n + [--] [...]]\n + or: git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n + [-u|--include-untracked] [-a|--all] []\n + [...]\n +\n + We'll only emit the former on the likes of \"git stash push --blah\"\n + now.\n \n That we emitted the usage for just \"push\" in the case of the\n subcommand not being explicitly specified was an unintentional\n @@ Commit message\n But in doing so the case of push being implicit and explicit was\n conflated. A variable was added to track this in 8c3713cede7 (stash:\n eliminate crude option parsing, 2020-02-17), but it did not update the\n - usage output accordingly. Let's do that.\n + usage output accordingly.\n \n This still leaves e.g. \"git stash push -h\" emitting the\n \"git_stash_usage\" output, instead of \"git_stash_push_usage\". That\n @@ t/t3903-stash.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n \n -+test_expect_success 'usage' '\n -+\ttest_expect_code 129 git stash -h >out &&\n -+\tgrep \"or: git stash\" out &&\n ++test_expect_success 'usage on cmd and subcommand invalid option' '\n ++\ttest_expect_code 129 git stash --invalid-option 2>usage &&\n ++\tgrep \"or: git stash\" usage &&\n +\n -+\ttest_expect_code 129 git stash --invalid-option >out 2>err &&\n -+\tgrep \"or: git stash\" err &&\n ++\ttest_expect_code 129 git stash push --invalid-option 2>usage &&\n ++\t! grep \"or: git stash\" usage\n ++'\n ++\n ++test_expect_success 'usage on main command -h emits a summary of subcommands' '\n ++\ttest_expect_code 129 git stash -h >usage &&\n ++\tgrep -F \"usage: git stash list\" usage &&\n ++\tgrep -F \"or: git stash show\" usage\n ++'\n +\n -+\ttest_expect_code 129 git stash push --invalid-option >out 2>err &&\n -+\t! grep \"or: git stash\" err\n ++test_expect_failure 'usage for subcommands should emit subcommand usage' '\n ++\ttest_expect_code 129 git stash push -h >usage &&\n ++\tgrep -F \"usage: git stash [push\" usage\n +'\n +\n diff_cmp () {\n\n builtin/stash.c | 1 +\n t/t3903-stash.sh | 19 +++++++++++++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 8f42360ca91..cf62c3b3cd8 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1608,6 +1608,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \tif (argc) {\n \t\tforce_assume = !strcmp(argv[0], \"-p\");\n \t\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t\t push_assumed ? git_stash_usage :\n \t\t\t\t git_stash_push_usage,\n \t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n \t}\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 873aa56e359..655f9be4352 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -10,6 +10,25 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n \n+test_expect_success 'usage on cmd and subcommand invalid option' '\n+\ttest_expect_code 129 git stash --invalid-option 2>usage &&\n+\tgrep \"or: git stash\" usage &&\n+\n+\ttest_expect_code 129 git stash push --invalid-option 2>usage &&\n+\t! grep \"or: git stash\" usage\n+'\n+\n+test_expect_success 'usage on main command -h emits a summary of subcommands' '\n+\ttest_expect_code 129 git stash -h >usage &&\n+\tgrep -F \"usage: git stash list\" usage &&\n+\tgrep -F \"or: git stash show\" usage\n+'\n+\n+test_expect_failure 'usage for subcommands should emit subcommand usage' '\n+\ttest_expect_code 129 git stash push -h >usage &&\n+\tgrep -F \"usage: git stash [push\" usage\n+'\n+\n diff_cmp () {\n \tfor i in \"$1\" \"$2\"\n \tdo\n-- \n2.33.0.1098.gf02a64c1a2d\n\n"},{"id":"444205","messageId":"patch-v3-1.1-6b33b104c84-20211216T125317Z-avarab@gmail.com","threadId":"56424","inReplyTo":"patch-v2-1.1-d1b9790904c-20210921T134436Z-avarab@gmail.com","subject":"[PATCH v3] stash: don't show \"git stash push\" usage on bad \"git stash\" usage","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T12:54:21Z","receivedAt":"2021-12-16T12:54:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the usage message emitted by \"git stash --invalid-option\" to\nemit usage information for \"git stash\" in general, and not just for\nthe \"push\" command. I.e. before:\n\n $ git stash --invalid-option\n error: unknown option `invalid-option'\n usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] [-m|--message ]\n [--] [...]]\n [...]\n\nAfter:\n\n $ git stash --invalid-option\n error: unknown option `invalid-option'\n usage: git stash list []\n or: git stash show [] []\n or: git stash drop [-q|--quiet] []\n or: git stash ( pop | apply ) [--index] [-q|--quiet] []\n or: git stash branch []\n or: git stash clear\n or: git stash [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] [-m|--message ]\n [--pathspec-from-file= [--pathspec-file-nul]]\n [--] [...]]\n or: git stash save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n [-u|--include-untracked] [-a|--all] []\n [...]\n\nThat we emitted the usage for just \"push\" in the case of the\nsubcommand not being explicitly specified was an unintentional\nside-effect of how it was implemented. When it was converted to C in\nd553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern\nof having per-subcommand usage information was rightly continued. The\n\"git-stash.sh\" shellscript did not have that, and always printed the\nequivalent of \"git_stash_usage\".\n\nBut in doing so the case of push being implicit and explicit was\nconflated. A variable was added to track this in 8c3713cede7 (stash:\neliminate crude option parsing, 2020-02-17), but it did not update the\nusage output accordingly.\n\nThis still leaves e.g. \"git stash push -h\" emitting the\n\"git_stash_usage\" output, instead of \"git_stash_push_usage\". That\nshould be fixed, but is a much deeper misbehavior in parse_options()\nnot being aware of subcommands at all. I.e. in how\nPARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in\ncommands such as \"git stash\".\n\nPerhaps PARSE_OPT_KEEP_UNKNOWN should imply\nPARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be\nextended to fully handle these subcommand cases that we handle\nmanually in \"git stash\", \"git commit-graph\", \"git multi-pack-index\"\netc. All of those musings would be a much bigger change than this\nisolated fix though, so let's leave that for some other time.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nI think this was ready the first couple of times, but it wasn't picked\nup. Hopefully this relatively trivial UX fix can land this time\naround.\n\nThe only updates are some commit message nits, including updating the\nexamples for the now-landed changes to align the parse-options output.\n\nRange-diff against v2:\n1: d1b9790904c ! 1: 6b33b104c84 stash: print the correct usage on \"git stash [push] --invalid-option\"\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - stash: print the correct usage on \"git stash [push] --invalid-option\"\n + stash: don't show \"git stash push\" usage on bad \"git stash\" usage\n \n Change the usage message emitted by \"git stash --invalid-option\" to\n emit usage information for \"git stash\" in general, and not just for\n the \"push\" command. I.e. before:\n \n - $ git stash --blah\n - error: unknown option `blah'\n + $ git stash --invalid-option\n + error: unknown option `invalid-option'\n usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n - [-u|--include-untracked] [-a|--all] [-m|--message ]\n - [--] [...]]\n + [-u|--include-untracked] [-a|--all] [-m|--message ]\n + [--] [...]]\n [...]\n \n After:\n \n - $ git stash --blah\n - error: unknown option `blah'\n + $ git stash --invalid-option\n + error: unknown option `invalid-option'\n usage: git stash list []\n or: git stash show [] []\n or: git stash drop [-q|--quiet] []\n or: git stash ( pop | apply ) [--index] [-q|--quiet] []\n or: git stash branch []\n or: git stash clear\n - or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n - [-u|--include-untracked] [-a|--all] [-m|--message ]\n - [--pathspec-from-file= [--pathspec-file-nul]]\n - [--] [...]]\n - or: git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n - [-u|--include-untracked] [-a|--all] []\n + or: git stash [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n + [-u|--include-untracked] [-a|--all] [-m|--message ]\n + [--pathspec-from-file= [--pathspec-file-nul]]\n + [--] [...]]\n + or: git stash save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]\n + [-u|--include-untracked] [-a|--all] []\n [...]\n \n - We'll only emit the former on the likes of \"git stash push --blah\"\n - now.\n -\n That we emitted the usage for just \"push\" in the case of the\n subcommand not being explicitly specified was an unintentional\n side-effect of how it was implemented. When it was converted to C in\n d553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern\n of having per-subcommand usage information was rightly continued. The\n - Git-stash.sh shellscript did not have that, and always printed the\n + \"git-stash.sh\" shellscript did not have that, and always printed the\n equivalent of \"git_stash_usage\".\n \n But in doing so the case of push being implicit and explicit was\n\n builtin/stash.c | 1 +\n t/t3903-stash.sh | 19 +++++++++++++++++++\n 2 files changed, 20 insertions(+)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex 18c812bbe03..5462840a073 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1681,6 +1681,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n \tif (argc) {\n \t\tforce_assume = !strcmp(argv[0], \"-p\");\n \t\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t\t push_assumed ? git_stash_usage :\n \t\t\t\t git_stash_push_usage,\n \t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n \t}\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 2c66cfbc3b7..b17c52d8807 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -10,6 +10,25 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n \n+test_expect_success 'usage on cmd and subcommand invalid option' '\n+\ttest_expect_code 129 git stash --invalid-option 2>usage &&\n+\tgrep \"or: git stash\" usage &&\n+\n+\ttest_expect_code 129 git stash push --invalid-option 2>usage &&\n+\t! grep \"or: git stash\" usage\n+'\n+\n+test_expect_success 'usage on main command -h emits a summary of subcommands' '\n+\ttest_expect_code 129 git stash -h >usage &&\n+\tgrep -F \"usage: git stash list\" usage &&\n+\tgrep -F \"or: git stash show\" usage\n+'\n+\n+test_expect_failure 'usage for subcommands should emit subcommand usage' '\n+\ttest_expect_code 129 git stash push -h >usage &&\n+\tgrep -F \"usage: git stash [push\" usage\n+'\n+\n diff_cmp () {\n \tfor i in \"$1\" \"$2\"\n \tdo\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444269","messageId":"xmqqczlw8axt.fsf@gitster.g","threadId":"56424","inReplyTo":"patch-v3-1.1-6b33b104c84-20211216T125317Z-avarab@gmail.com","subject":"Re: [PATCH v3] stash: don't show \"git stash push\" usage on bad \"git stash\" usage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-16T22:03:10Z","receivedAt":"2021-12-16T22:03:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> diff --git a/builtin/stash.c b/builtin/stash.c\n> index 18c812bbe03..5462840a073 100644\n> --- a/builtin/stash.c\n> +++ b/builtin/stash.c\n> @@ -1681,6 +1681,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,\n> \tif (argc) {\n> \t\tforce_assume = !strcmp(argv[0], \"-p\");\n> \t\targc = parse_options(argc, argv, prefix, options,\n> +\t\t\t\t push_assumed ? git_stash_usage :\n> \t\t\t\t git_stash_push_usage,\n> \t\t\t\t PARSE_OPT_KEEP_DASHDASH);\n> \t}\n\nYeah, \"git stash\" having the implicit default to \"push\" may have\nbeen convenient for its original intended use case (i.e. clear the\nworking area with as little typing and effort as possible), but\nwithout a care like this one, it would result in surprises.\n\nLooks good, will queue.\n\n\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index 2c66cfbc3b7..b17c52d8807 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -10,6 +10,25 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n> \n> . ./test-lib.sh\n> \n> +test_expect_success 'usage on cmd and subcommand invalid option' '\n> +\ttest_expect_code 129 git stash --invalid-option 2>usage &&\n> +\tgrep \"or: git stash\" usage &&\n> +\n> +\ttest_expect_code 129 git stash push --invalid-option 2>usage &&\n> +\t! grep \"or: git stash\" usage\n> +'\n> +\n> +test_expect_success 'usage on main command -h emits a summary of subcommands' '\n> +\ttest_expect_code 129 git stash -h >usage &&\n> +\tgrep -F \"usage: git stash list\" usage &&\n> +\tgrep -F \"or: git stash show\" usage\n> +'\n> +\n> +test_expect_failure 'usage for subcommands should emit subcommand usage' '\n> +\ttest_expect_code 129 git stash push -h >usage &&\n> +\tgrep -F \"usage: git stash [push\" usage\n> +'\n> +\n> diff_cmp () {\n> \tfor i in \"$1\" \"$2\"\n> \tdo\n"},{"id":"444270","messageId":"xmqq8rwk8atn.fsf@gitster.g","threadId":"56424","inReplyTo":"patch-v3-1.1-6b33b104c84-20211216T125317Z-avarab@gmail.com","subject":"Re: [PATCH v3] stash: don't show \"git stash push\" usage on bad \"git stash\" usage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-16T22:05:40Z","receivedAt":"2021-12-16T22:05:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This still leaves e.g. \"git stash push -h\" emitting the\n> \"git_stash_usage\" output, instead of \"git_stash_push_usage\". That\n> should be fixed, but is a much deeper misbehavior in parse_options()\n> not being aware of subcommands at all. I.e. in how\n> PARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in\n> commands such as \"git stash\".\n\nI do not necessarily think that is misbehaviour to begin with, but\nin any case, it is good to keep the initial fix focused on a small\nand clear case, like the one the patch addresses.\n"}]}