Re: [PATCH v2 2/5] help: correct usage & behavior of "git help --guides"
flat view
From: Junio C Hamano <hidden>
Date: 2021-09-11 01:22:54
Ævar Arnfjörð Bjarmason [off-list ref] writes:
As noted in 65f98358c0c (builtin/help.c: add --guide option, 2013-04-02) and a133737b809 (doc: include --guide option description for "git help", 2013-04-02) which introduced the --guide option it cannot be combined with e.g. <command>.
Missing comman ',' between 'option' and 'it'.
Change both the usage string to reflect that, and test and assert for
I couldn't immediately tell which two things "both the usage string" is referring to. Presumably in the doc and "help -h" output?
this behavior in the command itself. Now that we assert this in code we don't need to exhaustively describe the previous confusing behavior in the documentation either, instead of silently ignoring the provided argument we'll now error out. The comment being removed was added in 15f7d494380 (builtin/help.c: split "-a" processing into two, 2013-04-02). The "Ignore any remaining args" part of it is now no longer applicable as explained above, let's just remove it entirely, it's rather obvious that if we're returning we're done.
Three sentences strung together with two commas ',' in between at the end of this paragraph. "A, let's B, it's C" -> "A. Let's B, because it's C" or something.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt index 568a0b606f3..cb8e3d4da9e 100644 --- a/Documentation/git-help.txt +++ b/Documentation/git-help.txt@@ -8,8 +8,9 @@ git-help - Display help information about Git SYNOPSIS -------- [verse] -'git help' [-a|--all [--[no-]verbose]] [-g|--guides] +'git help' [-a|--all [--[no-]verbose]] [[-i|--info] [-m|--man] [-w|--web]] [COMMAND|GUIDE] +'git help' [-g|--guides]
Good.
quoted hunk ↗ jump to hunk
diff --git a/builtin/help.c b/builtin/help.c index 44ea2798cda..51b18c291d8 100644 --- a/builtin/help.c +++ b/builtin/help.c@@ -59,8 +59,9 @@ static struct option builtin_help_options[] = { }; static const char * const builtin_help_usage[] = { - N_("git help [-a|--all] [-g|--guides] [--[no-]verbose]]\n" + N_("git help [-a|--all] [--[no-]verbose]]\n" " [[-i|--info] [-m|--man] [-w|--web]] [<command>]"), + N_("git help [-g|--guides]"),
Good. I still think -s|--long is cluttering than helping, though.
quoted hunk ↗ jump to hunk
+ /* Options that take no further arguments */ + if (argc && show_guides) + usage_msg_opt(_("--guides cannot be combined with other options"), + builtin_help_usage, builtin_help_options);
At this point we have called parse_options() and there is no further funky command line parsing involved, so non-zero argc does mean we have something --guides does not know what to do. Good.
quoted hunk ↗ jump to hunk
@@ -582,9 +588,6 @@ int cmd_help(int argc, const char **argv, const char *prefix) if (show_all || show_guides) { printf("%s\n", _(git_more_info_string)); - /* - * We're done. Ignore any remaining args - */ return 0; }diff --git a/t/t0012-help.sh b/t/t0012-help.sh index 5679e29c624..c3aa016fd30 100755 --- a/t/t0012-help.sh +++ b/t/t0012-help.sh@@ -34,6 +34,10 @@ test_expect_success 'basic help commands' ' git help -a >/dev/null ' +test_expect_success 'invalid usage' ' + test_expect_code 129 git help -g git-add
;-) I would have expected a bare "add" not "git-add" here, but it is OK. It is funny that "git help git-add" still works, but that is a bug that is unrelated to this patch.
quoted hunk ↗ jump to hunk
+' + test_expect_success "works for commands and guides by default" ' configure_help && git help status &&