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

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

[Patrick Steinhardt]

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

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

[Patrick Steinhardt]

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(-)

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

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

[Junio C Hamano]

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.

>
> 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"),

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

[Patrick Steinhardt]

On Thu, Oct 01, 2026 at 10:38:08AM -0700, Junio C Hamano wrote:
> 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

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

[Patrick Steinhardt]

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(-)

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

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

[Junio C Hamano]

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.

> +		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

> 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.

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

[Patrick Steinhardt]

On Thu, Oct 01, 2026 at 10:46:24AM -0700, Junio C Hamano wrote:
> 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.

> > 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

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

[Patrick Steinhardt]

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(-)

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

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

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> 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.

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

[Patrick Steinhardt]

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

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

[Patrick Steinhardt]

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(-)

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

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

[Patrick Steinhardt]

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(-)

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

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

[Patrick Steinhardt]

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 `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(-)

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

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

[Patrick Steinhardt]

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(-)

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