Change the usage message emitted by "git stash --invalid-option" to
emit usage information for "git stash" in general, and not just for
the "push" command.
That we emitted the usage for just "push" in the case of the
subcommand not being explicitly specified was an unintentional
side-effect of how it was implemented. When it was converted to C in
d553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern
of having per-subcommand usage information was rightly continued. The
Git-stash.sh shellscript did not have that, and always printed the
equivalent of "git_stash_usage".
But in doing so the case of push being implicit and explicit was
conflated. A variable was added to track this in 8c3713cede7 (stash:
eliminate crude option parsing, 2020-02-17), but it did not update the
usage output accordingly. Let's do that.
This still leaves e.g. "git stash push -h" emitting the
"git_stash_usage" output, instead of "git_stash_push_usage". That
should be fixed, but is a much deeper misbehavior in parse_options()
not being aware of subcommands at all. I.e. in how
PARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in
commands such as "git stash".
Perhaps PARSE_OPT_KEEP_UNKNOWN should imply
PARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be
extended to fully handle these subcommand cases that we handle
manually in "git stash", "git commit-graph", "git multi-pack-index"
etc. All of those musings would be a much bigger change than this
isolated fix though, so let's leave that for some other time.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
builtin/stash.c | 1 +
t/t3903-stash.sh | 11 +++++++++++
2 files changed, 12 insertions(+)
Change the usage message emitted by "git stash --invalid-option" to
emit usage information for "git stash" in general, and not just for
the "push" command. I.e. before:
$ git stash --blah
error: unknown option `blah'
usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]
[--] [<pathspec>...]]
[...]
After:
$ git stash --blah
error: unknown option `blah'
usage: git stash list [<options>]
or: git stash show [<options>] [<stash>]
or: git stash drop [-q|--quiet] [<stash>]
or: git stash ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: git stash branch <branchname> [<stash>]
or: git stash clear
or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]
[--pathspec-from-file=<file> [--pathspec-file-nul]]
[--] [<pathspec>...]]
or: git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]
[...]
We'll only emit the former on the likes of "git stash push --blah"
now.
That we emitted the usage for just "push" in the case of the
subcommand not being explicitly specified was an unintentional
side-effect of how it was implemented. When it was converted to C in
d553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern
of having per-subcommand usage information was rightly continued. The
Git-stash.sh shellscript did not have that, and always printed the
equivalent of "git_stash_usage".
But in doing so the case of push being implicit and explicit was
conflated. A variable was added to track this in 8c3713cede7 (stash:
eliminate crude option parsing, 2020-02-17), but it did not update the
usage output accordingly.
This still leaves e.g. "git stash push -h" emitting the
"git_stash_usage" output, instead of "git_stash_push_usage". That
should be fixed, but is a much deeper misbehavior in parse_options()
not being aware of subcommands at all. I.e. in how
PARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in
commands such as "git stash".
Perhaps PARSE_OPT_KEEP_UNKNOWN should imply
PARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be
extended to fully handle these subcommand cases that we handle
manually in "git stash", "git commit-graph", "git multi-pack-index"
etc. All of those musings would be a much bigger change than this
isolated fix though, so let's leave that for some other time.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
A trivial patch for a small issue in "git stash" usage, unrelated and
non-conflicting with my other series to improve usage output alignment
at
https://lore.kernel.org/git/cover-v5-0.4-00000000000-20210921T132350Z-avarab@gmail.com/
Range-diff against v1:
1: 47c582f1218 ! 1: d1b9790904c stash: print the correct usage on "git stash [push] --invalid-option"
@@ Commit message
Change the usage message emitted by "git stash --invalid-option" to
emit usage information for "git stash" in general, and not just for
- the "push" command.
+ the "push" command. I.e. before:
+
+ $ git stash --blah
+ error: unknown option `blah'
+ usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]
+ [--] [<pathspec>...]]
+ [...]
+
+ After:
+
+ $ git stash --blah
+ error: unknown option `blah'
+ usage: git stash list [<options>]
+ or: git stash show [<options>] [<stash>]
+ or: git stash drop [-q|--quiet] [<stash>]
+ or: git stash ( pop | apply ) [--index] [-q|--quiet] [<stash>]
+ or: git stash branch <branchname> [<stash>]
+ or: git stash clear
+ or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]
+ [--pathspec-from-file=<file> [--pathspec-file-nul]]
+ [--] [<pathspec>...]]
+ or: git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
+ [...]
+
+ We'll only emit the former on the likes of "git stash push --blah"
+ now.
That we emitted the usage for just "push" in the case of the
subcommand not being explicitly specified was an unintentional
@@ Commit message
But in doing so the case of push being implicit and explicit was
conflated. A variable was added to track this in 8c3713cede7 (stash:
eliminate crude option parsing, 2020-02-17), but it did not update the
- usage output accordingly. Let's do that.
+ usage output accordingly.
This still leaves e.g. "git stash push -h" emitting the
"git_stash_usage" output, instead of "git_stash_push_usage". That
@@ t/t3903-stash.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
-+test_expect_success 'usage' '
-+ test_expect_code 129 git stash -h >out &&
-+ grep "or: git stash" out &&
++test_expect_success 'usage on cmd and subcommand invalid option' '
++ test_expect_code 129 git stash --invalid-option 2>usage &&
++ grep "or: git stash" usage &&
+
-+ test_expect_code 129 git stash --invalid-option >out 2>err &&
-+ grep "or: git stash" err &&
++ test_expect_code 129 git stash push --invalid-option 2>usage &&
++ ! grep "or: git stash" usage
++'
++
++test_expect_success 'usage on main command -h emits a summary of subcommands' '
++ test_expect_code 129 git stash -h >usage &&
++ grep -F "usage: git stash list" usage &&
++ grep -F "or: git stash show" usage
++'
+
-+ test_expect_code 129 git stash push --invalid-option >out 2>err &&
-+ ! grep "or: git stash" err
++test_expect_failure 'usage for subcommands should emit subcommand usage' '
++ test_expect_code 129 git stash push -h >usage &&
++ grep -F "usage: git stash [push" usage
+'
+
diff_cmp () {
builtin/stash.c | 1 +
t/t3903-stash.sh | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
Change the usage message emitted by "git stash --invalid-option" to
emit usage information for "git stash" in general, and not just for
the "push" command. I.e. before:
$ git stash --invalid-option
error: unknown option `invalid-option'
usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]
[--] [<pathspec>...]]
[...]
After:
$ git stash --invalid-option
error: unknown option `invalid-option'
usage: git stash list [<options>]
or: git stash show [<options>] [<stash>]
or: git stash drop [-q|--quiet] [<stash>]
or: git stash ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: git stash branch <branchname> [<stash>]
or: git stash clear
or: git stash [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [-m|--message <message>]
[--pathspec-from-file=<file> [--pathspec-file-nul]]
[--] [<pathspec>...]]
or: git stash save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]
[-u|--include-untracked] [-a|--all] [<message>]
[...]
That we emitted the usage for just "push" in the case of the
subcommand not being explicitly specified was an unintentional
side-effect of how it was implemented. When it was converted to C in
d553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern
of having per-subcommand usage information was rightly continued. The
"git-stash.sh" shellscript did not have that, and always printed the
equivalent of "git_stash_usage".
But in doing so the case of push being implicit and explicit was
conflated. A variable was added to track this in 8c3713cede7 (stash:
eliminate crude option parsing, 2020-02-17), but it did not update the
usage output accordingly.
This still leaves e.g. "git stash push -h" emitting the
"git_stash_usage" output, instead of "git_stash_push_usage". That
should be fixed, but is a much deeper misbehavior in parse_options()
not being aware of subcommands at all. I.e. in how
PARSE_OPT_KEEP_UNKNOWN and PARSE_OPT_NO_INTERNAL_HELP combine in
commands such as "git stash".
Perhaps PARSE_OPT_KEEP_UNKNOWN should imply
PARSE_OPT_NO_INTERNAL_HELP, or better yet parse_options() should be
extended to fully handle these subcommand cases that we handle
manually in "git stash", "git commit-graph", "git multi-pack-index"
etc. All of those musings would be a much bigger change than this
isolated fix though, so let's leave that for some other time.
Signed-off-by: Ævar Arnfjörð Bjarmason <redacted>
---
I think this was ready the first couple of times, but it wasn't picked
up. Hopefully this relatively trivial UX fix can land this time
around.
The only updates are some commit message nits, including updating the
examples for the now-landed changes to align the parse-options output.
Range-diff against v2:
1: d1b9790904c ! 1: 6b33b104c84 stash: print the correct usage on "git stash [push] --invalid-option"
@@ Metadata
Author: Ævar Arnfjörð Bjarmason [off-list ref]
## Commit message ##
- stash: print the correct usage on "git stash [push] --invalid-option"
+ stash: don't show "git stash push" usage on bad "git stash" usage
Change the usage message emitted by "git stash --invalid-option" to
emit usage information for "git stash" in general, and not just for
the "push" command. I.e. before:
- $ git stash --blah
- error: unknown option `blah'
+ $ git stash --invalid-option
+ error: unknown option `invalid-option'
usage: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [-m|--message <message>]
- [--] [<pathspec>...]]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]
+ [--] [<pathspec>...]]
[...]
After:
- $ git stash --blah
- error: unknown option `blah'
+ $ git stash --invalid-option
+ error: unknown option `invalid-option'
usage: git stash list [<options>]
or: git stash show [<options>] [<stash>]
or: git stash drop [-q|--quiet] [<stash>]
or: git stash ( pop | apply ) [--index] [-q|--quiet] [<stash>]
or: git stash branch <branchname> [<stash>]
or: git stash clear
- or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [-m|--message <message>]
- [--pathspec-from-file=<file> [--pathspec-file-nul]]
- [--] [<pathspec>...]]
- or: git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
- [-u|--include-untracked] [-a|--all] [<message>]
+ or: git stash [push [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [-m|--message <message>]
+ [--pathspec-from-file=<file> [--pathspec-file-nul]]
+ [--] [<pathspec>...]]
+ or: git stash save [-p|--patch] [-S|--staged] [-k|--[no-]keep-index] [-q|--quiet]
+ [-u|--include-untracked] [-a|--all] [<message>]
[...]
- We'll only emit the former on the likes of "git stash push --blah"
- now.
-
That we emitted the usage for just "push" in the case of the
subcommand not being explicitly specified was an unintentional
side-effect of how it was implemented. When it was converted to C in
d553f538b8a (stash: convert push to builtin, 2019-02-25) the pattern
of having per-subcommand usage information was rightly continued. The
- Git-stash.sh shellscript did not have that, and always printed the
+ "git-stash.sh" shellscript did not have that, and always printed the
equivalent of "git_stash_usage".
But in doing so the case of push being implicit and explicit was
builtin/stash.c | 1 +
t/t3903-stash.sh | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)