patch, 3 partsparse-options: fix completion format when first option is skipped

14 messages between Oct 1, 2026 and Oct 2, 2026, from Patrick Steinhardt, Junio C Hamano.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Patrick SteinhardtOct 1, 2026, 10:13 UTC on lore

[PATCH 0/3] builtin/refs: introduce subcommand groups

Hi,

the git-refs(1) command has grown quite a bunch of different subcommands by now. These subcommands can easily be grouped into three categories:

  - Operations that span across the whole reference database (migrate,
    verify, optimize).

  - Operations that read references (list, exists).

  - Operations that write references (create, delete, update, rename).

This patch series thus adapts the parse-options subsystem to support grouping subcommands and then introduces the grouping for git-refs(1). This results in the following output:

  usage: git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]
     or: git refs verify [--strict] [--verbose]
     or: git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
                                  [(--sort=<key>)...] [--format=<format>]
                                  [--include-root-refs] [--points-at=<object>]
                                  [--merged[=<object>]] [--no-merged[=<object>]]
                                  [--contains[=<object>]] [--no-contains[=<object>]]
                                  [(--exclude=<pattern>)...] [--start-after=<marker>]
                                  [ --stdin | (<pattern>...)]
     or: git refs exists <ref>
     or: git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
     or: git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>
     or: git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
     or: git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
     or: git refs rename [--message=<reason>] <old-ref> <new-ref>

  Reference database
      migrate               migrate the reference database to a different format
      verify                verify the consistency of the reference database
      optimize              optimize the reference database

  Reading references
      list                  list references
      exists                check whether a reference exists

  Writing references
      create                create a new reference
      delete                delete a reference
      update                update an existing reference
      rename                rename a reference

I expect that going forward, we'll probably have more use cases where we can use these new capabilities (e.g. an upcoming git-objects(1) command, which is going to be the equivalent to git-refs(1)).

Thanks!

Patrick

---
Patrick Steinhardt (3):
      parse-options: fix completion format when first option is skipped
      parse-options: allow grouping subcommands
      builtin/refs: introduce subcommand groups

 Documentation/git-refs.adoc                    |  2 +-
 Documentation/technical/api-parse-options.adoc | 10 ++++-
 builtin/refs.c                                 | 32 ++++++++-----
 parse-options.c                                | 62 ++++++++++++++------------
 parse-options.h                                |  7 +++
 t/helper/test-parse-options.c                  |  4 +-
 t/t0040-parse-options.sh                       | 16 +++++++
 7 files changed, 92 insertions(+), 41 deletions(-)

--- base-commit: a018953688f1b10bddf91bff8747068f5f4746a4 change-id: 20261001-b4-pks-parse-options-subcommand-groups-59b3f27da06c

Patrick SteinhardtOct 1, 2026, 10:13 UTC in reply to Patrick Steinhardt on lore

The "--git-completion-helper" option can be passed to any command or subcommand that uses the parse-options interface. The output it generates is a space-separated list of subcommands or options understood by the command.

The format is slightly broken though in the case where the first option is not being printed, like for example a group or a hidden option. In that case, `show_gitcomp()` will of course skip that first entry. But when printing the next option it checks for `opts == original_opts` to verify whether we're printing the first option. The check will evaluate to false though as we have skipped it, and thus we'll print a leading space even though we have printed nothing else yet.

Fix that bug by tracking whether we have already printed anything via a local variable.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 parse-options.c               | 4 +++-
 t/helper/test-parse-options.c | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

Show changes to 2 files +4 −1

parse-options.c, t/helper/test-parse-options.c

diff --git a/parse-options.c b/parse-options.c
index 4519ead9dc..356eeff016 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -845,6 +845,7 @@ static int show_gitcomp(const struct option *opts, int show_all)
 {
 	const struct option *original_opts = opts;
 	int nr_noopts = 0;
+	bool first = true;
 
 	for (; opts->type != OPTION_END; opts++) {
 		const char *prefix = "--";
@@ -882,8 +883,9 @@ static int show_gitcomp(const struct option *opts, int show_all)
 			suffix = "=";
 		if (starts_with(opts->long_name, "no-"))
 			nr_noopts++;
-		printf("%s%s%s%s", opts == original_opts ? "" : " ",
+		printf("%s%s%s%s", first ? "" : " ",
 		       prefix, opts->long_name, suffix);
+		first = false;
 	}
 	show_negated_gitcomp(original_opts, show_all, -1);
 	show_negated_gitcomp(original_opts, show_all, nr_noopts);
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index f181f0c02d..fbafd67756 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -351,6 +351,7 @@ static int parse_subcommand__cmd(int argc, const char **argv,
 	parse_opt_subcommand_fn *fn = NULL;
 	int opt = 0;
 	struct option options[] = {
+		OPT_GROUP("Subcommands"),
 		OPT_SUBCOMMAND("subcmd-one", &fn, subcmd_one),
 		OPT_SUBCOMMAND("subcmd-two", &fn, subcmd_two),
 		OPT_INTEGER('o', "opt", &opt, "an integer option"),

-- 
2.56.0.353.g0856645cf6.dirty

Patrick SteinhardtOct 1, 2026, 10:13 UTC in reply to Patrick Steinhardt on lore

[PATCH 2/3] parse-options: allow grouping subcommands

The `OPT_GROUP()` macro can be used to create a new group. These groups can only be used to group options though, they do not have any effect when used in combination with subcommands. As our use of subcommands grows though it can be quite useful to group these, as well.

Extend the parse-options interfaces to support this use case: the new `OPT_SUBCOMMAND_H()` macro can be used to specify a subcommand that has a description attached to it, and subcommands like these are now being considered for `OPT_GROUP()`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/technical/api-parse-options.adoc | 10 ++++-
 parse-options.c                                | 58 ++++++++++++++------------
 parse-options.h                                |  7 ++++
 t/helper/test-parse-options.c                  |  3 +-
 t/t0040-parse-options.sh                       | 16 +++++++
 5 files changed, 65 insertions(+), 29 deletions(-)

Show changes to 5 files +65 −29

Documentation/technical/api-parse-options.adoc, parse-options.c, parse-options.h, t/helper/test-parse-options.c, t/t0040-parse-options.sh

diff --git a/Documentation/technical/api-parse-options.adoc b/Documentation/technical/api-parse-options.adoc
index 95b7924e84..38dff82f72 100644
--- a/Documentation/technical/api-parse-options.adoc
+++ b/Documentation/technical/api-parse-options.adoc
@@ -243,6 +243,8 @@ with `flags` set to `0`.
 	Start an option group. `description` is a short string that
 	describes the group or an empty string.
 	Start the description with an upper-case letter.
+	Groups apply to options and subcommands defined with
+	`OPT_SUBCOMMAND_H()`.
 
 `OPT_HIDDEN_GROUP(description)`::
 	Like `OPT_GROUP()`, but the group header carries
@@ -362,7 +364,13 @@ with `flags` set to `0`.
 
 `OPT_SUBCOMMAND(long, &fn_ptr, subcommand_fn)`::
 	Define a subcommand.  `subcommand_fn` is put into `fn_ptr` when
-	this subcommand is used.
+	this subcommand is used. The subcommand is not listed in the
+	usage output.
+
+`OPT_SUBCOMMAND_H(long, &fn_ptr, subcommand_fn, description)`::
+	Like `OPT_SUBCOMMAND()`, but the subcommand is listed in the usage
+	output together with its `description`. This can be used together with
+	`OPT_GROUP()` to group together subcommands.
 
 The last element of the array must be `OPT_END()`.
 
diff --git a/parse-options.c b/parse-options.c
index 356eeff016..75f14b9767 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1414,7 +1414,7 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
 		const char *cp, *np;
 		const char *positive_name = NULL;
 
-		if (opts->type == OPTION_SUBCOMMAND)
+		if (opts->type == OPTION_SUBCOMMAND && !opts->help)
 			continue;
 		if (!full && (opts->flags & PARSE_OPT_HIDDEN))
 			continue;
@@ -1432,35 +1432,39 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
 		}
 
 		pos = usage_indent(outfile);
-		if (opts->short_name) {
-			if (opts->flags & PARSE_OPT_NODASH)
-				pos += fprintf(outfile, "%c", opts->short_name);
-			else
-				pos += fprintf(outfile, "-%c", opts->short_name);
-		}
-		if (opts->long_name && opts->short_name)
-			pos += fprintf(outfile, ", ");
-		if (opts->long_name) {
-			const char *long_name = opts->long_name;
-			if ((opts->flags & PARSE_OPT_NONEG) ||
-			    skip_prefix(long_name, "no-", &positive_name))
-				pos += fprintf(outfile, "--%s", long_name);
-			else
-				pos += fprintf(outfile, "--[no-]%s", long_name);
-		}
+		if (opts->type == OPTION_SUBCOMMAND) {
+			pos += fprintf(outfile, "%s", opts->long_name);
+		} else {
+			if (opts->short_name) {
+				if (opts->flags & PARSE_OPT_NODASH)
+					pos += fprintf(outfile, "%c", opts->short_name);
+				else
+					pos += fprintf(outfile, "-%c", opts->short_name);
+			}
+			if (opts->long_name && opts->short_name)
+				pos += fprintf(outfile, ", ");
+			if (opts->long_name) {
+				const char *long_name = opts->long_name;
+				if ((opts->flags & PARSE_OPT_NONEG) ||
+				    skip_prefix(long_name, "no-", &positive_name))
+					pos += fprintf(outfile, "--%s", long_name);
+				else
+					pos += fprintf(outfile, "--[no-]%s", long_name);
+			}
 
-		if (opts->type == OPTION_NUMBER)
-			pos += utf8_fprintf(outfile, _("-NUM"));
+			if (opts->type == OPTION_NUMBER)
+				pos += utf8_fprintf(outfile, _("-NUM"));
 
-		if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
-		    !(opts->flags & PARSE_OPT_NOARG))
-			pos += usage_argh(opts, outfile);
+			if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
+			    !(opts->flags & PARSE_OPT_NOARG))
+				pos += usage_argh(opts, outfile);
 
-		if (opts->type == OPTION_ALIAS) {
-			usage_padding(outfile, pos);
-			fprintf_ln(outfile, _("alias of --%s"),
-				   (const char *)opts->value);
-			continue;
+			if (opts->type == OPTION_ALIAS) {
+				usage_padding(outfile, pos);
+				fprintf_ln(outfile, _("alias of --%s"),
+					   (const char *)opts->value);
+				continue;
+			}
 		}
 
 		for (cp = opts->help ? _(opts->help) : ""; *cp; cp = np) {
diff --git a/parse-options.h b/parse-options.h
index d7f896a933..5249404b46 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -401,6 +401,13 @@ static char *parse_options_noop_ignored_value MAYBE_UNUSED;
 	.subcommand_fn = (fn), \
 }
 #define OPT_SUBCOMMAND(l, v, fn)    OPT_SUBCOMMAND_F((l), (v), (fn), 0)
+#define OPT_SUBCOMMAND_H(l, v, fn, h) { \
+	.type = OPTION_SUBCOMMAND, \
+	.long_name = (l), \
+	.value = (v), \
+	.help = (h), \
+	.subcommand_fn = (fn), \
+}
 
 /*
  * parse_options() will filter out the processed options and leave the
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index fbafd67756..4a146bd016 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -352,8 +352,9 @@ static int parse_subcommand__cmd(int argc, const char **argv,
 	int opt = 0;
 	struct option options[] = {
 		OPT_GROUP("Subcommands"),
-		OPT_SUBCOMMAND("subcmd-one", &fn, subcmd_one),
+		OPT_SUBCOMMAND_H("subcmd-one", &fn, subcmd_one, "the first subcommand"),
 		OPT_SUBCOMMAND("subcmd-two", &fn, subcmd_two),
+		OPT_GROUP("Options"),
 		OPT_INTEGER('o', "opt", &opt, "an integer option"),
 		OPT_END()
 	};
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 449fff4d34..ec55bb1414 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -629,6 +629,22 @@ test_expect_success 'KEEP_UNKNOWN_OPT | NO_INTERNAL_HELP works' '
 	test_cmp expect actual
 '
 
+test_expect_success 'subcommand - usage lists subcommands with help text under their group' '
+	test-tool parse-subcommand cmd -h >actual &&
+	cat >expect <<-\EOF &&
+	usage: <...> cmd subcmd-one
+	   or: <...> cmd subcmd-two
+
+	Subcommands
+	    subcmd-one            the first subcommand
+
+	Options
+	    -o, --[no-]opt <n>    an integer option
+
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'subcommand - no subcommand shows error and usage' '
 	test_expect_code 129 test-tool parse-subcommand cmd 2>err &&
 	test_grep "^error: need a subcommand" err &&

-- 
2.56.0.353.g0856645cf6.dirty

Patrick SteinhardtOct 1, 2026, 10:13 UTC in reply to Patrick Steinhardt on lore

[PATCH 3/3] builtin/refs: introduce subcommand groups

The git-refs(1) command nowadays has a bunch of different subcommands, which makes it hard to figure out what's what at a glance. Now that the parse-options subsystem supports grouping subcommands though we can do better. The commands roughly fall into the following categories:

  - Operations that span across the whole reference database.

  - Operations that read references.

  - Operations that write references.

Introduce these groups accordingly, which results in the following help output:

  Reference database
      migrate               migrate the reference database to a different format
      verify                verify the consistency of the reference database
      optimize              optimize the reference database

  Reading references
      list                  list references
      exists                check whether a reference exists

  Writing references
      create                create a new reference
      delete                delete a reference
      update                update an existing reference
      rename                rename a reference

Reorder the usage strings to match the new grouping.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git-refs.adoc |  2 +-
 builtin/refs.c              | 32 ++++++++++++++++++++++----------
 2 files changed, 23 insertions(+), 11 deletions(-)

Show changes to 2 files +23 −11

Documentation/git-refs.adoc, builtin/refs.c

diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index 9dc08cbca9..da7260c416 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -11,6 +11,7 @@ SYNOPSIS
 [synopsis]
 git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]
 git refs verify [--strict] [--verbose]
+git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
 git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>]
 		   [--include-root-refs] [--points-at=<object>]
@@ -19,7 +20,6 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--exclude=<pattern>)...] [--start-after=<marker>]
 		   [ --stdin | (<pattern>...)]
 git refs exists <ref>
-git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
 git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>
 git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
 git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
diff --git a/builtin/refs.c b/builtin/refs.c
index 5cd21c25fe..f46abd6268 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -382,9 +382,9 @@ int cmd_refs(int argc,
 	const char * const refs_usage[] = {
 		REFS_MIGRATE_USAGE,
 		REFS_VERIFY_USAGE,
+		REFS_OPTIMIZE_USAGE,
 		"git refs list " COMMON_USAGE_FOR_EACH_REF,
 		REFS_EXISTS_USAGE,
-		REFS_OPTIMIZE_USAGE,
 		REFS_CREATE_USAGE,
 		REFS_DELETE_USAGE,
 		REFS_UPDATE_USAGE,
@@ -393,15 +393,27 @@ int cmd_refs(int argc,
 	};
 	parse_opt_subcommand_fn *fn = NULL;
 	struct option opts[] = {
-		OPT_SUBCOMMAND("migrate", &fn, cmd_refs_migrate),
-		OPT_SUBCOMMAND("verify", &fn, cmd_refs_verify),
-		OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
-		OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
-		OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
-		OPT_SUBCOMMAND("create", &fn, cmd_refs_create),
-		OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
-		OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
-		OPT_SUBCOMMAND("rename", &fn, cmd_refs_rename),
+		OPT_GROUP(N_("Reference database")),
+		OPT_SUBCOMMAND_H("migrate", &fn, cmd_refs_migrate,
+				 N_("migrate the reference database to a different format")),
+		OPT_SUBCOMMAND_H("verify", &fn, cmd_refs_verify,
+				 N_("verify the consistency of the reference database")),
+		OPT_SUBCOMMAND_H("optimize", &fn, cmd_refs_optimize,
+				 N_("optimize the reference database")),
+		OPT_GROUP(N_("Reading references")),
+		OPT_SUBCOMMAND_H("list", &fn, cmd_refs_list,
+				 N_("list references")),
+		OPT_SUBCOMMAND_H("exists", &fn, cmd_refs_exists,
+				 N_("check whether a reference exists")),
+		OPT_GROUP(N_("Writing references")),
+		OPT_SUBCOMMAND_H("create", &fn, cmd_refs_create,
+				 N_("create a new reference")),
+		OPT_SUBCOMMAND_H("delete", &fn, cmd_refs_delete,
+				 N_("delete a reference")),
+		OPT_SUBCOMMAND_H("update", &fn, cmd_refs_update,
+				 N_("update an existing reference")),
+		OPT_SUBCOMMAND_H("rename", &fn, cmd_refs_rename,
+				 N_("rename a reference")),
 		OPT_END(),
 	};

-- 
2.56.0.353.g0856645cf6.dirty

Junio C HamanoOct 1, 2026, 17:38 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 1/3] parse-options: fix completion format when first option is skipped

Patrick Steinhardt <ps@pks.im> writes:

Show 15 quoted lines

> The "--git-completion-helper" option can be passed to any command or
> subcommand that uses the parse-options interface. The output it
> generates is a space-separated list of subcommands or options understood
> by the command.
>
> The format is slightly broken though in the case where the first option
> is not being printed, like for example a group or a hidden option. In
> that case, `show_gitcomp()` will of course skip that first entry. But
> when printing the next option it checks for `opts == original_opts` to
> verify whether we're printing the first option. The check will evaluate
> to false though as we have skipped it, and thus we'll print a leading
> space even though we have printed nothing else yet.
>
> Fix that bug by tracking whether we have already printed anything via a
> local variable.

Very clearly articulated. I would have chosen 'shown' as the variable name to so do, but 'first' may also be OK.

Show 42 quoted lines

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  parse-options.c               | 4 +++-
>  t/helper/test-parse-options.c | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 4519ead9dc..356eeff016 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -845,6 +845,7 @@ static int show_gitcomp(const struct option *opts, int show_all)
>  {
>  	const struct option *original_opts = opts;
>  	int nr_noopts = 0;
> +	bool first = true;
>  
>  	for (; opts->type != OPTION_END; opts++) {
>  		const char *prefix = "--";
> @@ -882,8 +883,9 @@ static int show_gitcomp(const struct option *opts, int show_all)
>  			suffix = "=";
>  		if (starts_with(opts->long_name, "no-"))
>  			nr_noopts++;
> -		printf("%s%s%s%s", opts == original_opts ? "" : " ",
> +		printf("%s%s%s%s", first ? "" : " ",
>  		       prefix, opts->long_name, suffix);
> +		first = false;
>  	}
>  	show_negated_gitcomp(original_opts, show_all, -1);
>  	show_negated_gitcomp(original_opts, show_all, nr_noopts);
> diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
> index f181f0c02d..fbafd67756 100644
> --- a/t/helper/test-parse-options.c
> +++ b/t/helper/test-parse-options.c
> @@ -351,6 +351,7 @@ static int parse_subcommand__cmd(int argc, const char **argv,
>  	parse_opt_subcommand_fn *fn = NULL;
>  	int opt = 0;
>  	struct option options[] = {
> +		OPT_GROUP("Subcommands"),
>  		OPT_SUBCOMMAND("subcmd-one", &fn, subcmd_one),
>  		OPT_SUBCOMMAND("subcmd-two", &fn, subcmd_two),
>  		OPT_INTEGER('o', "opt", &opt, "an integer option"),

Junio C HamanoOct 1, 2026, 17:46 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 2/3] parse-options: allow grouping subcommands

Patrick Steinhardt <ps@pks.im> writes:

Show 20 quoted lines

> @@ -1432,35 +1432,39 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
>  		}
>  
>  		pos = usage_indent(outfile);
> -		if (opts->short_name) {
> -			if (opts->flags & PARSE_OPT_NODASH)
> -				pos += fprintf(outfile, "%c", opts->short_name);
> -			else
> -				pos += fprintf(outfile, "-%c", opts->short_name);
> -		}
> -		if (opts->long_name && opts->short_name)
> -			pos += fprintf(outfile, ", ");
> -		if (opts->long_name) {
> -			const char *long_name = opts->long_name;
> -			if ((opts->flags & PARSE_OPT_NONEG) ||
> -			    skip_prefix(long_name, "no-", &positive_name))
> -				pos += fprintf(outfile, "--%s", long_name);
> -			else
> -				pos += fprintf(outfile, "--[no-]%s", long_name);
> -		}

It may have made it easier to follow if a preliminary step pushed the above to a helper function. It would have also prevented the nesting becoming too deep as we see below.

Show 17 quoted lines

> +		if (opts->type == OPTION_SUBCOMMAND) {
> +			pos += fprintf(outfile, "%s", opts->long_name);
> +		} else {
> +			if (opts->short_name) {
> +				if (opts->flags & PARSE_OPT_NODASH)
> +					pos += fprintf(outfile, "%c", opts->short_name);
> +				else
> +					pos += fprintf(outfile, "-%c", opts->short_name);
> +			}
> +			if (opts->long_name && opts->short_name)
> +				pos += fprintf(outfile, ", ");
> +			if (opts->long_name) {
> +				const char *long_name = opts->long_name;
> +				if ((opts->flags & PARSE_OPT_NONEG) ||
> +				    skip_prefix(long_name, "no-", &positive_name))
> +					pos += fprintf(outfile, "--%s", long_name);
> +				else

Show 15 quoted lines

> diff --git a/parse-options.h b/parse-options.h
> index d7f896a933..5249404b46 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -401,6 +401,13 @@ static char *parse_options_noop_ignored_value MAYBE_UNUSED;
>  	.subcommand_fn = (fn), \
>  }
>  #define OPT_SUBCOMMAND(l, v, fn)    OPT_SUBCOMMAND_F((l), (v), (fn), 0)
> +#define OPT_SUBCOMMAND_H(l, v, fn, h) { \
> +	.type = OPTION_SUBCOMMAND, \
> +	.long_name = (l), \
> +	.value = (v), \
> +	.help = (h), \
> +	.subcommand_fn = (fn), \
> +}

As presented, _F does not allow you to give it a help, and _H does not allow you to give it a flag word. I would have preferred to see OPT_SUBCOMMAND_F to be extended to also take the help text, as we only have two existing users in *.c code, rather than adding _H variant that is incomplete and keeping _F incomplete.

Junio C HamanoOct 1, 2026, 17:46 UTC in reply to Patrick Steinhardt on lore

Re: [PATCH 3/3] builtin/refs: introduce subcommand groups

Patrick Steinhardt <ps@pks.im> writes:

Show 28 quoted lines

> The git-refs(1) command nowadays has a bunch of different subcommands,
> which makes it hard to figure out what's what at a glance. Now that the
> parse-options subsystem supports grouping subcommands though we can do
> better. The commands roughly fall into the following categories:
>
>   - Operations that span across the whole reference database.
>
>   - Operations that read references.
>
>   - Operations that write references.
>
> Introduce these groups accordingly, which results in the following help
> output:
>
>   Reference database
>       migrate               migrate the reference database to a different format
>       verify                verify the consistency of the reference database
>       optimize              optimize the reference database
>
>   Reading references
>       list                  list references
>       exists                check whether a reference exists
>
>   Writing references
>       create                create a new reference
>       delete                delete a reference
>       update                update an existing reference
>       rename                rename a reference

Nice.

Patrick SteinhardtOct 2, 2026, 07:19 UTC in reply to Junio C Hamano on lore

Re: [PATCH 1/3] parse-options: fix completion format when first option is skipped

On Thu, Oct 01, 2026 at 10:38:08AM -0700, Junio C Hamano wrote:

Show 20 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > The "--git-completion-helper" option can be passed to any command or
> > subcommand that uses the parse-options interface. The output it
> > generates is a space-separated list of subcommands or options understood
> > by the command.
> >
> > The format is slightly broken though in the case where the first option
> > is not being printed, like for example a group or a hidden option. In
> > that case, `show_gitcomp()` will of course skip that first entry. But
> > when printing the next option it checks for `opts == original_opts` to
> > verify whether we're printing the first option. The check will evaluate
> > to false though as we have skipped it, and thus we'll print a leading
> > space even though we have printed nothing else yet.
> >
> > Fix that bug by tracking whether we have already printed anything via a
> > local variable.
> 
> Very clearly articulated.  I would have chosen 'shown' as the
> variable name to so do, but 'first' may also be OK.

That's a fair point. We explicitly _don't_ care whether it's the first entry or not, as that would match the old logic that caused this bug in the first place. So `shown` is a better name indeed.

Patrick

Patrick SteinhardtOct 2, 2026, 07:19 UTC in reply to Junio C Hamano on lore

Re: [PATCH 2/3] parse-options: allow grouping subcommands

On Thu, Oct 01, 2026 at 10:46:24AM -0700, Junio C Hamano wrote:

Show 26 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > @@ -1432,35 +1432,39 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
> >  		}
> >  
> >  		pos = usage_indent(outfile);
> > -		if (opts->short_name) {
> > -			if (opts->flags & PARSE_OPT_NODASH)
> > -				pos += fprintf(outfile, "%c", opts->short_name);
> > -			else
> > -				pos += fprintf(outfile, "-%c", opts->short_name);
> > -		}
> > -		if (opts->long_name && opts->short_name)
> > -			pos += fprintf(outfile, ", ");
> > -		if (opts->long_name) {
> > -			const char *long_name = opts->long_name;
> > -			if ((opts->flags & PARSE_OPT_NONEG) ||
> > -			    skip_prefix(long_name, "no-", &positive_name))
> > -				pos += fprintf(outfile, "--%s", long_name);
> > -			else
> > -				pos += fprintf(outfile, "--[no-]%s", long_name);
> > -		}
> 
> It may have made it easier to follow if a preliminary step pushed
> the above to a helper function.  It would have also prevented the
> nesting becoming too deep as we see below.

Will do.

Show 21 quoted lines

> > diff --git a/parse-options.h b/parse-options.h
> > index d7f896a933..5249404b46 100644
> > --- a/parse-options.h
> > +++ b/parse-options.h
> > @@ -401,6 +401,13 @@ static char *parse_options_noop_ignored_value MAYBE_UNUSED;
> >  	.subcommand_fn = (fn), \
> >  }
> >  #define OPT_SUBCOMMAND(l, v, fn)    OPT_SUBCOMMAND_F((l), (v), (fn), 0)
> > +#define OPT_SUBCOMMAND_H(l, v, fn, h) { \
> > +	.type = OPTION_SUBCOMMAND, \
> > +	.long_name = (l), \
> > +	.value = (v), \
> > +	.help = (h), \
> > +	.subcommand_fn = (fn), \
> > +}
> 
> As presented, _F does not allow you to give it a help, and _H does
> not allow you to give it a flag word.  I would have preferred to see
> OPT_SUBCOMMAND_F to be extended to also take the help text, as we
> only have two existing users in *.c code, rather than adding _H
> variant that is incomplete and keeping _F incomplete.

I was a bit torn here because I honestly wasn't quite sure whether the _F suffix stands for "full" or "flag". But okay, let's not introduce a new macro then.

Patrick

Patrick SteinhardtOct 2, 2026, 08:09 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 0/4] builtin/refs: introduce subcommand groups

Hi,

the git-refs(1) command has grown quite a bunch of different subcommands by now. These subcommands can easily be grouped into three categories:

  - Operations that span across the whole reference database (migrate,
    verify, optimize).

  - Operations that read references (list, exists).

  - Operations that write references (create, delete, update, rename).

This patch series thus adapts the parse-options subsystem to support grouping subcommands and then introduces the grouping for git-refs(1). This results in the following output:

  usage: git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]
     or: git refs verify [--strict] [--verbose]
     or: git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
                                  [(--sort=<key>)...] [--format=<format>]
                                  [--include-root-refs] [--points-at=<object>]
                                  [--merged[=<object>]] [--no-merged[=<object>]]
                                  [--contains[=<object>]] [--no-contains[=<object>]]
                                  [(--exclude=<pattern>)...] [--start-after=<marker>]
                                  [ --stdin | (<pattern>...)]
     or: git refs exists <ref>
     or: git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
     or: git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>
     or: git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
     or: git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
     or: git refs rename [--message=<reason>] <old-ref> <new-ref>

  Reference database
      migrate               migrate the reference database to a different format
      verify                verify the consistency of the reference database
      optimize              optimize the reference database

  Reading references
      list                  list references
      exists                check whether a reference exists

  Writing references
      create                create a new reference
      delete                delete a reference
      update                update an existing reference
      rename                rename a reference

I expect that going forward, we'll probably have more use cases where we can use these new capabilities (e.g. an upcoming git-objects(1) command, which is going to be the equivalent to git-refs(1)).

Changes in v2:
  - Add a preliminary refactoring for `usage_with_options_internal()` so
    that we don't have to reindent a bunch of its code.
  - Drop `OPT_SUBCOMMAND_H()` and extend `OPT_SUBCOMMAND_F()` instead.
  - Rename `bool first` to `bool shown`.
  - Link to v1: https://patch.msgid.link/20261001-b4-pks-parse-options-subcommand-groups-v1-0-01eb2f4a4c32@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (4):
      parse-options: fix completion format when first option is skipped
      parse-options: extract functions to print single option
      parse-options: allow grouping subcommands
      builtin/refs: introduce subcommand groups

 Documentation/git-refs.adoc                    |   2 +-
 Documentation/technical/api-parse-options.adoc |   4 +-
 builtin/refs.c                                 |  32 +++--
 builtin/remote.c                               |   2 +-
 builtin/stash.c                                |   2 +-
 parse-options.c                                | 176 ++++++++++++++-----------
 parse-options.h                                |   5 +-
 t/helper/test-parse-options.c                  |   4 +-
 t/t0040-parse-options.sh                       |  16 +++
 9 files changed, 150 insertions(+), 93 deletions(-)

Range-diff versus v1:

1:  7ba8a5ca15 ! 1:  10b2249fa6 parse-options: fix completion format when first option is skipped
    @@ parse-options.c: static int show_gitcomp(const struct option *opts, int show_all
      {
      	const struct option *original_opts = opts;
      	int nr_noopts = 0;
    -+	bool first = true;
    ++	bool shown = false;
      
      	for (; opts->type != OPTION_END; opts++) {
      		const char *prefix = "--";
    @@ parse-options.c: static int show_gitcomp(const struct option *opts, int show_all
      		if (starts_with(opts->long_name, "no-"))
      			nr_noopts++;
     -		printf("%s%s%s%s", opts == original_opts ? "" : " ",
    -+		printf("%s%s%s%s", first ? "" : " ",
    ++		printf("%s%s%s%s", shown ? " " : "",
      		       prefix, opts->long_name, suffix);
    -+		first = false;
    ++		shown = true;
      	}
      	show_negated_gitcomp(original_opts, show_all, -1);
      	show_negated_gitcomp(original_opts, show_all, nr_noopts);
2:  8176cf838d < -:  ---------- parse-options: allow grouping subcommands
-:  ---------- > 2:  31e5f08cdb parse-options: extract functions to print single option
-:  ---------- > 3:  20da51f904 parse-options: allow grouping subcommands
3:  9599245895 ! 4:  11cfbe5cae builtin/refs: introduce subcommand groups
    @@ builtin/refs.c: int cmd_refs(int argc,
     -		OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
     -		OPT_SUBCOMMAND("rename", &fn, cmd_refs_rename),
     +		OPT_GROUP(N_("Reference database")),
    -+		OPT_SUBCOMMAND_H("migrate", &fn, cmd_refs_migrate,
    -+				 N_("migrate the reference database to a different format")),
    -+		OPT_SUBCOMMAND_H("verify", &fn, cmd_refs_verify,
    -+				 N_("verify the consistency of the reference database")),
    -+		OPT_SUBCOMMAND_H("optimize", &fn, cmd_refs_optimize,
    -+				 N_("optimize the reference database")),
    ++		OPT_SUBCOMMAND_F("migrate", &fn, cmd_refs_migrate,
    ++				 N_("migrate the reference database to a different format"), 0),
    ++		OPT_SUBCOMMAND_F("verify", &fn, cmd_refs_verify,
    ++				 N_("verify the consistency of the reference database"), 0),
    ++		OPT_SUBCOMMAND_F("optimize", &fn, cmd_refs_optimize,
    ++				 N_("optimize the reference database"), 0),
     +		OPT_GROUP(N_("Reading references")),
    -+		OPT_SUBCOMMAND_H("list", &fn, cmd_refs_list,
    -+				 N_("list references")),
    -+		OPT_SUBCOMMAND_H("exists", &fn, cmd_refs_exists,
    -+				 N_("check whether a reference exists")),
    ++		OPT_SUBCOMMAND_F("list", &fn, cmd_refs_list,
    ++				 N_("list references"), 0),
    ++		OPT_SUBCOMMAND_F("exists", &fn, cmd_refs_exists,
    ++				 N_("check whether a reference exists"), 0),
     +		OPT_GROUP(N_("Writing references")),
    -+		OPT_SUBCOMMAND_H("create", &fn, cmd_refs_create,
    -+				 N_("create a new reference")),
    -+		OPT_SUBCOMMAND_H("delete", &fn, cmd_refs_delete,
    -+				 N_("delete a reference")),
    -+		OPT_SUBCOMMAND_H("update", &fn, cmd_refs_update,
    -+				 N_("update an existing reference")),
    -+		OPT_SUBCOMMAND_H("rename", &fn, cmd_refs_rename,
    -+				 N_("rename a reference")),
    ++		OPT_SUBCOMMAND_F("create", &fn, cmd_refs_create,
    ++				 N_("create a new reference"), 0),
    ++		OPT_SUBCOMMAND_F("delete", &fn, cmd_refs_delete,
    ++				 N_("delete a reference"), 0),
    ++		OPT_SUBCOMMAND_F("update", &fn, cmd_refs_update,
    ++				 N_("update an existing reference"), 0),
    ++		OPT_SUBCOMMAND_F("rename", &fn, cmd_refs_rename,
    ++				 N_("rename a reference"), 0),
      		OPT_END(),
      	};

--- base-commit: a018953688f1b10bddf91bff8747068f5f4746a4 change-id: 20261001-b4-pks-parse-options-subcommand-groups-59b3f27da06c

Patrick SteinhardtOct 2, 2026, 08:09 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 1/4] parse-options: fix completion format when first option is skipped

Fix that bug by tracking whether we have already printed anything via a local variable.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 parse-options.c               | 4 +++-
 t/helper/test-parse-options.c | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

Show changes to 2 files +4 −1

parse-options.c, t/helper/test-parse-options.c

diff --git a/parse-options.c b/parse-options.c
index 4519ead9dc..8bb30ec116 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -845,6 +845,7 @@ static int show_gitcomp(const struct option *opts, int show_all)
 {
 	const struct option *original_opts = opts;
 	int nr_noopts = 0;
+	bool shown = false;
 
 	for (; opts->type != OPTION_END; opts++) {
 		const char *prefix = "--";
@@ -882,8 +883,9 @@ static int show_gitcomp(const struct option *opts, int show_all)
 			suffix = "=";
 		if (starts_with(opts->long_name, "no-"))
 			nr_noopts++;
-		printf("%s%s%s%s", opts == original_opts ? "" : " ",
+		printf("%s%s%s%s", shown ? " " : "",
 		       prefix, opts->long_name, suffix);
+		shown = true;
 	}
 	show_negated_gitcomp(original_opts, show_all, -1);
 	show_negated_gitcomp(original_opts, show_all, nr_noopts);
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index f181f0c02d..fbafd67756 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -351,6 +351,7 @@ static int parse_subcommand__cmd(int argc, const char **argv,
 	parse_opt_subcommand_fn *fn = NULL;
 	int opt = 0;
 	struct option options[] = {
+		OPT_GROUP("Subcommands"),
 		OPT_SUBCOMMAND("subcmd-one", &fn, subcmd_one),
 		OPT_SUBCOMMAND("subcmd-two", &fn, subcmd_two),
 		OPT_INTEGER('o', "opt", &opt, "an integer option"),

-- 
2.56.0.353.g0856645cf6.dirty

Patrick SteinhardtOct 2, 2026, 08:09 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 2/4] parse-options: extract functions to print single option

The logic to print a single option has grown somewhat long. Extract the logic into two functions to print a single option and a flag, specifically. This refactoring makes a subsequent change easier to implement.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 parse-options.c | 169 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 94 insertions(+), 75 deletions(-)

Show changes to parse-options.c +94 −75

diff --git a/parse-options.c b/parse-options.c
index 8bb30ec116..fdcb29f2a1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1323,6 +1323,97 @@ static const struct option *find_option_by_long_name(const struct option *opts,
 	return NULL;
 }
 
+static int usage_print_flag(const struct option *opt,
+			    FILE *outfile,
+			    const char **positive_name)
+{
+	int off = 0;
+
+	if (opt->short_name) {
+		if (opt->flags & PARSE_OPT_NODASH)
+			off += fprintf(outfile, "%c", opt->short_name);
+		else
+			off += fprintf(outfile, "-%c", opt->short_name);
+	}
+	if (opt->long_name && opt->short_name)
+		off += fprintf(outfile, ", ");
+	if (opt->long_name) {
+		const char *long_name = opt->long_name;
+		if ((opt->flags & PARSE_OPT_NONEG) ||
+		    skip_prefix(long_name, "no-", positive_name))
+			off += fprintf(outfile, "--%s", long_name);
+		else
+			off += fprintf(outfile, "--[no-]%s", long_name);
+	}
+
+	if (opt->type == OPTION_NUMBER)
+		off += utf8_fprintf(outfile, _("-NUM"));
+
+	if ((opt->flags & PARSE_OPT_LITERAL_ARGHELP) ||
+	    !(opt->flags & PARSE_OPT_NOARG))
+		off += usage_argh(opt, outfile);
+
+	return off;
+}
+
+static void usage_print_option(const struct option *opt,
+			       const struct option *all_opts,
+			       int full,
+			       int *need_newline,
+			       FILE *outfile)
+{
+	const char *positive_name = NULL;
+	const char *cp, *np;
+	size_t pos;
+
+	if (opt->type == OPTION_SUBCOMMAND)
+		return;
+	if (!full && (opt->flags & PARSE_OPT_HIDDEN))
+		return;
+	if (opt->type == OPTION_GROUP) {
+		fputc('\n', outfile);
+		*need_newline = 0;
+		if (*opt->help)
+			fprintf(outfile, "%s\n", _(opt->help));
+		return;
+	}
+
+	if (*need_newline) {
+		fputc('\n', outfile);
+		*need_newline = 0;
+	}
+
+	pos = usage_indent(outfile);
+	pos += usage_print_flag(opt, outfile, &positive_name);
+
+	if (opt->type == OPTION_ALIAS) {
+		usage_padding(outfile, pos);
+		fprintf_ln(outfile, _("alias of --%s"),
+			   (const char *)opt->value);
+		return;
+	}
+
+	for (cp = opt->help ? _(opt->help) : ""; *cp; cp = np) {
+		np = strchrnul(cp, '\n');
+		if (*np)
+			np++;
+		usage_padding(outfile, pos);
+		fwrite(cp, 1, np - cp, outfile);
+		pos = 0;
+	}
+	fputc('\n', outfile);
+
+	if (positive_name) {
+		if (find_option_by_long_name(all_opts, positive_name))
+			return;
+		pos = usage_indent(outfile);
+		pos += fprintf(outfile, "--%s", positive_name);
+		usage_padding(outfile, pos);
+		fprintf_ln(outfile, _("opposite of --no-%s"),
+			   positive_name);
+	}
+}
+
 static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t *ctx,
 							 const char * const *usagestr,
 							 const struct option *opts,
@@ -1408,81 +1499,9 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
 	}
 
 	need_newline = 1;
-
-	for (; opts->type != OPTION_END; opts++) {
-		size_t pos;
-		const char *cp, *np;
-		const char *positive_name = NULL;
-
-		if (opts->type == OPTION_SUBCOMMAND)
-			continue;
-		if (!full && (opts->flags & PARSE_OPT_HIDDEN))
-			continue;
-		if (opts->type == OPTION_GROUP) {
-			fputc('\n', outfile);
-			need_newline = 0;
-			if (*opts->help)
-				fprintf(outfile, "%s\n", _(opts->help));
-			continue;
-		}
-
-		if (need_newline) {
-			fputc('\n', outfile);
-			need_newline = 0;
-		}
-
-		pos = usage_indent(outfile);
-		if (opts->short_name) {
-			if (opts->flags & PARSE_OPT_NODASH)
-				pos += fprintf(outfile, "%c", opts->short_name);
-			else
-				pos += fprintf(outfile, "-%c", opts->short_name);
-		}
-		if (opts->long_name && opts->short_name)
-			pos += fprintf(outfile, ", ");
-		if (opts->long_name) {
-			const char *long_name = opts->long_name;
-			if ((opts->flags & PARSE_OPT_NONEG) ||
-			    skip_prefix(long_name, "no-", &positive_name))
-				pos += fprintf(outfile, "--%s", long_name);
-			else
-				pos += fprintf(outfile, "--[no-]%s", long_name);
-		}
-
-		if (opts->type == OPTION_NUMBER)
-			pos += utf8_fprintf(outfile, _("-NUM"));
-
-		if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
-		    !(opts->flags & PARSE_OPT_NOARG))
-			pos += usage_argh(opts, outfile);
-
-		if (opts->type == OPTION_ALIAS) {
-			usage_padding(outfile, pos);
-			fprintf_ln(outfile, _("alias of --%s"),
-				   (const char *)opts->value);
-			continue;
-		}
-
-		for (cp = opts->help ? _(opts->help) : ""; *cp; cp = np) {
-			np = strchrnul(cp, '\n');
-			if (*np)
-				np++;
-			usage_padding(outfile, pos);
-			fwrite(cp, 1, np - cp, outfile);
-			pos = 0;
-		}
-		fputc('\n', outfile);
-
-		if (positive_name) {
-			if (find_option_by_long_name(all_opts, positive_name))
-				continue;
-			pos = usage_indent(outfile);
-			pos += fprintf(outfile, "--%s", positive_name);
-			usage_padding(outfile, pos);
-			fprintf_ln(outfile, _("opposite of --no-%s"),
-				   positive_name);
-		}
-	}
+	for (; opts->type != OPTION_END; opts++)
+		usage_print_option(opts, all_opts, full,
+				   &need_newline, outfile);
 	fputc('\n', outfile);
 
 	if (!err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL)

-- 
2.56.0.353.g0856645cf6.dirty

Patrick SteinhardtOct 2, 2026, 08:09 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 3/4] parse-options: allow grouping subcommands

Extend `OPT_SUBCOMMAND_F()` to take an optional help string. If given, such subcommands will be considered as part of `OPT_GROUP()` and printed with that help string.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/technical/api-parse-options.adoc |  4 +++-
 builtin/remote.c                               |  2 +-
 builtin/stash.c                                |  2 +-
 parse-options.c                                |  7 +++++--
 parse-options.h                                |  5 +++--
 t/helper/test-parse-options.c                  |  3 ++-
 t/t0040-parse-options.sh                       | 16 ++++++++++++++++
 7 files changed, 31 insertions(+), 8 deletions(-)

Show changes to 7 files +31 −8

Documentation/technical/api-parse-options.adoc, builtin/remote.c, builtin/stash.c, parse-options.c, parse-options.h, t/helper/test-parse-options.c, t/t0040-parse-options.sh

diff --git a/Documentation/technical/api-parse-options.adoc b/Documentation/technical/api-parse-options.adoc
index 95b7924e84..6dfea34220 100644
--- a/Documentation/technical/api-parse-options.adoc
+++ b/Documentation/technical/api-parse-options.adoc
@@ -243,6 +243,7 @@ with `flags` set to `0`.
 	Start an option group. `description` is a short string that
 	describes the group or an empty string.
 	Start the description with an upper-case letter.
+	Groups apply to options and subcommands that have a help string.
 
 `OPT_HIDDEN_GROUP(description)`::
 	Like `OPT_GROUP()`, but the group header carries
@@ -362,7 +363,8 @@ with `flags` set to `0`.
 
 `OPT_SUBCOMMAND(long, &fn_ptr, subcommand_fn)`::
 	Define a subcommand.  `subcommand_fn` is put into `fn_ptr` when
-	this subcommand is used.
+	this subcommand is used. The subcommand is not listed in the
+	usage output.
 
 The last element of the array must be `OPT_END()`.
 
diff --git a/builtin/remote.c b/builtin/remote.c
index de989ea3ba..fb7e5b114f 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1940,7 +1940,7 @@ int cmd_remote(int argc,
 		OPT__VERBOSE(&verbose, N_("be verbose; must be placed before a subcommand")),
 		OPT_SUBCOMMAND("add", &fn, add),
 		OPT_SUBCOMMAND("rename", &fn, mv),
-		OPT_SUBCOMMAND_F("rm", &fn, rm, PARSE_OPT_NOCOMPLETE),
+		OPT_SUBCOMMAND_F("rm", &fn, rm, NULL, PARSE_OPT_NOCOMPLETE),
 		OPT_SUBCOMMAND("remove", &fn, rm),
 		OPT_SUBCOMMAND("set-head", &fn, set_head),
 		OPT_SUBCOMMAND("set-branches", &fn, set_branches),
diff --git a/builtin/stash.c b/builtin/stash.c
index 7a9843413b..8d606ee11d 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -2475,7 +2475,7 @@ int cmd_stash(int argc,
 		OPT_SUBCOMMAND("push", &fn, push_stash_unassumed),
 		OPT_SUBCOMMAND("export", &fn, export_stash),
 		OPT_SUBCOMMAND("import", &fn, import_stash),
-		OPT_SUBCOMMAND_F("save", &fn, save_stash, PARSE_OPT_NOCOMPLETE),
+		OPT_SUBCOMMAND_F("save", &fn, save_stash, NULL, PARSE_OPT_NOCOMPLETE),
 		OPT_END()
 	};
 	const char **args_copy;
diff --git a/parse-options.c b/parse-options.c
index fdcb29f2a1..2b59932d2c 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1366,7 +1366,7 @@ static void usage_print_option(const struct option *opt,
 	const char *cp, *np;
 	size_t pos;
 
-	if (opt->type == OPTION_SUBCOMMAND)
+	if (opt->type == OPTION_SUBCOMMAND && !opt->help)
 		return;
 	if (!full && (opt->flags & PARSE_OPT_HIDDEN))
 		return;
@@ -1384,7 +1384,10 @@ static void usage_print_option(const struct option *opt,
 	}
 
 	pos = usage_indent(outfile);
-	pos += usage_print_flag(opt, outfile, &positive_name);
+	if (opt->type == OPTION_SUBCOMMAND)
+		pos += fprintf(outfile, "%s", opt->long_name);
+	else
+		pos += usage_print_flag(opt, outfile, &positive_name);
 
 	if (opt->type == OPTION_ALIAS) {
 		usage_padding(outfile, pos);
diff --git a/parse-options.h b/parse-options.h
index d7f896a933..99c12c77cb 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -393,14 +393,15 @@ static char *parse_options_noop_ignored_value MAYBE_UNUSED;
 	.value = (char *)(source_long_name), \
 }
 
-#define OPT_SUBCOMMAND_F(l, v, fn, f) { \
+#define OPT_SUBCOMMAND_F(l, v, fn, h, f) { \
 	.type = OPTION_SUBCOMMAND, \
 	.long_name = (l), \
 	.value = (v), \
+	.help = (h), \
 	.flags = (f), \
 	.subcommand_fn = (fn), \
 }
-#define OPT_SUBCOMMAND(l, v, fn)    OPT_SUBCOMMAND_F((l), (v), (fn), 0)
+#define OPT_SUBCOMMAND(l, v, fn)    OPT_SUBCOMMAND_F((l), (v), (fn), NULL, 0)
 
 /*
  * parse_options() will filter out the processed options and leave the
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index fbafd67756..950db78673 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -352,8 +352,9 @@ static int parse_subcommand__cmd(int argc, const char **argv,
 	int opt = 0;
 	struct option options[] = {
 		OPT_GROUP("Subcommands"),
-		OPT_SUBCOMMAND("subcmd-one", &fn, subcmd_one),
+		OPT_SUBCOMMAND_F("subcmd-one", &fn, subcmd_one, "the first subcommand", 0),
 		OPT_SUBCOMMAND("subcmd-two", &fn, subcmd_two),
+		OPT_GROUP("Options"),
 		OPT_INTEGER('o', "opt", &opt, "an integer option"),
 		OPT_END()
 	};
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 449fff4d34..ec55bb1414 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -629,6 +629,22 @@ test_expect_success 'KEEP_UNKNOWN_OPT | NO_INTERNAL_HELP works' '
 	test_cmp expect actual
 '
 
+test_expect_success 'subcommand - usage lists subcommands with help text under their group' '
+	test-tool parse-subcommand cmd -h >actual &&
+	cat >expect <<-\EOF &&
+	usage: <...> cmd subcmd-one
+	   or: <...> cmd subcmd-two
+
+	Subcommands
+	    subcmd-one            the first subcommand
+
+	Options
+	    -o, --[no-]opt <n>    an integer option
+
+	EOF
+	test_cmp expect actual
+'
+
 test_expect_success 'subcommand - no subcommand shows error and usage' '
 	test_expect_code 129 test-tool parse-subcommand cmd 2>err &&
 	test_grep "^error: need a subcommand" err &&

-- 
2.56.0.353.g0856645cf6.dirty

Patrick SteinhardtOct 2, 2026, 08:09 UTC in reply to Patrick Steinhardt on lore

[PATCH v2 4/4] builtin/refs: introduce subcommand groups

  - Operations that span across the whole reference database.

  - Operations that read references.

  - Operations that write references.

Introduce these groups accordingly, which results in the following help output:

  Reference database
      migrate               migrate the reference database to a different format
      verify                verify the consistency of the reference database
      optimize              optimize the reference database

  Reading references
      list                  list references
      exists                check whether a reference exists

  Writing references
      create                create a new reference
      delete                delete a reference
      update                update an existing reference
      rename                rename a reference

Reorder the usage strings to match the new grouping.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/git-refs.adoc |  2 +-
 builtin/refs.c              | 32 ++++++++++++++++++++++----------
 2 files changed, 23 insertions(+), 11 deletions(-)

Show changes to 2 files +23 −11

Documentation/git-refs.adoc, builtin/refs.c

diff --git a/Documentation/git-refs.adoc b/Documentation/git-refs.adoc
index 9dc08cbca9..da7260c416 100644
--- a/Documentation/git-refs.adoc
+++ b/Documentation/git-refs.adoc
@@ -11,6 +11,7 @@ SYNOPSIS
 [synopsis]
 git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]
 git refs verify [--strict] [--verbose]
+git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
 git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--sort=<key>)...] [--format=<format>]
 		   [--include-root-refs] [--points-at=<object>]
@@ -19,7 +20,6 @@ git refs list [--count=<count>] [--shell|--perl|--python|--tcl]
 		   [(--exclude=<pattern>)...] [--start-after=<marker>]
 		   [ --stdin | (<pattern>...)]
 git refs exists <ref>
-git refs optimize [--all] [--no-prune] [--auto] [--include <pattern>] [--exclude <pattern>]
 git refs create [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value>
 git refs delete [--message=<reason>] [--no-deref] <ref> [<old-value>]
 git refs update [--message=<reason>] [--no-deref] [--create-reflog] <ref> <new-value> [<old-value>]
diff --git a/builtin/refs.c b/builtin/refs.c
index 5cd21c25fe..decccc4364 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -382,9 +382,9 @@ int cmd_refs(int argc,
 	const char * const refs_usage[] = {
 		REFS_MIGRATE_USAGE,
 		REFS_VERIFY_USAGE,
+		REFS_OPTIMIZE_USAGE,
 		"git refs list " COMMON_USAGE_FOR_EACH_REF,
 		REFS_EXISTS_USAGE,
-		REFS_OPTIMIZE_USAGE,
 		REFS_CREATE_USAGE,
 		REFS_DELETE_USAGE,
 		REFS_UPDATE_USAGE,
@@ -393,15 +393,27 @@ int cmd_refs(int argc,
 	};
 	parse_opt_subcommand_fn *fn = NULL;
 	struct option opts[] = {
-		OPT_SUBCOMMAND("migrate", &fn, cmd_refs_migrate),
-		OPT_SUBCOMMAND("verify", &fn, cmd_refs_verify),
-		OPT_SUBCOMMAND("list", &fn, cmd_refs_list),
-		OPT_SUBCOMMAND("exists", &fn, cmd_refs_exists),
-		OPT_SUBCOMMAND("optimize", &fn, cmd_refs_optimize),
-		OPT_SUBCOMMAND("create", &fn, cmd_refs_create),
-		OPT_SUBCOMMAND("delete", &fn, cmd_refs_delete),
-		OPT_SUBCOMMAND("update", &fn, cmd_refs_update),
-		OPT_SUBCOMMAND("rename", &fn, cmd_refs_rename),
+		OPT_GROUP(N_("Reference database")),
+		OPT_SUBCOMMAND_F("migrate", &fn, cmd_refs_migrate,
+				 N_("migrate the reference database to a different format"), 0),
+		OPT_SUBCOMMAND_F("verify", &fn, cmd_refs_verify,
+				 N_("verify the consistency of the reference database"), 0),
+		OPT_SUBCOMMAND_F("optimize", &fn, cmd_refs_optimize,
+				 N_("optimize the reference database"), 0),
+		OPT_GROUP(N_("Reading references")),
+		OPT_SUBCOMMAND_F("list", &fn, cmd_refs_list,
+				 N_("list references"), 0),
+		OPT_SUBCOMMAND_F("exists", &fn, cmd_refs_exists,
+				 N_("check whether a reference exists"), 0),
+		OPT_GROUP(N_("Writing references")),
+		OPT_SUBCOMMAND_F("create", &fn, cmd_refs_create,
+				 N_("create a new reference"), 0),
+		OPT_SUBCOMMAND_F("delete", &fn, cmd_refs_delete,
+				 N_("delete a reference"), 0),
+		OPT_SUBCOMMAND_F("update", &fn, cmd_refs_update,
+				 N_("update an existing reference"), 0),
+		OPT_SUBCOMMAND_F("rename", &fn, cmd_refs_rename,
+				 N_("rename a reference"), 0),
 		OPT_END(),
 	};

-- 
2.56.0.353.g0856645cf6.dirty