patch, 4 partspermit -h/--help-all in more scenarios

This series depends on ua/t1517-short-help-tests with some fixes, which show up in the first patch. Merge that branch to a new topic branch, then apply this series.

This series enables --help-all outside of repository contexts, and allows -h with other arguments (without breaking existing ls-remote/grep usage).

It consists of preparatory steps (fixes for a dependency branch; refactoring to make an internal helper's arguments clearer) followed by the main commits.

D. Ben Knoble (4):
  t1517: fixup for ua/t1517-short-help-tests
  parse-options: name flags passed to usage_with_options_internal
  builtin: also setup gently for --help-all
  builtins: show help on "-h"/"--help-all" with more than 2 arguments
    left

 builtin/merge-recursive.c     |  3 ++-
 git.c                         |  2 +-
 parse-options.c               | 41 +++++++++++++++++++++++++----------
 t/t1517-outside-repo.sh       |  9 +++++---
 t/t5200-update-server-info.sh |  2 +-
 usage.c                       |  3 ++-
 6 files changed, 41 insertions(+), 19 deletions(-)

base-commit: e4ef0485fd78fcb05866ea78df35796b904e4a8e prerequisite-patch-id: ffce2dd036e61c8d36485a17321f858e454db874 prerequisite-patch-id: 52539022c824997adfc1be0bed8de6b1851d2187

- drop spurious message during test
- fix known breakages that actually work
- fix new t5200 test

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

I expect this and other fixes to get squashed into the upstream branch, but I'm including it here so it's easy to create a clean build.

 t/t1517-outside-repo.sh       | 5 ++---
 t/t5200-update-server-info.sh | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

Thanks for fixing this, I do not have to send the update version.

Thanks

Looks good to me. Thanks.

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

So, can I ignore this step from the series and expect the fixes to be already in your updated series we will see in the future?

Thanks.

When reading or editing calls to usage_with_options_internal, it is difficult to tell what trailing "0, 0", "0, 1", "1, 0" arguments mean (NB there is never a "1, 1" case).

Give the flags readable names to improve call-sites.

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---
 parse-options.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

It is a good idea to explicitly say that this step introduces no change in behaviour, and only changes the way how these 0/1 are spelled.

These are very much internal implementation detail, so I am not sure if this churn is a good thing, though.

For example, it ought to be sufficient, for the purpose of improved readability, to instead doing this

just do

		int full_usage,
		int usage_to_stderr);

here. Dropping the parameter names in the function prototype is allowed, and we encourage to do so in our codebase but _only_ when the meaning of each parameter is obvious from their type. The first 3 parameters we see above are of distinct types and except for the second one being the usage string given to the users, they should be obvious. But the last two unnamed integers are not obvious and they should have been spelled out---otherwise a developer who is adding a new callsite cannot work from the prototype alone and has to go to the implementation to figure out what to pass.

Adding two enums for this is a bit overkill, but is OK here locally.

But this is not an improvement as-is. Wrap long lines or the result is even harder to read.

This one ...

... and this one become markedly harder to read. I think the primary reason is because unlike the original, the parameter names are not biased. "If we are doing full usage, do this" is far easier to grok than "If the "style" we are told to use is the "style_full", then do this", but use of "enum" inherently is about not making the variables and parameters of that enum type unbiased.

Ditto.

Ditto.

One way to reduce this churn is to do

	int err = (to_where == to_stderr);
	int full = (help_style == style_full);

at the very beginning of the function. Then you do not have to change the body of the function harder to read at all.

Thanks.

Junio C Hamano <gitster@pobox.com> writes:

And with that lessor impact change, you could still add a smaller change to help callers (you do not need any change to the callee, which uses biased parameter names for these two), if you wanted to (though as I said this is internal implementation detail of the parse_options API, so you do not have to). For example, you could do

#define USAGE_FULL 1 #define USAGE_TO_STDERR 1

without anything else and do

			return usage_with_options_internal(ctx,	usagestr, options,
							   USAGE_FULL, 0);

which may make it easier to follow. The point is that you be more verbose only when you do a non-standard thing.

And without enum, of course you do not need any change like below.

On Mon, Jul 28, 2025 at 11:26 AM Junio C Hamano <gitster@pobox.com> wrote:

Woops; definitely meant for that to be clearer. Will touch up.

Yeah, but that relies on folks reading the prototype, no? I wanted it to be easier to read at the call sites (_without_ special tooling, preferably).

I'll snip the rest, though, due to your downthread suggestion to use #define's instead, which I think gives the result I want without extra churn in other places.

Ah, indeed: I wasn't sure where to draw the line there.

Git experts often check the help summary of a command to make sure they spell options right when suggesting advice to colleagues. Further, they might check hidden options when responding to queries about deprecated options like git-rebase(1)'s "preserve merges" option. But some commands don't support "--help-all" outside of a git directory. Running (for example)

    git rebase --help-all

outside a directory fails in "setup_git_directory", erroring with the localized form of

    fatal: not a git repository (or any of the parent directories): .git

Like 99caeed05d (Let 'git <command> -h' show usage without a git dir, 2009-11-09), we want to show the "--help-all" output even without a git dir. Make "--help-all" where we expect "-h" to mean "setup_git_directory_gently", and interpose early in the natural place ("show_usage_with_options_if_asked").

Do the same for usage callers with show_usage_if_asked.

The exception is merge-recursive, whose help block doesn't use newer APIs.

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

Notes:
    On interposition points:
    
    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.
    
    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?

 builtin/merge-recursive.c |  3 ++-
 git.c                     |  2 +-
 parse-options.c           | 11 ++++++++---
 t/t1517-outside-repo.sh   |  4 ++++
 usage.c                   |  3 ++-
 5 files changed, 17 insertions(+), 6 deletions(-)

"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

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

I think this is fine.

;-)

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

Again, that is obvious and straight-forward.

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.

On Mon, Jul 28, 2025 at 11:33 AM Junio C Hamano <gitster@pobox.com> wrote:

I'm glad it was obvious to you! I spent a lot of time trying to get commands to call parse_options early, before touching any repository variables, since that was the only place where I knew "--help-all" was handled. This turned out to be a terrible mess, so I looked around for a different path forward (the one here now), and that turned out to work much better.

In that old world, parse-options doesn't change and the builtins need a lot of surgery (and probably still aren't completely right)—I can see how that note is confusing without that context, though, and I'll drop it in future rounds (esp. now we've discussed this version).

When asking for short help on a previous command, the user may use their shell history to recall a command like

    git rebase new-base

Then inserting "-h" after "rebase" doesn't yield the help; make it so.

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

Notes:
    I reviewed the following results
    
        git grep '[^[:alpha:]-]-h' 'Documentation/**adoc' ':(exclude)Documentation/RelNotes'
    
    The 2 commands that accept "-h" to mean other things are git-grep(1) and
    git-ls-remote(1).
    
    - "git grep -h 123" gives results sans filenames, while a lone "-h"
      gives help. Behavior is unchanged from 2.48.1 (my local install).
    - "git ls-remote -h" gives help; "git ls-remote -h origin" lists heads.
      Behavior is unchanged from 2.48.1.
    
    But I'd welcome a more thorough check to make sure I didn't overlook
    things. This is probably the most controversial patch, and if it's
    dropped, that would be alright (though it is helpful for me to reduce
    typing).

 builtin/merge-recursive.c | 2 +-
 git.c                     | 2 +-
 parse-options.c           | 2 +-
 usage.c                   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

I doubt this is a good idea for at least two reasons.

 * As "git help cli" says, we should be discouraging, not
   encouraging peope to say "git rebase new-base -h".

 * "git rebase -h new-base" that shows help is probably a bug (think
   what should happen with s/rebase/grep/) in the first place.

If anything, we probably should fix the "-h" codepath to

 - react and do the short-help only when "-h" is the only command
   line option; with argument, it should probably barf, saying "-h
   does not take an argument".

 - if "-h" resulted in reported an alias, it should stop there.
   E.g. "git -c alias.x=ls-files x -h" would currently invoke "git
   ls-files -h" after reporting that 'x' is aliased to 'ls-files'.
   If the alias is to one of our commands, it is not too risky, but
   otherwise we should not assume it is safe to append "-h" to the
   underlying command and run it.  Imagine

   $ git -c alias.x='!echo rm -rf .' x -h

   and worse yet, if your alias did not have "echo" in it ;-)???

The only end-user expectation we can safely assume is when they
say,
 
   $ git frotz -h

is that they would get a help on 'frotz' without doing any harm. If frotz is an alias to some external command, for which we have no idea what it would do when we run it with "-h" appended to the command line, the user would be in a lot of pain if the aliased operation is destructive.

On Sat, Jul 26, 2025 at 8:28 PM Junio C Hamano <gitster@pobox.com> wrote:

As I said in the cover letter, I think this is the most controversial and can be dropped. However…

…that's not what I'm encouraging here. Instead, it's more like below:

And at least according to my tests, "git grep -h new-base" still greps
rather than shows help. Compare
- "git grep -h squash" (greps squash)
- "git rebase -h @{u}" (shows help)

I think we have the first half already ("argc == 2" in the preimage). I'm not interested in writing the second half right now, personally, if we end up dropping this patch, so someone else could take that up.

I think this is being discussed on another series, and I'll leave it there :)

On Wed, Jul 30, 2025 at 05:55:32PM -0400, D. Ben Knoble wrote:

I was somewhat surprised that grep would still work, looking at the diff. The reason is that it does not call any of the touched functions, but instead relies on this line in parse-options to trigger help:

  $ git grep -A2 'lone -h'
  parse-options.c:                /* lone -h asks for help */
  parse-options.c-                if (internal_help && ctx->total == 1 && !strcmp(arg + 1, "h"))
  parse-options.c-                        goto show_usage;

rather than any of the if_asked functions you touched. So I think there may be two problems:

  1. You didn't touch this spot in the parse-options code. Would you
     need to for it to be consistent with the non-parse-options callers
     that use the if_asked functions?

  2. We can only get here if we make it past the help check in
     run_builtin(), that you do modify in your patch. That works for
     git-grep because it does not use RUN_SETUP, and calls
     parse_options() before checking whether we are in a repository.

     So in run_builtin() we do set "help" to 1, but it does nothing
     without the RUN_SETUP flag. But imagine a hypothetical git-foo that
     takes a "-h" option and does require a repository. It would set the
     RUN_SETUP flag, and then:

       git foo -h bar

     would show the help before we even get into cmd_foo() to parse the
     options.

BTW, I applied your patch 4 manually to dig into this. I wasn't able to apply the whole series. It doesn't go on top of the current 'master', and applying with "am -3" mentions "sha1 information is lacking or useless". Did you build this on some other unpublished series?

-Peff

On Sat, Aug 2, 2025 at 5:23 AM Jeff King <peff@peff.net> wrote:

I think I need to consider both questions in parallel: as you point out, this patch probably doesn't work for a hypothetical command that both needs a repository and has a "-h" option. (I note that ls-remote also is RUN_SETUP_GENTLY, like grep). Since no such command exists today, we /could/ take some version of this patch and refine later if a command needs both RUN_SETUP and a "-h" option. Or we could reject this patch (assuming there's no workaround for now). Given Junio's concern, I'm inclined to just drop the patch from the series…

…which moots question 1, I think. OTOH, if we keep the patch, it does seem like we might want the parse-options API to be consistent. Fortunately, I don't think this area needs adjusted for 3/4 based on the tests.

The base is published and mentioned in the cover letter [1]; if I can make that more explicit in any way going forward, please let me know!

[1]: https://lore.kernel.org/git/20250726165320.4039-1-ben.knoble+github@gmail.com/

On Sat, Aug 02, 2025 at 12:10:17PM -0400, D. Ben Knoble wrote:

I think I mostly share Junio's concern. The issue is that we want to detect the "user is asking for help" situation without having access to the option-parsing information for the actual sub-command. And so our strategy has been to make the rule for triggering "asking for help" to be fairly conservative.

If we loosened it now, even though it happens to work for all current commands, we'd later potentially have to re-tighten (which is awkward) or start carrying extra signals back to git.c (e.g., a HAVE_H_OPTION flag).

Ah, hmm. I was trying on top of ua/t1517-short-help-tests, which still fails. But it works if I merge that branch to 'master'. I guess that's what you meant by "Merge that branch to a new topic branch".

-Peff

On Sat, Aug 2, 2025 at 12:28 PM Jeff King <peff@peff.net> wrote:

Sensible. Will drop.

Ah, yep, that could be clearer (and it didn't occur to me that basing on a merge would cause application issues relative to basing directly on the branch). If it helps, the graph I have is pasted below (but beware GMail whitespace munging?)

    git log --graph --boundary ^origin/master @

* 3099d83cdf (HEAD -> help-all-tweaks) builtins: show help on
"-h"/"--help-all" with more than 2 arguments left
* 352fe87c80 builtin: also setup gently for --help-all
* 56665594a8 parse-options: name flags passed to usage_with_options_internal
* 852a4547af t1517: fixup for ua/t1517-short-help-tests
*   14c7e9dddd Merge remote-tracking branch
'broken-out/ua/t1517-short-help-tests' into help-all-tweaks-tests
|\
| * 344c7067e6 (broken-out/ua/t1517-short-help-tests) t5200: move
`update-server-info -h` test from t1517
| * e552926bc4 t/t1517: automate `git subcmd -h` tests outside a repository
| o 16bd9f20a4 (tag: v2.50.0) Git 2.50
o | e4ef0485fd (origin/master, origin/HEAD, broken-out/master,
broken-out/HEAD) The fourteenth batch
 /

I've also (now) made the series available on GitHub at

    https://github.com/benknoble/git/tree/help-all-tweaks

Changes from v1:
- tweak refactor commit message to indicate no behavior is changed;
- use #define'd constants instead of an enum for internal flags;
- drop controversial [4/4]

QQ: Is there a better way to add --cc's to format-patch than grabbing the right
folks off threads from lore.kernel.org/git? (I could also grab them out of my
mail client.) I assume this is where using tools like b4 starts to really shine?
To be clear: I mean CC's _in addition to_ the use of

    sendemail.ccCmd = perl contrib/contacts/git-contacts

to keep folks that participated in prior rounds of discussion abreast of the next version.

-------- Original --------

This series depends on ua/t1517-short-help-tests with some fixes, which show up in the first patch. Merge that branch to a new topic branch:

    git switch -c topic v2.50.0 # or origin/master
    git merge gitster/ua/t1517-short-help-tests

then apply this series.

This series enables --help-all outside of repository contexts, and allows -h with other arguments (without breaking existing ls-remote/grep usage).

It consists of preparatory steps (fixes for a dependency branch; refactoring to make an internal helper's arguments clearer) followed by the main commits.

v1: https://lore.kernel.org/git/20250726165320.4039-1-ben.knoble+github@gmail.com/
Published-as: https://github.com/benknoble/git/tree/help-all-tweaks

D. Ben Knoble (3):
  t1517: fixup for ua/t1517-short-help-tests
  parse-options: refactor flags for usage_with_options_internal
  builtin: also setup gently for --help-all

 builtin/merge-recursive.c     |  3 ++-
 git.c                         |  2 +-
 parse-options.c               | 30 +++++++++++++++++++++++-------
 t/t1517-outside-repo.sh       | 11 +++++++----
 t/t5200-update-server-info.sh |  2 +-
 usage.c                       |  3 ++-
 6 files changed, 36 insertions(+), 15 deletions(-)

Diff-intervalle contre v1 :
1:  852a4547af ! 1:  7a3e0a601d t1517: fixup for ua/t1517-short-help-tests
    @@ Commit message
     
         - drop spurious message during test
         - fix known breakages that actually work
    +    - fix instaweb marker for known failure
         - fix new t5200 test
     
     
    @@ Notes
     
      ## t/t1517-outside-repo.sh ##
     @@
    + 	case "$cmd" in
    + 	archimport | cvsexportcommit | cvsimport | cvsserver | daemon | \
      	difftool--helper | filter-branch | fsck-objects | get-tar-commit-id | \
    - 	http-backend | http-fetch | http-push | init-db | instaweb.sh | \
    +-	http-backend | http-fetch | http-push | init-db | instaweb.sh | \
    ++	http-backend | http-fetch | http-push | init-db | \
      	merge-octopus | merge-one-file | merge-resolve | mergetool | \
     -	mktag | p4 | p4.py | pickaxe | quiltimport | remote-ftp | remote-ftps | \
     -	remote-http | remote-https | replay | request-pull | send-email | \
2:  56665594a8 ! 2:  db74b1eff7 parse-options: name flags passed to usage_with_options_internal
    @@ Metadata
     Author: D. Ben Knoble <ben.knoble+github@gmail.com>
     
      ## Commit message ##
    -    parse-options: name flags passed to usage_with_options_internal
    +    parse-options: refactor flags for usage_with_options_internal
     
         When reading or editing calls to usage_with_options_internal, it is
         difficult to tell what trailing "0, 0", "0, 1", "1, 0" arguments mean
         (NB there is never a "1, 1" case).
     
    -    Give the flags readable names to improve call-sites.
    +    Give the flags readable names to improve call-sites without changing any
    +    behavior.
     
      ## parse-options.c ##
     @@ parse-options.c: static void free_preprocessed_options(struct option *options)
      	free(options);
      }
      
    -+enum usage_style {
    -+	style_normal = 0,
    -+	style_full = 1,
    -+};
    -+
    -+enum usage_output {
    -+	to_out = 0,
    -+	to_err = 1,
    -+};
    ++#define USAGE_NORMAL 0
    ++#define USAGE_FULL 1
    ++#define USAGE_TO_STDOUT 0
    ++#define USAGE_TO_STDERR 1
     +
      static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t *,
      							 const char * const *,
      							 const struct option *,
     -							 int, int);
    -+							 enum usage_style,
    -+							 enum usage_output);
    ++							 int full_usage,
    ++							 int usage_to_stderr);
      
      enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
      					 const struct option *options,
    @@ parse-options.c: enum parse_opt_result parse_options_step(struct parse_opt_ctx_t
      
      		if (internal_help && !strcmp(arg + 2, "help-all"))
     -			return usage_with_options_internal(ctx, usagestr, options, 1, 0);
    -+			return usage_with_options_internal(ctx, usagestr, options, style_full, to_out);
    ++			return usage_with_options_internal(ctx, usagestr, options,
    ++							   USAGE_FULL, USAGE_TO_STDOUT);
      		if (internal_help && !strcmp(arg + 2, "help"))
      			goto show_usage;
      		switch (parse_long_opt(ctx, arg + 2, options)) {
    @@ parse-options.c: enum parse_opt_result parse_options_step(struct parse_opt_ctx_t
      
       show_usage:
     -	return usage_with_options_internal(ctx, usagestr, options, 0, 0);
    -+	return usage_with_options_internal(ctx, usagestr, options, style_normal, to_out);
    ++	return usage_with_options_internal(ctx, usagestr, options,
    ++					   USAGE_NORMAL, USAGE_TO_STDOUT);
      }
      
      int parse_options_end(struct parse_opt_ctx_t *ctx)
    -@@ parse-options.c: static const struct option *find_option_by_long_name(const struct option *opts,
    - static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t *ctx,
    - 							 const char * const *usagestr,
    - 							 const struct option *opts,
    --							 int full, int err)
    -+							 enum usage_style help_style,
    -+							 enum usage_output to_where)
    - {
    - 	const struct option *all_opts = opts;
    --	FILE *outfile = err ? stderr : stdout;
    -+	FILE *outfile = to_where == to_err ? stderr : stdout;
    - 	int need_newline;
    - 
    - 	const char *usage_prefix = _("usage: %s");
    -@@ parse-options.c: static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
    - 	if (!usagestr)
    - 		return PARSE_OPT_HELP;
    - 
    --	if (!err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL)
    -+	if (to_where != to_err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL)
    - 		fprintf(outfile, "cat <<\\EOF\n");
    - 
    - 	while (*usagestr) {
    -@@ parse-options.c: static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
    - 				fprintf(outfile, "%s\n", _(opts->help));
    - 			continue;
    - 		}
    --		if (!full && (opts->flags & PARSE_OPT_HIDDEN))
    -+		if (help_style != style_full && (opts->flags & PARSE_OPT_HIDDEN))
    - 			continue;
    - 
    - 		if (need_newline) {
    -@@ parse-options.c: static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
    - 	}
    - 	fputc('\n', outfile);
    - 
    --	if (!err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL)
    -+	if (to_where != to_err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL)
    - 		fputs("EOF\n", outfile);
    - 
    - 	return PARSE_OPT_HELP;
     @@ parse-options.c: static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
      void NORETURN usage_with_options(const char * const *usagestr,
      			const struct option *opts)
      {
     -	usage_with_options_internal(NULL, usagestr, opts, 0, 1);
    -+	usage_with_options_internal(NULL, usagestr, opts, style_normal, to_err);
    ++	usage_with_options_internal(NULL, usagestr, opts,
    ++				    USAGE_NORMAL, USAGE_TO_STDERR);
      	exit(129);
      }
      
    @@ parse-options.c: void show_usage_with_options_if_asked(int ac, const char **av,
      {
      	if (ac == 2 && !strcmp(av[1], "-h")) {
     -		usage_with_options_internal(NULL, usagestr, opts, 0, 0);
    -+		usage_with_options_internal(NULL, usagestr, opts, style_normal, to_out);
    ++		usage_with_options_internal(NULL, usagestr, opts,
    ++					    USAGE_NORMAL, USAGE_TO_STDOUT);
      		exit(129);
      	}
      }
3:  352fe87c80 ! 3:  cb4113f77d builtin: also setup gently for --help-all
    @@ Commit message
         The exception is merge-recursive, whose help block doesn't use newer
         APIs.
     
    +    Best-viewed-with: --ignore-space-change
    +
     
      ## Notes ##
         Some usage.c callers, like check-ref-format, probably deserve to be
    @@ parse-options.c: void show_usage_with_options_if_asked(int ac, const char **av,
      				      const struct option *opts)
      {
     -	if (ac == 2 && !strcmp(av[1], "-h")) {
    --		usage_with_options_internal(NULL, usagestr, opts, style_normal, to_out);
    +-		usage_with_options_internal(NULL, usagestr, opts,
    +-					    USAGE_NORMAL, USAGE_TO_STDOUT);
     -		exit(129);
     +	if (ac == 2) {
     +		if (!strcmp(av[1], "-h")) {
    -+			usage_with_options_internal(NULL, usagestr, opts, style_normal, to_out);
    ++			usage_with_options_internal(NULL, usagestr, opts,
    ++						    USAGE_NORMAL, USAGE_TO_STDOUT);
     +			exit(129);
     +		} else if (!strcmp(av[1], "--help-all")) {
    -+			usage_with_options_internal(NULL, usagestr, opts, style_full, to_out);
    ++			usage_with_options_internal(NULL, usagestr, opts,
    ++						    USAGE_FULL, USAGE_TO_STDOUT);
     +			exit(129);
     +		}
      	}
4:  3099d83cdf < -:  ---------- builtins: show help on "-h"/"--help-all" with more than 2 arguments left

base-commit: e4ef0485fd78fcb05866ea78df35796b904e4a8e prerequisite-patch-id: ffce2dd036e61c8d36485a17321f858e454db874 prerequisite-patch-id: 52539022c824997adfc1be0bed8de6b1851d2187

Changes from v2:
- Rebase on unpublished ua/t1517… v5 (https://lore.kernel.org/git/20250803020744.1037392-1-usmanakinyemi202@gmail.com/)
  which is rebased on master
- Changed fixup to account for instaweb Perl dependency

Changes from v1:
- tweak refactor commit message to indicate no behavior is changed;
- use #define'd constants instead of an enum for internal flags;
- drop controversial [4/4]

QQ: Is there a better way to add --cc's to format-patch than grabbing
the right folks off threads from lore.kernel.org/git? (I could also grab
them out of my mail client.) I assume this is where using tools like b4
starts to really shine? To be clear: I mean CC's _in addition to_ the
use of

    sendemail.ccCmd = perl contrib/contacts/git-contacts

to keep folks that participated in prior rounds of discussion abreast of the next version.

-------- Original --------

This series depends on ua/t1517-short-help-tests with some fixes, which show up in the first patch. Start a new topic from the (unpublished) v5 of that branch:

    git switch -c topic origin/master
    git am ua/t1517-short-help-tests-v5.mbox

Then apply this series.

This series enables --help-all outside of repository contexts, and allows -h with other arguments (without breaking existing ls-remote/grep usage).

It consists of preparatory steps (fixes for a dependency branch; refactoring to make an internal helper's arguments clearer) followed by the main commits.

v1: https://lore.kernel.org/git/20250726165320.4039-1-ben.knoble+github@gmail.com/
v2: https://lore.kernel.org/git/20250803012613.54086-1-ben.knoble+github@gmail.com/
Published-as: https://github.com/benknoble/git/tree/help-all-tweaks

D. Ben Knoble (3):
  t1517: fixup for ua/t1517-short-help-tests
  parse-options: refactor flags for usage_with_options_internal
  builtin: also setup gently for --help-all

 builtin/merge-recursive.c |  3 ++-
 git.c                     |  2 +-
 parse-options.c           | 30 +++++++++++++++++++++++-------
 t/t1517-outside-repo.sh   | 12 +++++++++++-
 usage.c                   |  3 ++-
 5 files changed, 39 insertions(+), 11 deletions(-)

Diff-intervalle contre v2 :
1:  e552926bc4 < -:  ---------- t/t1517: automate `git subcmd -h` tests outside a repository
2:  344c7067e6 < -:  ---------- t5200: move `update-server-info -h` test from t1517
3:  7a3e0a601d < -:  ---------- t1517: fixup for ua/t1517-short-help-tests
-:  ---------- > 1:  855931592f t1517: fixup for ua/t1517-short-help-tests
4:  db74b1eff7 = 2:  2392039374 parse-options: refactor flags for usage_with_options_internal
5:  cb4113f77d ! 3:  4dfc046e6d builtin: also setup gently for --help-all
    @@ t/t1517-outside-repo.sh
      		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_outcome $prereq "'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' '
    + test_done
     
      ## usage.c ##
     @@ usage.c: static void show_usage_if_asked_helper(const char *err, ...)

base-commit: 866e6a391f466baeeb98bc585845ea638322c04b prerequisite-patch-id: eb2c55de73b2b733aa0c8c94469a45d9bc361d81 prerequisite-patch-id: 12ddad29e334853cf61c797a2c08302893f4b909 prerequisite-patch-id: 250bae2541030fcdfc5b35ede44c23e1138c7a3c

"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

I've been familiar with "git commit -h" (and commands other than "commit") outside a repository, but did not even know that "git commit --help-all" didn't work outside. For commands that use parse-options, these come from the same source of informatino, so it does not make any sense for one to work and the other to refuse to work. Good.

I somehow thought we already talked you out of this.

Do you mean something like "git add -h foo" and "git add -h -N foo" would say "'git add foo' would add the current contents in foo to the index" and "'git add -N foo' would make the index aware of the path foo without actually adding its contents (yet)"? I do not think it makes much sense to behave exactly the same as "git add -h" when the user says "git add -h foo" or "git add -h -N foo", as if we didn't even see the extra things on the command line.

Thanks.

On Mon, Aug 4, 2025 at 12:53 AM Junio C Hamano <gitster@pobox.com> wrote:

You did! Forgot to drop that segment from the cover letter.

So this is moot. Good catch, thanks.

- fix instaweb test prereqs

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---
 t/t1517-outside-repo.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:

Ideally this would want to be squashed (or moved) into the base topic. Can you two coordinate among yourselves?

Thanks.

On Sun, Aug 3, 2025 at 12:41 PM Junio C Hamano <gitster@pobox.com> wrote:

Agreed, and happy to—just not sure what that looks like here. Advice welcome :)

When reading or editing calls to usage_with_options_internal, it is difficult to tell what trailing "0, 0", "0, 1", "1, 0" arguments mean (NB there is never a "1, 1" case).

Give the flags readable names to improve call-sites without changing any behavior.

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---
 parse-options.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Git experts often check the help summary of a command to make sure they spell options right when suggesting advice to colleagues. Further, they might check hidden options when responding to queries about deprecated options like git-rebase(1)'s "preserve merges" option. But some commands don't support "--help-all" outside of a git directory. Running (for example)

    git rebase --help-all

outside a directory fails in "setup_git_directory", erroring with the localized form of

    fatal: not a git repository (or any of the parent directories): .git

Like 99caeed05d (Let 'git <command> -h' show usage without a git dir, 2009-11-09), we want to show the "--help-all" output even without a git dir. Make "--help-all" where we expect "-h" to mean "setup_git_directory_gently", and interpose early in the natural place ("show_usage_with_options_if_asked").

Do the same for usage callers with show_usage_if_asked.

The exception is merge-recursive, whose help block doesn't use newer APIs.

Best-viewed-with: --ignore-space-change
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

Notes:
    Some usage.c callers, like check-ref-format, probably deserve to be
    ported to parse-options at this point.

 builtin/merge-recursive.c |  3 ++-
 git.c                     |  2 +-
 parse-options.c           | 14 ++++++++++----
 t/t1517-outside-repo.sh   |  4 ++++
 usage.c                   |  3 ++-
 5 files changed, 19 insertions(+), 7 deletions(-)

- drop spurious message during test
- fix known breakages that actually work
- fix instaweb marker for known failure: it is not a failure, but should not
  have .sh suffix
- fix new t5200 test

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

Notes:
    I expect this and other fixes to get squashed into the upstream branch,
    but I'm including it here so it's easy to create a clean build.

 t/t1517-outside-repo.sh       | 7 +++----
 t/t5200-update-server-info.sh | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

When reading or editing calls to usage_with_options_internal, it is difficult to tell what trailing "0, 0", "0, 1", "1, 0" arguments mean (NB there is never a "1, 1" case).

Give the flags readable names to improve call-sites without changing any behavior.

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

I went with a slightly more verbose approach here: Junio argued that the "0" case is common so deserves to be short, but it's still hard IMO to tell what "0" represents when reading only the function calls (say, in a patch with minimal context).

Unfortunately, the deep rightward indent is also a bit challenging on my eyes :/

 parse-options.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Git experts often check the help summary of a command to make sure they spell options right when suggesting advice to colleagues. Further, they might check hidden options when responding to queries about deprecated options like git-rebase(1)'s "preserve merges" option. But some commands don't support "--help-all" outside of a git directory. Running (for example)

    git rebase --help-all

outside a directory fails in "setup_git_directory", erroring with the localized form of

    fatal: not a git repository (or any of the parent directories): .git

Like 99caeed05d (Let 'git <command> -h' show usage without a git dir, 2009-11-09), we want to show the "--help-all" output even without a git dir. Make "--help-all" where we expect "-h" to mean "setup_git_directory_gently", and interpose early in the natural place ("show_usage_with_options_if_asked").

Do the same for usage callers with show_usage_if_asked.

The exception is merge-recursive, whose help block doesn't use newer APIs.

Best-viewed-with: --ignore-space-change
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---

Notes:
    Some usage.c callers, like check-ref-format, probably deserve to be
    ported to parse-options at this point.

 builtin/merge-recursive.c |  3 ++-
 git.c                     |  2 +-
 parse-options.c           | 14 ++++++++++----
 t/t1517-outside-repo.sh   |  4 ++++
 usage.c                   |  3 ++-
 5 files changed, 19 insertions(+), 7 deletions(-)