threads / patch / 57159

patch, 7 partshelp: test and fix small "help -g" regression

Subject: [PATCH 0/7] help: test and fix small "help -g" regression

## tl;dr

29 messages between Dec 28, 2021 and Feb 23, 2022. Diffs are folded; open one to read it.

replies: 28people: 3as markdown or json

Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · lore

Fix a trivial regression in 17b3e515050 (Merge branch 'nd/command-list' into nd/complete-config-vars, 2018-05-29) where we'd overly whitespace pad "git help -g", since the codepath moved to a function that assumed it needed to "\n\n"-pad multiple items being emitted, so we'd print an extra leading newline when printing one item (as opposed to "git help -a").

In doing so I wanted to add regression tests, and to do that we first need to make the format stable ("git help -a" will change depending on whatever git-* you have in $PATH).

These are things I noticed when re-rolling my no-yet-picked-up series to move Documentation/technical/* to manpages[1]. An unsubmitted re-roll of this series depends on this one. I'll hold off on a re-submission of it until this series has graduated.

1. https://lore.kernel.org/git/cover-v2-0.5-00000000000-20211212T194047Z-avarab@gmail.com/
Ævar Arnfjörð Bjarmason (7):
  help doc: add missing "]" to "[-a|--all]"
  help.c: use puts() instead of printf{,_ln}() for consistency
  help tests: test "git" and "git help [-a|-g] spacing
  help.c: split up list_all_cmds_help() function
  help: error if [-a|-g|-c] and [-i|-m|-w] are combined
  help: add --no-[external-commands|aliases] for use with --all
  help: don't print "\n" before single-section output
 Documentation/git-help.txt | 12 ++++-
 builtin/help.c             | 34 ++++++++++++--
 help.c                     | 37 ++++++++++++----
 help.h                     |  2 +-
 t/t0012-help.sh            | 91 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 161 insertions(+), 15 deletions(-)
-- 
2.34.1.1257.g2af47340c7b
Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 1/7] help doc: add missing "]" to "[-a|--all]"

Add a missing "]" to documentation added in 63eae83f8f3 (help: add "-a --verbose" to list all commands with synopsis, 2018-05-20). This made it seem as though "--[no-]verbose" can only be provided with "--all", not "-a". The corresponding usage information in the C code ("builtin_help_usage") does not have the same problem.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-help.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Documentation/git-help.txt +1 −1
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 44ea63cc6d3..cf1d53e9499 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
 SYNOPSIS
 --------
 [verse]
-'git help' [-a|--all [--[no-]verbose]]
+'git help' [-a|--all] [--[no-]verbose]
 	   [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
 'git help' [-g|--guides]
 'git help' [-c|--config]
-- 
2.34.1.1257.g2af47340c7b
Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 2/7] help.c: use puts() instead of printf{,_ln}() for consistency

Change code in "help.c" that used printf_ln() without format specifiers to use puts() instead, as other existing code in the file does. Let's also change related code to use puts() instead of the equivalent of calling "printf" with a "%s\n" format.

This formatting-only change will make a subsequent functional change easier to read, as it'll be changing code that's consistently using the same functions to do the same things.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 help.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
Show changes to help.c +4 −3
diff --git a/help.c b/help.c
index 973e47cdc30..0ba9b866f03 100644
--- a/help.c
+++ b/help.c
@@ -124,7 +124,8 @@ static void print_cmd_by_category(const struct category_description *catdesc,
 		uint32_t mask = catdesc[i].category;
 		const char *desc = catdesc[i].desc;
 
-		printf("\n%s\n", _(desc));
+		putchar('\n');
+		puts(_(desc));
 		print_command_list(cmds, mask, longest);
 	}
 	free(cmds);
@@ -317,7 +318,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
 	}
 
 	if (other_cmds->cnt) {
-		printf_ln(_("git commands available from elsewhere on your $PATH"));
+		puts(_("git commands available from elsewhere on your $PATH"));
 		putchar('\n');
 		pretty_print_cmdnames(other_cmds, colopts);
 		putchar('\n');
@@ -439,7 +440,7 @@ void list_all_cmds_help(void)
 	struct cmdname_help *aliases;
 	int i, longest;
 
-	printf_ln(_("See 'git help <command>' to read about a specific subcommand"));
+	puts(_("See 'git help <command>' to read about a specific subcommand"));
 	print_cmd_by_category(main_categories, &longest);
 
 	list_all_other_cmds(&others);
-- 
2.34.1.1257.g2af47340c7b
Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 3/7] help tests: test "git" and "git help [-a|-g] spacing

There's logic in "help.c"'s "print_cmd_by_category()" to emit "help" output with particular spacing, which doesn't make much sense when emitting only one section with "help -g".

Let's add tests for the current spacing in preparation for a subsequent whitespace formatting fix, and make sure that that fix doesn't cause regressions for the "git" and "git help" output.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0012-help.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
Show changes to t/t0012-help.sh +45 −0
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 91b68c74a15..6ac293c19ed 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -138,6 +138,51 @@ test_expect_success 'git help --config-sections-for-completion' '
 	test_cmp human.munged sections
 '
 
+test_section_spacing () {
+	cat >expect &&
+	"$@" >out &&
+	grep -E "(^[^ ]|^$)" out >actual
+}
+
+test_section_spacing_trailer () {
+	test_section_spacing "$@" &&
+	test_expect_code 1 git >out &&
+	sed -n '/list available subcommands/,$p' <out >>expect
+}
+
+
+for cmd in git "git help"
+do
+	test_expect_success "'$cmd' section spacing" '
+		test_section_spacing_trailer git help <<-\EOF &&
+		usage: git [--version] [--help] [-C <path>] [-c <name>=<value>]
+
+		These are common Git commands used in various situations:
+
+		start a working area (see also: git help tutorial)
+
+		work on the current change (see also: git help everyday)
+
+		examine the history and state (see also: git help revisions)
+
+		grow, mark and tweak your common history
+
+		collaborate (see also: git help workflows)
+
+		EOF
+		test_cmp expect actual
+	'
+done
+
+test_expect_success "'git help -g' section spacing" '
+	test_section_spacing_trailer git help -g <<-\EOF &&
+
+	The Git concept guides are:
+
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'generate builtin list' '
 	git --list-cmds=builtins >builtins
 '
-- 
2.34.1.1257.g2af47340c7b
Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 4/7] help.c: split up list_all_cmds_help() function

Split up the listing of commands and aliases from list_all_cmds_help(). This will make a subsequent functional change smaller.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 help.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)
Show changes to help.c +20 −7
diff --git a/help.c b/help.c
index 0ba9b866f03..ef1aa26efa6 100644
--- a/help.c
+++ b/help.c
@@ -433,15 +433,10 @@ static int get_alias(const char *var, const char *value, void *data)
 	return 0;
 }
 
-void list_all_cmds_help(void)
+static void list_all_cmds_help_external_commands(void)
 {
 	struct string_list others = STRING_LIST_INIT_DUP;
-	struct string_list alias_list = STRING_LIST_INIT_DUP;
-	struct cmdname_help *aliases;
-	int i, longest;
-
-	puts(_("See 'git help <command>' to read about a specific subcommand"));
-	print_cmd_by_category(main_categories, &longest);
+	int i;
 
 	list_all_other_cmds(&others);
 	if (others.nr)
@@ -449,6 +444,13 @@ void list_all_cmds_help(void)
 	for (i = 0; i < others.nr; i++)
 		printf("   %s\n", others.items[i].string);
 	string_list_clear(&others, 0);
+}
+
+static void list_all_cmds_help_aliases(int longest)
+{
+	struct string_list alias_list = STRING_LIST_INIT_DUP;
+	struct cmdname_help *aliases;
+	int i;
 
 	git_config(get_alias, &alias_list);
 	string_list_sort(&alias_list);
@@ -474,6 +476,17 @@ void list_all_cmds_help(void)
 	string_list_clear(&alias_list, 1);
 }
 
+void list_all_cmds_help(void)
+{
+	int longest;
+
+	puts(_("See 'git help <command>' to read about a specific subcommand"));
+	print_cmd_by_category(main_categories, &longest);
+
+	list_all_cmds_help_external_commands();
+	list_all_cmds_help_aliases(longest);
+}
+
 int is_in_cmdlist(struct cmdnames *c, const char *s)
 {
 	int i;
-- 
2.34.1.1257.g2af47340c7b
Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 5/7] help: error if [-a|-g|-c] and [-i|-m|-w] are combined

Add more sanity checking to "git help" usage by erroring out if these man viewer options are combined with incompatible command-modes that will never use these documentation viewers.

This continues the work started in d35d03cf93e (help: simplify by moving to OPT_CMDMODE(), 2021-09-22) of adding more sanity checking to "git help". Doing this allows us to clarify the "SYNOPSIS" in the documentation, and the "git help -h" output.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-help.txt |  2 +-
 builtin/help.c             | 20 ++++++++++++++++++--
 t/t0012-help.sh            | 10 ++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)
Show changes to 3 files +29 −3

Documentation/git-help.txt, builtin/help.c, t/t0012-help.sh

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index cf1d53e9499..61d52d30f6c 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git help' [-a|--all] [--[no-]verbose]
-	   [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
+'git help' [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
 'git help' [-g|--guides]
 'git help' [-c|--config]
 
diff --git a/builtin/help.c b/builtin/help.c
index d387131dd83..125f50f1bb0 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -75,8 +75,8 @@ static struct option builtin_help_options[] = {
 };
 
 static const char * const builtin_help_usage[] = {
-	N_("git help [-a|--all] [--[no-]verbose]]\n"
-	   "         [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
+	N_("git help [-a|--all] [--[no-]verbose]]"),
+	N_("git help [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
 	N_("git help [-g|--guides]"),
 	N_("git help [-c|--config]"),
 	NULL
@@ -581,6 +581,13 @@ static void no_extra_argc(int argc)
 			      builtin_help_usage, builtin_help_options);
 }
 
+static void no_format(void)
+{
+	if (help_format != HELP_FORMAT_NONE)
+		usage_msg_opt(_("[-a|--all] cannot be combined with [[-i|--info] [-m|--man] [-w|--web]]"),
+			      builtin_help_usage, builtin_help_options);
+}
+
 int cmd_help(int argc, const char **argv, const char *prefix)
 {
 	int nongit;
@@ -593,6 +600,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	switch (cmd_mode) {
 	case HELP_ACTION_ALL:
+		no_format();
 		if (verbose) {
 			setup_pager();
 			list_all_cmds_help();
@@ -605,19 +613,27 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 		break;
 	case HELP_ACTION_GUIDES:
 		no_extra_argc(argc);
+		no_format();
+
 		list_guides_help();
 		printf("%s\n", _(git_more_info_string));
 		return 0;
 	case HELP_ACTION_CONFIG_FOR_COMPLETION:
 		no_extra_argc(argc);
+		no_format();
+
 		list_config_help(SHOW_CONFIG_VARS);
 		return 0;
 	case HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION:
 		no_extra_argc(argc);
+		no_format();
+
 		list_config_help(SHOW_CONFIG_SECTIONS);
 		return 0;
 	case HELP_ACTION_CONFIG:
 		no_extra_argc(argc);
+		no_format();
+
 		setup_pager();
 		list_config_help(SHOW_CONFIG_HUMAN);
 		printf("\n%s\n", _("'git help config' for more information"));
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 6ac293c19ed..a8c603abd44 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -46,6 +46,16 @@ test_expect_success 'invalid usage' '
 	test_expect_code 129 git help --config-sections-for-completion add
 '
 
+for opt in '-a' '-g' '-c' '--config-for-completion' '--config-sections-for-completion'
+do
+	test_expect_success "invalid usage of '$opt' with [-i|-m|-w]" '
+		git help $opt &&
+		test_expect_code 129 git help $opt -i &&
+		test_expect_code 129 git help $opt -m &&
+		test_expect_code 129 git help $opt -w
+	'
+done
+
 test_expect_success "works for commands and guides by default" '
 	configure_help &&
 	git help status &&
-- 
2.34.1.1257.g2af47340c7b
Eric Sunshine· Dec 28, 2021, 16:18 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 5/7] help: error if [-a|-g|-c] and [-i|-m|-w] are combined

On Tue, Dec 28, 2021 at 10:36 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 19 quoted lines
> Add more sanity checking to "git help" usage by erroring out if these
> man viewer options are combined with incompatible command-modes that
> will never use these documentation viewers.
>
> This continues the work started in d35d03cf93e (help: simplify by
> moving to OPT_CMDMODE(), 2021-09-22) of adding more sanity checking to
> "git help". Doing this allows us to clarify the "SYNOPSIS" in the
> documentation, and the "git help -h" output.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/builtin/help.c b/builtin/help.c
> @@ -581,6 +581,13 @@ static void no_extra_argc(int argc)
> +static void no_format(void)
> +{
> +       if (help_format != HELP_FORMAT_NONE)
> +               usage_msg_opt(_("[-a|--all] cannot be combined with [[-i|--info] [-m|--man] [-w|--web]]"),
> +                             builtin_help_usage, builtin_help_options);
> +}
Nit: The square brackets in the message may be unnecessarily
confusing. (Indeed, what exactly do they mean in this context?) Also,
the short options may not add much value: the user who typed `-w`
knows presumably that it is shorthand for `--web`. So, one
simplification would be:
    '--all' cannot be combined with '--info', '--man', or '--web'
Ævar Arnfjörð Bjarmason· Dec 29, 2021, 00:04 UTC · re: Eric Sunshine · lore

Re: [PATCH 5/7] help: error if [-a|-g|-c] and [-i|-m|-w] are combined

On Tue, Dec 28 2021, Eric Sunshine wrote:
Show 29 quoted lines
> On Tue, Dec 28, 2021 at 10:36 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Add more sanity checking to "git help" usage by erroring out if these
>> man viewer options are combined with incompatible command-modes that
>> will never use these documentation viewers.
>>
>> This continues the work started in d35d03cf93e (help: simplify by
>> moving to OPT_CMDMODE(), 2021-09-22) of adding more sanity checking to
>> "git help". Doing this allows us to clarify the "SYNOPSIS" in the
>> documentation, and the "git help -h" output.
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> diff --git a/builtin/help.c b/builtin/help.c
>> @@ -581,6 +581,13 @@ static void no_extra_argc(int argc)
>> +static void no_format(void)
>> +{
>> +       if (help_format != HELP_FORMAT_NONE)
>> +               usage_msg_opt(_("[-a|--all] cannot be combined with [[-i|--info] [-m|--man] [-w|--web]]"),
>> +                             builtin_help_usage, builtin_help_options);
>> +}
>
> Nit: The square brackets in the message may be unnecessarily
> confusing. (Indeed, what exactly do they mean in this context?) Also,
> the short options may not add much value: the user who typed `-w`
> knows presumably that it is shorthand for `--web`. So, one
> simplification would be:
>
>     '--all' cannot be combined with '--info', '--man', or '--web'
Thanks, will change it.
Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 6/7] help: add --no-[external-commands|aliases] for use with --all

Add the ability to only emit git's own usage information under --all. This also allows us to extend the "test_section_spacing" tests added in a preceding commit to test "git help --all" output.

Previously we could not do that, as the tests might find a git-* command in the "$PATH", which would make the output differ from one setup to another.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-help.txt | 10 +++++++++-
 builtin/help.c             | 16 ++++++++++++++--
 help.c                     |  8 +++++---
 help.h                     |  2 +-
 t/t0012-help.sh            | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+), 7 deletions(-)
Show changes to 5 files +66 −7

Documentation/git-help.txt, builtin/help.c, help.c, help.h, t/t0012-help.sh

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 61d52d30f6c..9307445c333 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
 SYNOPSIS
 --------
 [verse]
-'git help' [-a|--all] [--[no-]verbose]
+'git help' [-a|--all] [--[no-]verbose] [--[no-](external-commands|aliases)]
 'git help' [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
 'git help' [-g|--guides]
 'git help' [-c|--config]
@@ -49,6 +49,14 @@ OPTIONS
 	Prints all the available commands on the standard output. This
 	option overrides any given command or guide name.
 
+--no-external-commands::
+	When used with `--all`, exclude the listing of external "git-*"
+	commands found in the `$PATH`.
+
+--no-aliases::
+	When used with `--all`, exclude the listing of configured
+	aliases.
+
 --verbose::
 	When used with `--all` print description for all recognized
 	commands. This is the default.
diff --git a/builtin/help.c b/builtin/help.c
index 125f50f1bb0..1131b437389 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -51,9 +51,14 @@ static const char *html_path;
 static int verbose = 1;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static int exclude_guides;
+static int show_external_commands = -1;
+static int show_aliases = -1;
 static struct option builtin_help_options[] = {
 	OPT_CMDMODE('a', "all", &cmd_mode, N_("print all available commands"),
 		    HELP_ACTION_ALL),
+	OPT_BOOL(0, "external-commands", &show_external_commands,
+		 N_("show external commands in --all?")),
+	OPT_BOOL(0, "aliases", &show_aliases, N_("show aliases in --all?")),
 	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
 	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
 	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
@@ -75,7 +80,7 @@ static struct option builtin_help_options[] = {
 };
 
 static const char * const builtin_help_usage[] = {
-	N_("git help [-a|--all] [--[no-]verbose]]"),
+	N_("git help [-a|--all] [--[no-]verbose]] [--[no-](external-commands|aliases)]"),
 	N_("git help [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
 	N_("git help [-g|--guides]"),
 	N_("git help [-c|--config]"),
@@ -598,12 +603,19 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 			builtin_help_usage, 0);
 	parsed_help_format = help_format;
 
+	if (cmd_mode != HELP_ACTION_ALL &&
+	    (show_external_commands >= 0 ||
+	     show_aliases >= 0))
+		usage_msg_opt(_("the '--no-[external-commands|aliases]' options can only be used with '--all'"),
+			      builtin_help_usage, builtin_help_options);
+
 	switch (cmd_mode) {
 	case HELP_ACTION_ALL:
 		no_format();
 		if (verbose) {
 			setup_pager();
-			list_all_cmds_help();
+			list_all_cmds_help(show_external_commands,
+					   show_aliases);
 			return 0;
 		}
 		printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
diff --git a/help.c b/help.c
index ef1aa26efa6..2296f06ad9f 100644
--- a/help.c
+++ b/help.c
@@ -476,15 +476,17 @@ static void list_all_cmds_help_aliases(int longest)
 	string_list_clear(&alias_list, 1);
 }
 
-void list_all_cmds_help(void)
+void list_all_cmds_help(int show_external_commands, int show_aliases)
 {
 	int longest;
 
 	puts(_("See 'git help <command>' to read about a specific subcommand"));
 	print_cmd_by_category(main_categories, &longest);
 
-	list_all_cmds_help_external_commands();
-	list_all_cmds_help_aliases(longest);
+	if (show_external_commands)
+		list_all_cmds_help_external_commands();
+	if (show_aliases)
+		list_all_cmds_help_aliases(longest);
 }
 
 int is_in_cmdlist(struct cmdnames *c, const char *s)
diff --git a/help.h b/help.h
index 9d383f1a0b2..971a3ad855a 100644
--- a/help.h
+++ b/help.h
@@ -20,7 +20,7 @@ static inline void mput_char(char c, unsigned int num)
 }
 
 void list_common_cmds_help(void);
-void list_all_cmds_help(void);
+void list_all_cmds_help(int show_external_commands, int show_aliases);
 void list_guides_help(void);
 
 void list_all_main_cmds(struct string_list *list);
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index a8c603abd44..c41b412e34a 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -54,6 +54,19 @@ do
 		test_expect_code 129 git help $opt -m &&
 		test_expect_code 129 git help $opt -w
 	'
+
+	if test "$opt" = "-a"
+	then
+		continue
+	fi
+
+	test_expect_success "invalid usage of '$opt' with --no-external-commands" '
+		test_expect_code 129 git help $opt --no-external-commands
+	'
+
+	test_expect_success "invalid usage of '$opt' with --no-aliases" '
+		test_expect_code 129 git help $opt --no-external-commands
+	'
 done
 
 test_expect_success "works for commands and guides by default" '
@@ -184,6 +197,30 @@ do
 	'
 done
 
+test_expect_success "'git help -a' section spacing" '
+	test_section_spacing \
+		git help -a --no-external-commands --no-aliases <<-\EOF &&
+	See '\''git help <command>'\'' to read about a specific subcommand
+
+	Main Porcelain Commands
+
+	Ancillary Commands / Manipulators
+
+	Ancillary Commands / Interrogators
+
+	Interacting with Others
+
+	Low-level Commands / Manipulators
+
+	Low-level Commands / Interrogators
+
+	Low-level Commands / Syncing Repositories
+
+	Low-level Commands / Internal Helpers
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success "'git help -g' section spacing" '
 	test_section_spacing_trailer git help -g <<-\EOF &&
 
-- 
2.34.1.1257.g2af47340c7b
Eric Sunshine· Dec 28, 2021, 16:28 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH 6/7] help: add --no-[external-commands|aliases] for use with --all

On Tue, Dec 28, 2021 at 10:36 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 15 quoted lines
> Add the ability to only emit git's own usage information under
> --all. This also allows us to extend the "test_section_spacing" tests
> added in a preceding commit to test "git help --all"
> output.
>
> Previously we could not do that, as the tests might find a git-*
> command in the "$PATH", which would make the output differ from one
> setup to another.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
> @@ -8,7 +8,7 @@ git-help - Display help information about Git
> -'git help' [-a|--all] [--[no-]verbose]
> +'git help' [-a|--all] [--[no-]verbose] [--[no-](external-commands|aliases)]

Are these two new options mutually exclusive as this synopsis line seems to indicate? Glancing at the code, it looks like they can be used together. So, perhaps the synopsis should just say:

    'git help' [-a|--all] [--[no-]verbose] [--[no-]external-commands]
[--[no-]aliases]
Show 8 quoted lines
> diff --git a/builtin/help.c b/builtin/help.c
> @@ -51,9 +51,14 @@ static const char *html_path;
>         OPT_CMDMODE('a', "all", &cmd_mode, N_("print all available commands"),
>                     HELP_ACTION_ALL),
> +       OPT_BOOL(0, "external-commands", &show_external_commands,
> +                N_("show external commands in --all?")),
> +       OPT_BOOL(0, "aliases", &show_aliases, N_("show aliases in --all?")),
>         OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
Nit: I don't see question marks on any of the existing options. It's
not clear what purpose they serve here.
> @@ -75,7 +80,7 @@ static struct option builtin_help_options[] = {
>  static const char * const builtin_help_usage[] = {
> -       N_("git help [-a|--all] [--[no-]verbose]]"),
> +       N_("git help [-a|--all] [--[no-]verbose]] [--[no-](external-commands|aliases)]"),
Same observation made above about mutual exclusion.
Ævar Arnfjörð Bjarmason· Dec 28, 2021, 15:35 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH 7/7] help: don't print "\n" before single-section output

Fix a formatting regression in 1b81d8cb19d (help: use command-list.txt for the source of guides, 2018-05-20). Adjust the output of "git help --guides" and any other future single-section commands so that a newline isn't inserted before the only section being printed.

This changes the output from:
    $ git help --guides
    The Git concept guides are:
    [...]
To:
    $ git help --guides
    The Git concept guides are:
    [...]

That we started printing an extra "\n" in 1b81d8cb19d wasn't intended, but an emergent effect of moving all of the printing of "git help" output to code that was ready to handle printing N sections.

With 1b81d8cb19d we started using the "print_cmd_by_category()" function added earlier in the same series, or in cfb22a02ab5 (help: use command-list.h for common command list, 2018-05-10).

Fixing this formatting nit is easy enough. Let's have all of the output that would like to be "\n"-separated from other lines emit its own "\n". We then adjust "print_cmd_by_category()" to only print a "\n" to delimit the sections it's printing out.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 help.c          | 5 ++++-
 t/t0012-help.sh | 1 -
 2 files changed, 4 insertions(+), 2 deletions(-)
Show changes to 2 files +4 −2

help.c, t/t0012-help.sh

diff --git a/help.c b/help.c
index 2296f06ad9f..30be45cea59 100644
--- a/help.c
+++ b/help.c
@@ -124,7 +124,8 @@ static void print_cmd_by_category(const struct category_description *catdesc,
 		uint32_t mask = catdesc[i].category;
 		const char *desc = catdesc[i].desc;
 
-		putchar('\n');
+		if (i)
+			putchar('\n');
 		puts(_(desc));
 		print_command_list(cmds, mask, longest);
 	}
@@ -328,6 +329,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
 void list_common_cmds_help(void)
 {
 	puts(_("These are common Git commands used in various situations:"));
+	putchar('\n');
 	print_cmd_by_category(common_categories, NULL);
 }
 
@@ -481,6 +483,7 @@ void list_all_cmds_help(int show_external_commands, int show_aliases)
 	int longest;
 
 	puts(_("See 'git help <command>' to read about a specific subcommand"));
+	putchar('\n');
 	print_cmd_by_category(main_categories, &longest);
 
 	if (show_external_commands)
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index c41b412e34a..aae01f15229 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -223,7 +223,6 @@ test_expect_success "'git help -a' section spacing" '
 
 test_expect_success "'git help -g' section spacing" '
 	test_section_spacing_trailer git help -g <<-\EOF &&
-
 	The Git concept guides are:
 
 	EOF
-- 
2.34.1.1257.g2af47340c7b
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 0/9] help: tests, parse_options() sanity, fix "help -g" regression

A late re-roll of [1] which addresses all the comments Eric Sunshine raised, thanks for the review, and sorry the v2 took so long!

As the range-diff shows the main change is a mid-series set of changes to sanity check more "git help <opts>" incompaitibilities. The SYNOPSIS issues etc. Eric noted have also been fixed.

1. https://lore.kernel.org/git/cover-0.7-00000000000-20211228T153456Z-avarab@gmail.com/
Ævar Arnfjörð Bjarmason (9):
  help doc: add missing "]" to "[-a|--all]"
  help.c: use puts() instead of printf{,_ln}() for consistency
  help tests: test "git" and "git help [-a|-g] spacing
  help.c: split up list_all_cmds_help() function
  help: note the option name on option incompatibility
  help: correct usage & behavior of "git help --all"
  help: error if [-a|-g|-c] and [-i|-m|-w] are combined
  help: add --no-[external-commands|aliases] for use with --all
  help: don't print "\n" before single-section output
 Documentation/git-help.txt | 15 ++++--
 builtin/help.c             | 63 +++++++++++++++++++++----
 help.c                     | 37 +++++++++++----
 help.h                     |  2 +-
 t/t0012-help.sh            | 94 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 187 insertions(+), 24 deletions(-)
Range-diff against v1:
 1:  652dae26bf4 =  1:  c4b66c36c17 help doc: add missing "]" to "[-a|--all]"
 2:  f84662469a3 =  2:  124643c4b35 help.c: use puts() instead of printf{,_ln}() for consistency
 3:  3956937cf3b !  3:  3e39116f197 help tests: test "git" and "git help [-a|-g] spacing
    @@ t/t0012-help.sh: test_expect_success 'git help --config-sections-for-completion'
     +'
     +
      test_expect_success 'generate builtin list' '
    + 	mkdir -p sub &&
      	git --list-cmds=builtins >builtins
    - '
 4:  f040dd549b4 =  4:  f9c4d5e2d28 help.c: split up list_all_cmds_help() function
 5:  12ff152bd57 <  -:  ----------- help: error if [-a|-g|-c] and [-i|-m|-w] are combined
 -:  ----------- >  5:  e5c49089106 help: note the option name on option incompatibility
 -:  ----------- >  6:  868e8a6cf83 help: correct usage & behavior of "git help --all"
 -:  ----------- >  7:  992ee6580ac help: error if [-a|-g|-c] and [-i|-m|-w] are combined
 6:  a5ef9f69530 !  8:  c81c0cbbcdb help: add --no-[external-commands|aliases] for use with --all
    @@ Documentation/git-help.txt: git-help - Display help information about Git
      --------
      [verse]
     -'git help' [-a|--all] [--[no-]verbose]
    -+'git help' [-a|--all] [--[no-]verbose] [--[no-](external-commands|aliases)]
    ++'git help' [-a|--all] [--[no-]verbose] [--[no-]external-commands] [--[no-]aliases]
      'git help' [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
      'git help' [-g|--guides]
      'git help' [-c|--config]
     @@ Documentation/git-help.txt: OPTIONS
    - 	Prints all the available commands on the standard output. This
    - 	option overrides any given command or guide name.
    + --all::
    + 	Prints all the available commands on the standard output.
      
     +--no-external-commands::
     +	When used with `--all`, exclude the listing of external "git-*"
    @@ builtin/help.c: static const char *html_path;
      	OPT_CMDMODE('a', "all", &cmd_mode, N_("print all available commands"),
      		    HELP_ACTION_ALL),
     +	OPT_BOOL(0, "external-commands", &show_external_commands,
    -+		 N_("show external commands in --all?")),
    -+	OPT_BOOL(0, "aliases", &show_aliases, N_("show aliases in --all?")),
    ++		 N_("show external commands in --all")),
    ++	OPT_BOOL(0, "aliases", &show_aliases, N_("show aliases in --all")),
      	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
      	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
      	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
    @@ builtin/help.c: static struct option builtin_help_options[] = {
      
      static const char * const builtin_help_usage[] = {
     -	N_("git help [-a|--all] [--[no-]verbose]]"),
    -+	N_("git help [-a|--all] [--[no-]verbose]] [--[no-](external-commands|aliases)]"),
    ++	N_("git help [-a|--all] [--[no-]verbose]] [--[no-]external-commands] [--[no-]aliases]"),
      	N_("git help [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
      	N_("git help [-g|--guides]"),
      	N_("git help [-c|--config]"),
    @@ builtin/help.c: int cmd_help(int argc, const char **argv, const char *prefix)
     +
      	switch (cmd_mode) {
      	case HELP_ACTION_ALL:
    - 		no_format();
    + 		opt_mode_usage(argc, "--all", help_format);
      		if (verbose) {
      			setup_pager();
     -			list_all_cmds_help();
 7:  08fd12fe7b4 =  9:  08dc693dc3e help: don't print "\n" before single-section output
-- 
2.35.1.1132.ga1fe46f8690
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 1/9] help doc: add missing "]" to "[-a|--all]"

Add a missing "]" to documentation added in 63eae83f8f3 (help: add "-a --verbose" to list all commands with synopsis, 2018-05-20). This made it seem as though "--[no-]verbose" can only be provided with "--all", not "-a". The corresponding usage information in the C code ("builtin_help_usage") does not have the same problem.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-help.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Documentation/git-help.txt +1 −1
diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index 44ea63cc6d3..cf1d53e9499 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
 SYNOPSIS
 --------
 [verse]
-'git help' [-a|--all [--[no-]verbose]]
+'git help' [-a|--all] [--[no-]verbose]
 	   [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
 'git help' [-g|--guides]
 'git help' [-c|--config]
-- 
2.35.1.1132.ga1fe46f8690
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 2/9] help.c: use puts() instead of printf{,_ln}() for consistency

Change code in "help.c" that used printf_ln() without format specifiers to use puts() instead, as other existing code in the file does. Let's also change related code to use puts() instead of the equivalent of calling "printf" with a "%s\n" format.

This formatting-only change will make a subsequent functional change easier to read, as it'll be changing code that's consistently using the same functions to do the same things.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 help.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
Show changes to help.c +4 −3
diff --git a/help.c b/help.c
index 71444906ddf..77af953826e 100644
--- a/help.c
+++ b/help.c
@@ -124,7 +124,8 @@ static void print_cmd_by_category(const struct category_description *catdesc,
 		uint32_t mask = catdesc[i].category;
 		const char *desc = catdesc[i].desc;
 
-		printf("\n%s\n", _(desc));
+		putchar('\n');
+		puts(_(desc));
 		print_command_list(cmds, mask, longest);
 	}
 	free(cmds);
@@ -317,7 +318,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
 	}
 
 	if (other_cmds->cnt) {
-		printf_ln(_("git commands available from elsewhere on your $PATH"));
+		puts(_("git commands available from elsewhere on your $PATH"));
 		putchar('\n');
 		pretty_print_cmdnames(other_cmds, colopts);
 		putchar('\n');
@@ -439,7 +440,7 @@ void list_all_cmds_help(void)
 	struct cmdname_help *aliases;
 	int i, longest;
 
-	printf_ln(_("See 'git help <command>' to read about a specific subcommand"));
+	puts(_("See 'git help <command>' to read about a specific subcommand"));
 	print_cmd_by_category(main_categories, &longest);
 
 	list_all_other_cmds(&others);
-- 
2.35.1.1132.ga1fe46f8690
Junio C Hamano· Feb 23, 2022, 21:51 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 2/9] help.c: use puts() instead of printf{,_ln}() for consistency

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> -		printf("\n%s\n", _(desc));
> +		putchar('\n');
> +		puts(_(desc));

This is sort of "Meh". Even the justification that says "we'll do the same thing in future patches" is not really a justification, as it is entirely fine to add more of the "line-break plus %\n" printf() in the later steps in the same series.

Show 9 quoted lines
>  		print_command_list(cmds, mask, longest);
>  	}
>  	free(cmds);
> @@ -317,7 +318,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
>  	}
>  
>  	if (other_cmds->cnt) {
> -		printf_ln(_("git commands available from elsewhere on your $PATH"));
> +		puts(_("git commands available from elsewhere on your $PATH"));

This *IS* an improvement, as the first parameter to printf_ln() is supposed to be a format string, and should have been

	printf_ln("%s", _("git commands ..."));
> -	printf_ln(_("See 'git help <command>' to read about a specific subcommand"));
> +	puts(_("See 'git help <command>' to read about a specific subcommand"));
Ditto.
Ævar Arnfjörð Bjarmason· Feb 23, 2022, 21:57 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 2/9] help.c: use puts() instead of printf{,_ln}() for consistency

On Wed, Feb 23 2022, Junio C Hamano wrote:
Show 10 quoted lines
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> -		printf("\n%s\n", _(desc));
>> +		putchar('\n');
>> +		puts(_(desc));
>
> This is sort of "Meh".  Even the justification that says "we'll do
> the same thing in future patches" is not really a justification, as
> it is entirely fine to add more of the "line-break plus %\n" printf()
> in the later steps in the same series.

Yes, I agree that justification wouldn't make sense, "let's change this for consistency with code that doesn't exist yet and I'm about to introduce" is a non-starter as an argument.

But that's not what I mentioned in the commit message or why I changed this, as noted it's for doing the same "as other existing code in the file does".

I.e. you'll see that adjacent and related code if you run this on master:

    git grep -W '\\n' -- help.c

Now, I fully agree that's not a *strong* reason to change this, it could just be left in place.

I just thought post-series that skimming through those related functions made for marginally easier reading if they all used the same pattern to accomplish the same thing.

In any case, I don't at all feel strongly about including this change, so I can drop it if you'd like. I just wanted to clarify why I changed it.

Show 19 quoted lines
>>  		print_command_list(cmds, mask, longest);
>>  	}
>>  	free(cmds);
>> @@ -317,7 +318,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
>>  	}
>>  
>>  	if (other_cmds->cnt) {
>> -		printf_ln(_("git commands available from elsewhere on your $PATH"));
>> +		puts(_("git commands available from elsewhere on your $PATH"));
>
> This *IS* an improvement, as the first parameter to printf_ln() is
> supposed to be a format string, and should have been
>
> 	printf_ln("%s", _("git commands ..."));
>
>> -	printf_ln(_("See 'git help <command>' to read about a specific subcommand"));
>> +	puts(_("See 'git help <command>' to read about a specific subcommand"));
>
> Ditto.
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 3/9] help tests: test "git" and "git help [-a|-g] spacing

There's logic in "help.c"'s "print_cmd_by_category()" to emit "help" output with particular spacing, which doesn't make much sense when emitting only one section with "help -g".

Let's add tests for the current spacing in preparation for a subsequent whitespace formatting fix, and make sure that that fix doesn't cause regressions for the "git" and "git help" output.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0012-help.sh | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
Show changes to t/t0012-help.sh +45 −0
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index cbd725ccac8..9ac3f5d3c4b 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -138,6 +138,51 @@ test_expect_success 'git help --config-sections-for-completion' '
 	test_cmp human.munged sections
 '
 
+test_section_spacing () {
+	cat >expect &&
+	"$@" >out &&
+	grep -E "(^[^ ]|^$)" out >actual
+}
+
+test_section_spacing_trailer () {
+	test_section_spacing "$@" &&
+	test_expect_code 1 git >out &&
+	sed -n '/list available subcommands/,$p' <out >>expect
+}
+
+
+for cmd in git "git help"
+do
+	test_expect_success "'$cmd' section spacing" '
+		test_section_spacing_trailer git help <<-\EOF &&
+		usage: git [--version] [--help] [-C <path>] [-c <name>=<value>]
+
+		These are common Git commands used in various situations:
+
+		start a working area (see also: git help tutorial)
+
+		work on the current change (see also: git help everyday)
+
+		examine the history and state (see also: git help revisions)
+
+		grow, mark and tweak your common history
+
+		collaborate (see also: git help workflows)
+
+		EOF
+		test_cmp expect actual
+	'
+done
+
+test_expect_success "'git help -g' section spacing" '
+	test_section_spacing_trailer git help -g <<-\EOF &&
+
+	The Git concept guides are:
+
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'generate builtin list' '
 	mkdir -p sub &&
 	git --list-cmds=builtins >builtins
-- 
2.35.1.1132.ga1fe46f8690
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 6/9] help: correct usage & behavior of "git help --all"

Do the same for the "--all" option that I did for "--guides" in 9856ea6785c (help: correct usage & behavior of "git help --guides", 2021-09-22). I.e. we've documented it as ignoring non-option arguments, let's have it error out instead.

As with other changes made in 62f035aee3f (Merge branch 'ab/help-config-vars', 2021-10-13) this is technically a change in behavior, but in practice it's just a bug fix. We were ignoring this before, but by erroring we can simplify our documentation and synopsis, as well as avoid user confusion as they wonder what the difference between e.g. "git help --all" and "git help --all status" is (there wasn't any difference).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-help.txt | 5 ++---
 builtin/help.c             | 5 +++--
 t/t0012-help.sh            | 3 +++
 3 files changed, 8 insertions(+), 5 deletions(-)
Show changes to 3 files +8 −5

Documentation/git-help.txt, builtin/help.c, t/t0012-help.sh

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index cf1d53e9499..d07590c8ff7 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git help' [-a|--all] [--[no-]verbose]
-	   [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
+'git help' [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
 'git help' [-g|--guides]
 'git help' [-c|--config]
 
@@ -46,8 +46,7 @@ OPTIONS
 -------
 -a::
 --all::
-	Prints all the available commands on the standard output. This
-	option overrides any given command or guide name.
+	Prints all the available commands on the standard output.
 
 --verbose::
 	When used with `--all` print description for all recognized
diff --git a/builtin/help.c b/builtin/help.c
index 1c1581ef850..b682446bbf5 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -75,8 +75,8 @@ static struct option builtin_help_options[] = {
 };
 
 static const char * const builtin_help_usage[] = {
-	N_("git help [-a|--all] [--[no-]verbose]]\n"
-	   "         [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
+	N_("git help [-a|--all] [--[no-]verbose]]"),
+	N_("git help [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
 	N_("git help [-g|--guides]"),
 	N_("git help [-c|--config]"),
 	NULL
@@ -594,6 +594,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	switch (cmd_mode) {
 	case HELP_ACTION_ALL:
+		opt_mode_usage(argc, "--all");
 		if (verbose) {
 			setup_pager();
 			list_all_cmds_help();
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 9ac3f5d3c4b..c87730aa920 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -35,6 +35,9 @@ test_expect_success 'basic help commands' '
 '
 
 test_expect_success 'invalid usage' '
+	test_expect_code 129 git help -a add &&
+	test_expect_code 129 git help --all add &&
+
 	test_expect_code 129 git help -g add &&
 	test_expect_code 129 git help -a -c &&
 
-- 
2.35.1.1132.ga1fe46f8690
Junio C Hamano· Feb 23, 2022, 22:12 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 6/9] help: correct usage & behavior of "git help --all"

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 10 quoted lines
> diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
> index cf1d53e9499..d07590c8ff7 100644
> --- a/Documentation/git-help.txt
> +++ b/Documentation/git-help.txt
> @@ -9,7 +9,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git help' [-a|--all] [--[no-]verbose]
> -	   [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
> +'git help' [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]

Hmph, OK. The earlier one made it appear that when -a is given to "git help" it would also accept command or guide, which is no longer true.

>  'git help' [-g|--guides]
>  'git help' [-c|--config]

This is not new, but don't we need to fix the mark-up of [-a|--all] and [-g|--guides] and [-c|--config]? It's not like "We can give 0 or more of i/-m/-w". These are "we have to give either -a or --all to trigger this mode".

Show 20 quoted lines
> diff --git a/builtin/help.c b/builtin/help.c
> index 1c1581ef850..b682446bbf5 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -75,8 +75,8 @@ static struct option builtin_help_options[] = {
>  };
>  
>  static const char * const builtin_help_usage[] = {
> -	N_("git help [-a|--all] [--[no-]verbose]]\n"
> -	   "         [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
> +	N_("git help [-a|--all] [--[no-]verbose]]"),
> +	N_("git help [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
>  	N_("git help [-g|--guides]"),
>  	N_("git help [-c|--config]"),
>  	NULL
> @@ -594,6 +594,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  
>  	switch (cmd_mode) {
>  	case HELP_ACTION_ALL:
> +		opt_mode_usage(argc, "--all");
OK.
Show 16 quoted lines
>  		if (verbose) {
>  			setup_pager();
>  			list_all_cmds_help();
> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
> index 9ac3f5d3c4b..c87730aa920 100755
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -35,6 +35,9 @@ test_expect_success 'basic help commands' '
>  '
>  
>  test_expect_success 'invalid usage' '
> +	test_expect_code 129 git help -a add &&
> +	test_expect_code 129 git help --all add &&
> +
>  	test_expect_code 129 git help -g add &&
>  	test_expect_code 129 git help -a -c &&
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 4/9] help.c: split up list_all_cmds_help() function

Split up the listing of commands and aliases from list_all_cmds_help(). This will make a subsequent functional change smaller.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 help.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)
Show changes to help.c +20 −7
diff --git a/help.c b/help.c
index 77af953826e..004117347ee 100644
--- a/help.c
+++ b/help.c
@@ -433,15 +433,10 @@ static int get_alias(const char *var, const char *value, void *data)
 	return 0;
 }
 
-void list_all_cmds_help(void)
+static void list_all_cmds_help_external_commands(void)
 {
 	struct string_list others = STRING_LIST_INIT_DUP;
-	struct string_list alias_list = STRING_LIST_INIT_DUP;
-	struct cmdname_help *aliases;
-	int i, longest;
-
-	puts(_("See 'git help <command>' to read about a specific subcommand"));
-	print_cmd_by_category(main_categories, &longest);
+	int i;
 
 	list_all_other_cmds(&others);
 	if (others.nr)
@@ -449,6 +444,13 @@ void list_all_cmds_help(void)
 	for (i = 0; i < others.nr; i++)
 		printf("   %s\n", others.items[i].string);
 	string_list_clear(&others, 0);
+}
+
+static void list_all_cmds_help_aliases(int longest)
+{
+	struct string_list alias_list = STRING_LIST_INIT_DUP;
+	struct cmdname_help *aliases;
+	int i;
 
 	git_config(get_alias, &alias_list);
 	string_list_sort(&alias_list);
@@ -474,6 +476,17 @@ void list_all_cmds_help(void)
 	string_list_clear(&alias_list, 1);
 }
 
+void list_all_cmds_help(void)
+{
+	int longest;
+
+	puts(_("See 'git help <command>' to read about a specific subcommand"));
+	print_cmd_by_category(main_categories, &longest);
+
+	list_all_cmds_help_external_commands();
+	list_all_cmds_help_aliases(longest);
+}
+
 int is_in_cmdlist(struct cmdnames *c, const char *s)
 {
 	int i;
-- 
2.35.1.1132.ga1fe46f8690
Junio C Hamano· Feb 23, 2022, 22:03 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 4/9] help.c: split up list_all_cmds_help() function

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 14 quoted lines
> -void list_all_cmds_help(void)
> +static void list_all_cmds_help_external_commands(void)
>  {
>  	struct string_list others = STRING_LIST_INIT_DUP;
> -	struct string_list alias_list = STRING_LIST_INIT_DUP;
> -	struct cmdname_help *aliases;
> -	int i, longest;
> -
> -	puts(_("See 'git help <command>' to read about a specific subcommand"));
> -	print_cmd_by_category(main_categories, &longest);
> +	int i;
>  
>  	list_all_other_cmds(&others);
>  	if (others.nr)

Let's note that in the body of this new helper function, we still use the printf("\n%s\n", _("group header")) pattern, not putchar('\n') followed by puts().

Show 14 quoted lines
> @@ -449,6 +444,13 @@ void list_all_cmds_help(void)
>  	for (i = 0; i < others.nr; i++)
>  		printf("   %s\n", others.items[i].string);
>  	string_list_clear(&others, 0);
> +}
> +
> +static void list_all_cmds_help_aliases(int longest)
> +{
> +	struct string_list alias_list = STRING_LIST_INIT_DUP;
> +	struct cmdname_help *aliases;
> +	int i;
>  
>  	git_config(get_alias, &alias_list);
>  	string_list_sort(&alias_list);
And this helper, too.
Show 14 quoted lines
> @@ -474,6 +476,17 @@ void list_all_cmds_help(void)
>  	string_list_clear(&alias_list, 1);
>  }
>  
> +void list_all_cmds_help(void)
> +{
> +	int longest;
> +
> +	puts(_("See 'git help <command>' to read about a specific subcommand"));
> +	print_cmd_by_category(main_categories, &longest);
> +
> +	list_all_cmds_help_external_commands();
> +	list_all_cmds_help_aliases(longest);
> +}
This does make sense ;-)
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 7/9] help: error if [-a|-g|-c] and [-i|-m|-w] are combined

Add more sanity checking to "git help" usage by erroring out if these man viewer options are combined with incompatible command-modes that will never use these documentation viewers.

This continues the work started in d35d03cf93e (help: simplify by moving to OPT_CMDMODE(), 2021-09-22) of adding more sanity checking to "git help". Doing this allows us to clarify the "SYNOPSIS" in the documentation, and the "git help -h" output.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/help.c  | 41 +++++++++++++++++++++++++++++++++++------
 t/t0012-help.sh | 10 ++++++++++
 2 files changed, 45 insertions(+), 6 deletions(-)
Show changes to 2 files +45 −6

builtin/help.c, t/t0012-help.sh

diff --git a/builtin/help.c b/builtin/help.c
index b682446bbf5..1fc45adfcc7 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -574,12 +574,40 @@ static const char *check_git_cmd(const char* cmd)
 	return cmd;
 }
 
-static void opt_mode_usage(int argc, const char *opt_mode)
+static void no_help_format(const char *opt_mode, enum help_format fmt)
+{
+	const char *opt_fmt;
+
+	switch (fmt) {
+	case HELP_FORMAT_NONE:
+		return;
+	case HELP_FORMAT_MAN:
+		opt_fmt = "--man";
+		break;
+	case HELP_FORMAT_INFO:
+		opt_fmt = "--info";
+		break;
+	case HELP_FORMAT_WEB:
+		opt_fmt = "--web";
+		break;
+	default:
+		BUG("unreachable");
+	}
+
+	usage_msg_optf(_("options '%s' and '%s' cannot be used together"),
+		       builtin_help_usage, builtin_help_options, opt_mode,
+		       opt_fmt);
+}
+
+static void opt_mode_usage(int argc, const char *opt_mode,
+			   enum help_format fmt)
 {
 	if (argc)
 		usage_msg_optf(_("the '%s' option doesn't take any non-option arguments"),
 			       builtin_help_usage, builtin_help_options,
 			       opt_mode);
+
+	no_help_format(opt_mode, fmt);
 }
 
 int cmd_help(int argc, const char **argv, const char *prefix)
@@ -594,7 +622,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 
 	switch (cmd_mode) {
 	case HELP_ACTION_ALL:
-		opt_mode_usage(argc, "--all");
+		opt_mode_usage(argc, "--all", help_format);
 		if (verbose) {
 			setup_pager();
 			list_all_cmds_help();
@@ -606,20 +634,21 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 		printf("%s\n", _(git_more_info_string));
 		break;
 	case HELP_ACTION_GUIDES:
-		opt_mode_usage(argc, "--guides");
+		opt_mode_usage(argc, "--guides", help_format);
 		list_guides_help();
 		printf("%s\n", _(git_more_info_string));
 		return 0;
 	case HELP_ACTION_CONFIG_FOR_COMPLETION:
-		opt_mode_usage(argc, "--config-for-completion");
+		opt_mode_usage(argc, "--config-for-completion", help_format);
 		list_config_help(SHOW_CONFIG_VARS);
 		return 0;
 	case HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION:
-		opt_mode_usage(argc, "--config-sections-for-completion");
+		opt_mode_usage(argc, "--config-sections-for-completion",
+			       help_format);
 		list_config_help(SHOW_CONFIG_SECTIONS);
 		return 0;
 	case HELP_ACTION_CONFIG:
-		opt_mode_usage(argc, "--config");
+		opt_mode_usage(argc, "--config", help_format);
 		setup_pager();
 		list_config_help(SHOW_CONFIG_HUMAN);
 		printf("\n%s\n", _("'git help config' for more information"));
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index c87730aa920..f12783fd153 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -49,6 +49,16 @@ test_expect_success 'invalid usage' '
 	test_expect_code 129 git help --config-sections-for-completion add
 '
 
+for opt in '-a' '-g' '-c' '--config-for-completion' '--config-sections-for-completion'
+do
+	test_expect_success "invalid usage of '$opt' with [-i|-m|-w]" '
+		git help $opt &&
+		test_expect_code 129 git help $opt -i &&
+		test_expect_code 129 git help $opt -m &&
+		test_expect_code 129 git help $opt -w
+	'
+done
+
 test_expect_success "works for commands and guides by default" '
 	configure_help &&
 	git help status &&
-- 
2.35.1.1132.ga1fe46f8690
Junio C Hamano· Feb 23, 2022, 22:16 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 7/9] help: error if [-a|-g|-c] and [-i|-m|-w] are combined

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 8 quoted lines
> Add more sanity checking to "git help" usage by erroring out if these
> man viewer options are combined with incompatible command-modes that
> will never use these documentation viewers.
>
> This continues the work started in d35d03cf93e (help: simplify by
> moving to OPT_CMDMODE(), 2021-09-22) of adding more sanity checking to
> "git help". Doing this allows us to clarify the "SYNOPSIS" in the
> documentation, and the "git help -h" output.

While this is not wrong per-se, "git help --all --web" in the future that pops the same information up in your running browser would not be a bad thing to have.

That use of --web is certainly not in line with --man and --info, though (at that point we'd be saying "show in browser or show in terminal").

So I guess this is OK at least for now.
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 9/9] help: don't print "\n" before single-section output

Fix a formatting regression in 1b81d8cb19d (help: use command-list.txt for the source of guides, 2018-05-20). Adjust the output of "git help --guides" and any other future single-section commands so that a newline isn't inserted before the only section being printed.

This changes the output from:
    $ git help --guides
    The Git concept guides are:
    [...]
To:
    $ git help --guides
    The Git concept guides are:
    [...]

That we started printing an extra "\n" in 1b81d8cb19d wasn't intended, but an emergent effect of moving all of the printing of "git help" output to code that was ready to handle printing N sections.

With 1b81d8cb19d we started using the "print_cmd_by_category()" function added earlier in the same series, or in cfb22a02ab5 (help: use command-list.h for common command list, 2018-05-10).

Fixing this formatting nit is easy enough. Let's have all of the output that would like to be "\n"-separated from other lines emit its own "\n". We then adjust "print_cmd_by_category()" to only print a "\n" to delimit the sections it's printing out.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 help.c          | 5 ++++-
 t/t0012-help.sh | 1 -
 2 files changed, 4 insertions(+), 2 deletions(-)
Show changes to 2 files +4 −2

help.c, t/t0012-help.sh

diff --git a/help.c b/help.c
index 45a21e7e35c..afd3af24124 100644
--- a/help.c
+++ b/help.c
@@ -124,7 +124,8 @@ static void print_cmd_by_category(const struct category_description *catdesc,
 		uint32_t mask = catdesc[i].category;
 		const char *desc = catdesc[i].desc;
 
-		putchar('\n');
+		if (i)
+			putchar('\n');
 		puts(_(desc));
 		print_command_list(cmds, mask, longest);
 	}
@@ -328,6 +329,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
 void list_common_cmds_help(void)
 {
 	puts(_("These are common Git commands used in various situations:"));
+	putchar('\n');
 	print_cmd_by_category(common_categories, NULL);
 }
 
@@ -481,6 +483,7 @@ void list_all_cmds_help(int show_external_commands, int show_aliases)
 	int longest;
 
 	puts(_("See 'git help <command>' to read about a specific subcommand"));
+	putchar('\n');
 	print_cmd_by_category(main_categories, &longest);
 
 	if (show_external_commands)
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 64321480c68..6c3e1f7159d 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -226,7 +226,6 @@ test_expect_success "'git help -a' section spacing" '
 
 test_expect_success "'git help -g' section spacing" '
 	test_section_spacing_trailer git help -g <<-\EOF &&
-
 	The Git concept guides are:
 
 	EOF
-- 
2.35.1.1132.ga1fe46f8690
Junio C Hamano· Feb 23, 2022, 22:31 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 9/9] help: don't print "\n" before single-section output

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 26 quoted lines
> @@ -124,7 +124,8 @@ static void print_cmd_by_category(const struct category_description *catdesc,
>  		uint32_t mask = catdesc[i].category;
>  		const char *desc = catdesc[i].desc;
>  
> -		putchar('\n');
> +		if (i)
> +			putchar('\n');
>  		puts(_(desc));
>  		print_command_list(cmds, mask, longest);
>  	}
> @@ -328,6 +329,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
>  void list_common_cmds_help(void)
>  {
>  	puts(_("These are common Git commands used in various situations:"));
> +	putchar('\n');
>  	print_cmd_by_category(common_categories, NULL);
>  }
>  
> @@ -481,6 +483,7 @@ void list_all_cmds_help(int show_external_commands, int show_aliases)
>  	int longest;
>  
>  	puts(_("See 'git help <command>' to read about a specific subcommand"));
> +	putchar('\n');
>  	print_cmd_by_category(main_categories, &longest);
>  
>  	if (show_external_commands)

This patch is a good example to demonstrate that some changes cannot be reviewed without showing in the whole file what didn't get changed. Among three callers of print_cmd_by_category(), two of them we see here are *not* the first to speak, and do add a blank line before the call. The other caller that does not appear in the patch is what is being fixed, list_guides_help(), which has nobody to speak before it.

The correct pattern we want to follow in these messages is that before you say something, you add a blank line if somebody else has spoken before you. It might make sense to tell these print_cmd_by* helpers if they are the first to speak by passing the pointer to a "state" variable to indicate what has been emitted so far, instead of making the callers responsible to physically add blank lines, as the callers would stop knowing if nobody else has spoken before them if we further refactor them and introduce new direct callers.

In any case, the posted patch itself looks OK. After following these 9 steps, I still do not see enough justification for the earlier "use puts()" step, except for correcting the misused printf_ln().

Thanks, will queue the whole thing.
Show 12 quoted lines
> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
> index 64321480c68..6c3e1f7159d 100755
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -226,7 +226,6 @@ test_expect_success "'git help -a' section spacing" '
>  
>  test_expect_success "'git help -g' section spacing" '
>  	test_section_spacing_trailer git help -g <<-\EOF &&
> -
>  	The Git concept guides are:
>  
>  	EOF
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 8/9] help: add --no-[external-commands|aliases] for use with --all

Add the ability to only emit git's own usage information under --all. This also allows us to extend the "test_section_spacing" tests added in a preceding commit to test "git help --all" output.

Previously we could not do that, as the tests might find a git-* command in the "$PATH", which would make the output differ from one setup to another.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-help.txt | 10 +++++++++-
 builtin/help.c             | 16 ++++++++++++++--
 help.c                     |  8 +++++---
 help.h                     |  2 +-
 t/t0012-help.sh            | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 66 insertions(+), 7 deletions(-)
Show changes to 5 files +66 −7

Documentation/git-help.txt, builtin/help.c, help.c, help.h, t/t0012-help.sh

diff --git a/Documentation/git-help.txt b/Documentation/git-help.txt
index d07590c8ff7..239c68db457 100644
--- a/Documentation/git-help.txt
+++ b/Documentation/git-help.txt
@@ -8,7 +8,7 @@ git-help - Display help information about Git
 SYNOPSIS
 --------
 [verse]
-'git help' [-a|--all] [--[no-]verbose]
+'git help' [-a|--all] [--[no-]verbose] [--[no-]external-commands] [--[no-]aliases]
 'git help' [[-i|--info] [-m|--man] [-w|--web]] [<command>|<guide>]
 'git help' [-g|--guides]
 'git help' [-c|--config]
@@ -48,6 +48,14 @@ OPTIONS
 --all::
 	Prints all the available commands on the standard output.
 
+--no-external-commands::
+	When used with `--all`, exclude the listing of external "git-*"
+	commands found in the `$PATH`.
+
+--no-aliases::
+	When used with `--all`, exclude the listing of configured
+	aliases.
+
 --verbose::
 	When used with `--all` print description for all recognized
 	commands. This is the default.
diff --git a/builtin/help.c b/builtin/help.c
index 1fc45adfcc7..01eda326c31 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -51,9 +51,14 @@ static const char *html_path;
 static int verbose = 1;
 static enum help_format help_format = HELP_FORMAT_NONE;
 static int exclude_guides;
+static int show_external_commands = -1;
+static int show_aliases = -1;
 static struct option builtin_help_options[] = {
 	OPT_CMDMODE('a', "all", &cmd_mode, N_("print all available commands"),
 		    HELP_ACTION_ALL),
+	OPT_BOOL(0, "external-commands", &show_external_commands,
+		 N_("show external commands in --all")),
+	OPT_BOOL(0, "aliases", &show_aliases, N_("show aliases in --all")),
 	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
 	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
 	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
@@ -75,7 +80,7 @@ static struct option builtin_help_options[] = {
 };
 
 static const char * const builtin_help_usage[] = {
-	N_("git help [-a|--all] [--[no-]verbose]]"),
+	N_("git help [-a|--all] [--[no-]verbose]] [--[no-]external-commands] [--[no-]aliases]"),
 	N_("git help [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
 	N_("git help [-g|--guides]"),
 	N_("git help [-c|--config]"),
@@ -620,12 +625,19 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 			builtin_help_usage, 0);
 	parsed_help_format = help_format;
 
+	if (cmd_mode != HELP_ACTION_ALL &&
+	    (show_external_commands >= 0 ||
+	     show_aliases >= 0))
+		usage_msg_opt(_("the '--no-[external-commands|aliases]' options can only be used with '--all'"),
+			      builtin_help_usage, builtin_help_options);
+
 	switch (cmd_mode) {
 	case HELP_ACTION_ALL:
 		opt_mode_usage(argc, "--all", help_format);
 		if (verbose) {
 			setup_pager();
-			list_all_cmds_help();
+			list_all_cmds_help(show_external_commands,
+					   show_aliases);
 			return 0;
 		}
 		printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
diff --git a/help.c b/help.c
index 004117347ee..45a21e7e35c 100644
--- a/help.c
+++ b/help.c
@@ -476,15 +476,17 @@ static void list_all_cmds_help_aliases(int longest)
 	string_list_clear(&alias_list, 1);
 }
 
-void list_all_cmds_help(void)
+void list_all_cmds_help(int show_external_commands, int show_aliases)
 {
 	int longest;
 
 	puts(_("See 'git help <command>' to read about a specific subcommand"));
 	print_cmd_by_category(main_categories, &longest);
 
-	list_all_cmds_help_external_commands();
-	list_all_cmds_help_aliases(longest);
+	if (show_external_commands)
+		list_all_cmds_help_external_commands();
+	if (show_aliases)
+		list_all_cmds_help_aliases(longest);
 }
 
 int is_in_cmdlist(struct cmdnames *c, const char *s)
diff --git a/help.h b/help.h
index 9d383f1a0b2..971a3ad855a 100644
--- a/help.h
+++ b/help.h
@@ -20,7 +20,7 @@ static inline void mput_char(char c, unsigned int num)
 }
 
 void list_common_cmds_help(void);
-void list_all_cmds_help(void);
+void list_all_cmds_help(int show_external_commands, int show_aliases);
 void list_guides_help(void);
 
 void list_all_main_cmds(struct string_list *list);
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index f12783fd153..64321480c68 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -57,6 +57,19 @@ do
 		test_expect_code 129 git help $opt -m &&
 		test_expect_code 129 git help $opt -w
 	'
+
+	if test "$opt" = "-a"
+	then
+		continue
+	fi
+
+	test_expect_success "invalid usage of '$opt' with --no-external-commands" '
+		test_expect_code 129 git help $opt --no-external-commands
+	'
+
+	test_expect_success "invalid usage of '$opt' with --no-aliases" '
+		test_expect_code 129 git help $opt --no-external-commands
+	'
 done
 
 test_expect_success "works for commands and guides by default" '
@@ -187,6 +200,30 @@ do
 	'
 done
 
+test_expect_success "'git help -a' section spacing" '
+	test_section_spacing \
+		git help -a --no-external-commands --no-aliases <<-\EOF &&
+	See '\''git help <command>'\'' to read about a specific subcommand
+
+	Main Porcelain Commands
+
+	Ancillary Commands / Manipulators
+
+	Ancillary Commands / Interrogators
+
+	Interacting with Others
+
+	Low-level Commands / Manipulators
+
+	Low-level Commands / Interrogators
+
+	Low-level Commands / Syncing Repositories
+
+	Low-level Commands / Internal Helpers
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success "'git help -g' section spacing" '
 	test_section_spacing_trailer git help -g <<-\EOF &&
 
-- 
2.35.1.1132.ga1fe46f8690
Junio C Hamano· Feb 23, 2022, 22:19 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 8/9] help: add --no-[external-commands|aliases] for use with --all

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
> Add the ability to only emit git's own usage information under
> --all. This also allows us to extend the "test_section_spacing" tests
> added in a preceding commit to test "git help --all"
> output.
Makes sense.
Show 29 quoted lines
> +static int show_external_commands = -1;
> +static int show_aliases = -1;
>  static struct option builtin_help_options[] = {
>  	OPT_CMDMODE('a', "all", &cmd_mode, N_("print all available commands"),
>  		    HELP_ACTION_ALL),
> +	OPT_BOOL(0, "external-commands", &show_external_commands,
> +		 N_("show external commands in --all")),
> +	OPT_BOOL(0, "aliases", &show_aliases, N_("show aliases in --all")),
>  	OPT_HIDDEN_BOOL(0, "exclude-guides", &exclude_guides, N_("exclude guides")),
>  	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
>  	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
> @@ -75,7 +80,7 @@ static struct option builtin_help_options[] = {
>  };
>  
>  static const char * const builtin_help_usage[] = {
> -	N_("git help [-a|--all] [--[no-]verbose]]"),
> +	N_("git help [-a|--all] [--[no-]verbose]] [--[no-]external-commands] [--[no-]aliases]"),
>  	N_("git help [[-i|--info] [-m|--man] [-w|--web]] [<command>]"),
>  	N_("git help [-g|--guides]"),
>  	N_("git help [-c|--config]"),
> @@ -620,12 +625,19 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  			builtin_help_usage, 0);
>  	parsed_help_format = help_format;
>  
> +	if (cmd_mode != HELP_ACTION_ALL &&
> +	    (show_external_commands >= 0 ||
> +	     show_aliases >= 0))
> +		usage_msg_opt(_("the '--no-[external-commands|aliases]' options can only be used with '--all'"),
> +			      builtin_help_usage, builtin_help_options);
Nice attention to a small detail.
Show 34 quoted lines
>  	switch (cmd_mode) {
>  	case HELP_ACTION_ALL:
>  		opt_mode_usage(argc, "--all", help_format);
>  		if (verbose) {
>  			setup_pager();
> -			list_all_cmds_help();
> +			list_all_cmds_help(show_external_commands,
> +					   show_aliases);
>  			return 0;
>  		}
>  		printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
> diff --git a/help.c b/help.c
> index 004117347ee..45a21e7e35c 100644
> --- a/help.c
> +++ b/help.c
> @@ -476,15 +476,17 @@ static void list_all_cmds_help_aliases(int longest)
>  	string_list_clear(&alias_list, 1);
>  }
>  
> -void list_all_cmds_help(void)
> +void list_all_cmds_help(int show_external_commands, int show_aliases)
>  {
>  	int longest;
>  
>  	puts(_("See 'git help <command>' to read about a specific subcommand"));
>  	print_cmd_by_category(main_categories, &longest);
>  
> -	list_all_cmds_help_external_commands();
> -	list_all_cmds_help_aliases(longest);
> +	if (show_external_commands)
> +		list_all_cmds_help_external_commands();
> +	if (show_aliases)
> +		list_all_cmds_help_aliases(longest);
>  }
OK.  Quite straight-forward after the earlier split.
Ævar Arnfjörð Bjarmason· Feb 21, 2022, 19:38 UTC · re: Ævar Arnfjörð Bjarmason · lore

[PATCH v2 5/9] help: note the option name on option incompatibility

Change the errors added in d35d03cf93e (help: simplify by moving to OPT_CMDMODE(), 2021-09-22) to quote the offending option at the user when invoked as e.g.:

    git help --guides garbage
Now instead of:
    fatal: this option doesn't take any other arguments
We'll emit:
    fatal: the '--guides' option doesn't take any non-option arguments

Let's also rename the function, as it will be extended to do other checks that aren't "no extra argc" in a subsequent commit.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/help.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
Show changes to builtin/help.c +8 −7
diff --git a/builtin/help.c b/builtin/help.c
index d387131dd83..1c1581ef850 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -574,11 +574,12 @@ static const char *check_git_cmd(const char* cmd)
 	return cmd;
 }
 
-static void no_extra_argc(int argc)
+static void opt_mode_usage(int argc, const char *opt_mode)
 {
 	if (argc)
-		usage_msg_opt(_("this option doesn't take any other arguments"),
-			      builtin_help_usage, builtin_help_options);
+		usage_msg_optf(_("the '%s' option doesn't take any non-option arguments"),
+			       builtin_help_usage, builtin_help_options,
+			       opt_mode);
 }
 
 int cmd_help(int argc, const char **argv, const char *prefix)
@@ -604,20 +605,20 @@ int cmd_help(int argc, const char **argv, const char *prefix)
 		printf("%s\n", _(git_more_info_string));
 		break;
 	case HELP_ACTION_GUIDES:
-		no_extra_argc(argc);
+		opt_mode_usage(argc, "--guides");
 		list_guides_help();
 		printf("%s\n", _(git_more_info_string));
 		return 0;
 	case HELP_ACTION_CONFIG_FOR_COMPLETION:
-		no_extra_argc(argc);
+		opt_mode_usage(argc, "--config-for-completion");
 		list_config_help(SHOW_CONFIG_VARS);
 		return 0;
 	case HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION:
-		no_extra_argc(argc);
+		opt_mode_usage(argc, "--config-sections-for-completion");
 		list_config_help(SHOW_CONFIG_SECTIONS);
 		return 0;
 	case HELP_ACTION_CONFIG:
-		no_extra_argc(argc);
+		opt_mode_usage(argc, "--config");
 		setup_pager();
 		list_config_help(SHOW_CONFIG_HUMAN);
 		printf("\n%s\n", _("'git help config' for more information"));
-- 
2.35.1.1132.ga1fe46f8690
Junio C Hamano· Feb 23, 2022, 22:04 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH v2 5/9] help: note the option name on option incompatibility

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
Show 13 quoted lines
> Change the errors added in d35d03cf93e (help: simplify by moving to
> OPT_CMDMODE(), 2021-09-22) to quote the offending option at the user
> when invoked as e.g.:
>
>     git help --guides garbage
>
> Now instead of:
>
>     fatal: this option doesn't take any other arguments
>
> We'll emit:
>
>     fatal: the '--guides' option doesn't take any non-option arguments
Very good.
Show 53 quoted lines
> Let's also rename the function, as it will be extended to do other
> checks that aren't "no extra argc" in a subsequent commit.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/help.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index d387131dd83..1c1581ef850 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -574,11 +574,12 @@ static const char *check_git_cmd(const char* cmd)
>  	return cmd;
>  }
>  
> -static void no_extra_argc(int argc)
> +static void opt_mode_usage(int argc, const char *opt_mode)
>  {
>  	if (argc)
> -		usage_msg_opt(_("this option doesn't take any other arguments"),
> -			      builtin_help_usage, builtin_help_options);
> +		usage_msg_optf(_("the '%s' option doesn't take any non-option arguments"),
> +			       builtin_help_usage, builtin_help_options,
> +			       opt_mode);
>  }
>  
>  int cmd_help(int argc, const char **argv, const char *prefix)
> @@ -604,20 +605,20 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  		printf("%s\n", _(git_more_info_string));
>  		break;
>  	case HELP_ACTION_GUIDES:
> -		no_extra_argc(argc);
> +		opt_mode_usage(argc, "--guides");
>  		list_guides_help();
>  		printf("%s\n", _(git_more_info_string));
>  		return 0;
>  	case HELP_ACTION_CONFIG_FOR_COMPLETION:
> -		no_extra_argc(argc);
> +		opt_mode_usage(argc, "--config-for-completion");
>  		list_config_help(SHOW_CONFIG_VARS);
>  		return 0;
>  	case HELP_ACTION_CONFIG_SECTIONS_FOR_COMPLETION:
> -		no_extra_argc(argc);
> +		opt_mode_usage(argc, "--config-sections-for-completion");
>  		list_config_help(SHOW_CONFIG_SECTIONS);
>  		return 0;
>  	case HELP_ACTION_CONFIG:
> -		no_extra_argc(argc);
> +		opt_mode_usage(argc, "--config");
>  		setup_pager();
>  		list_config_help(SHOW_CONFIG_HUMAN);
>  		printf("\n%s\n", _("'git help config' for more information"));

← back to recent threads