git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/4] builtin: also setup gently for --help-all

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 28, 2025, 15:33 UTC
Message-ID
<xmqqcy9kxrbw.fsf@gitster.g>
In-Reply-To
<20250726165320.4039-4-ben.knoble+github@gmail.com>
"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
Show 10 quoted lines
>     I originally considered leaving out the changes to
>     show_usage_with_options_if_asked and relying on the parse-options API to
>     do the right thing. Unfortunately, most commands can't make it all the
>     way to parse-options when setting up gently, and trying to parse options
>     without a repo creates myriad dependency problems (like: we might read
>     config after parsing CLI options, so we have to make sure the parsed
>     options overrride config).
>     
>     Some usage.c callers, like check-ref-format, probably deserve to be
>     ported to parse-options at this point.

It is unclear what you mean by all of the above, but hopefully it would become clear as we read the code changes.

>     I opted not to do anything too invasive with merge-recursive (like a
>     prepatory "migrate to newer usage APIs") since I think it's going the
>     way of the dodo?
I think this is fine.
Show 14 quoted lines
> diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
> index 03b5100cfa..17aa4db37a 100644
> --- a/builtin/merge-recursive.c
> +++ b/builtin/merge-recursive.c
> @@ -38,7 +38,8 @@ int cmd_merge_recursive(int argc,
>  	if (argv[0] && ends_with(argv[0], "-subtree"))
>  		o.subtree_shift = "";
>  
> -	if (argc == 2 && !strcmp(argv[1], "-h")) {
> +	if (argc == 2 && (!strcmp(argv[1], "-h") ||
> +			  !strcmp(argv[1], "--help-all"))) {
>  		struct strbuf msg = STRBUF_INIT;
>  		strbuf_addf(&msg, builtin_merge_recursive_usage, argv[0]);
>  		show_usage_if_asked(argc, argv, msg.buf);
;-)
Show 13 quoted lines
> diff --git a/git.c b/git.c
> index 07a5fe39fb..40d3df1b76 100644
> --- a/git.c
> +++ b/git.c
> @@ -445,7 +445,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv, struct
>  	const char *prefix;
>  	int run_setup = (p->option & (RUN_SETUP | RUN_SETUP_GENTLY));
>  
> -	help = argc == 2 && !strcmp(argv[1], "-h");
> +	help = argc == 2 && (!strcmp(argv[1], "-h") || !strcmp(argv[1], "--help-all"));
>  	if (help && (run_setup & RUN_SETUP))
>  		/* demote to GENTLY to allow 'git cmd -h' outside repo */
>  		run_setup = RUN_SETUP_GENTLY;

OK. That's obvious and straight-forward. Behave as closely as the case when "-h" is given, and let the lower-layer deal with the differences between "-h" and "--help-all".

Show 21 quoted lines
> diff --git a/parse-options.c b/parse-options.c
> index c3222cc9bb..e3ed42f709 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -1464,9 +1464,14 @@ void show_usage_with_options_if_asked(int ac, const char **av,
>  				      const char * const *usagestr,
>  				      const struct option *opts)
>  {
> -	if (ac == 2 && !strcmp(av[1], "-h")) {
> -		usage_with_options_internal(NULL, usagestr, opts, style_normal, to_out);
> -		exit(129);
> +	if (ac == 2) {
> +		if (!strcmp(av[1], "-h")) {
> +			usage_with_options_internal(NULL, usagestr, opts, style_normal, to_out);
> +			exit(129);
> +		} else if (!strcmp(av[1], "--help-all")) {
> +			usage_with_options_internal(NULL, usagestr, opts, style_full, to_out);
> +			exit(129);
> +		}
>  	}
>  }
Again, that is obvious and straight-forward.
Show 28 quoted lines
> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> index e235ecccde..b26a03d8a0 100755
> --- a/t/t1517-outside-repo.sh
> +++ b/t/t1517-outside-repo.sh
> @@ -127,6 +127,10 @@
>  		test_expect_code 129 nongit git $cmd -h >usage &&
>  		test_grep "[Uu]sage: git $cmd " usage
>  	'
> +	test_$expect_outcome "'git $cmd --help-all' outside a repository" '
> +		test_expect_code 129 nongit git $cmd --help-all >usage &&
> +		test_grep "[Uu]sage: git $cmd " usage
> +	'
>  done
>  
>  test_expect_success 'prune does not crash with -h' '
> diff --git a/usage.c b/usage.c
> index 81913236a4..4c245ba0cb 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -192,7 +192,8 @@ static void show_usage_if_asked_helper(const char *err, ...)
>  
>  void show_usage_if_asked(int ac, const char **av, const char *err)
>  {
> -	if (ac == 2 && !strcmp(av[1], "-h"))
> +	if (ac == 2 && (!strcmp(av[1], "-h") ||
> +			!strcmp(av[1], "--help-all")))
>  		show_usage_if_asked_helper(err);
>  }
This looks good.

I don't understand your "I originally considered leaving out ..." at all. We are special casing a lone "-h" here already because we know this is where we should stop without exercising unnecessary code because we may not even have repo!=NULL and that is the reason why this function exists in the first place. It is obvious to me that we need the same special casing for "--help-all".

In other words, I think all the above changes are good.
Thanks.
Previous: D. Ben KnobleNext: D. Ben Knoble
Message 10 of 30 in “permit -h/--help-all in more scenarios”
  1. 0/4 permit -h/--help-all in more scenariosD. Ben Knoble, Jul 26, 2025
  2. 1/4 t1517: fixup for ua/t1517-short-help-testsD. Ben Knoble, Jul 26, 2025
  3. Usman AkinyemiJul 26, 2025
  4. Junio C HamanoJul 28, 2025
  5. 2/4 parse-options: name flags passed to usage_with_options_internalD. Ben Knoble, Jul 26, 2025
  6. Junio C HamanoJul 28, 2025
  7. Junio C HamanoJul 28, 2025
  8. D. Ben KnobleJul 30, 2025
  9. 3/4 builtin: also setup gently for --help-allD. Ben Knoble, Jul 26, 2025
  10. Junio C HamanoJul 28, 2025
  11. D. Ben KnobleJul 30, 2025
  12. 4/4 builtins: show help on "-h"/"--help-all" with more than 2 arguments leftD. Ben Knoble, Jul 26, 2025
  13. Junio C HamanoJul 27, 2025
  14. D. Ben KnobleJul 30, 2025
  15. Jeff KingAug 2, 2025
  16. D. Ben KnobleAug 2, 2025
  17. Jeff KingAug 2, 2025
  18. D. Ben KnobleAug 2, 2025
  19. 0/3 permit -h/--help-all in more scenariosD. Ben Knoble, Aug 3, 2025
  20. 0/3 permit -h/--help-all in more scenariosD. Ben Knoble, Aug 3, 2025
  21. Junio C HamanoAug 4, 2025
  22. D. Ben KnobleAug 5, 2025
  23. 1/3 t1517: fixup for ua/t1517-short-help-testsD. Ben Knoble, Aug 3, 2025
  24. Junio C HamanoAug 3, 2025
  25. D. Ben KnobleAug 3, 2025
  26. 2/3 parse-options: refactor flags for usage_with_options_internalD. Ben Knoble, Aug 3, 2025
  27. 3/3 builtin: also setup gently for --help-allD. Ben Knoble, Aug 3, 2025
  28. 1/3 t1517: fixup for ua/t1517-short-help-testsD. Ben Knoble, Aug 3, 2025
  29. 2/3 parse-options: refactor flags for usage_with_options_internalD. Ben Knoble, Aug 3, 2025
  30. 3/3 builtin: also setup gently for --help-allD. Ben Knoble, Aug 3, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.