Re: [PATCH] rev-parse: have --parseopt callers exit 0 on --help

[Jeff King <peff@peff.net>]

On Tue, Mar 17, 2026 at 11:59:02AM +0000, brian m. carlson wrote:

> On 2026-03-17 at 00:47:19, Junio C Hamano wrote:
> > "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> > 
> > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > > ---
> > >  parse-options.c               | 2 +-
> > >  t/t1502-rev-parse-parseopt.sh | 9 +++++++--
> > >  t/t1502/optionspec-neg.help   | 1 +
> > >  t/t1502/optionspec.help       | 1 +
> > >  4 files changed, 10 insertions(+), 3 deletions(-)
> > 
> > Has t1517 passed for you?
> > 
> > Queued directly on top of v2.53.0, I am seeing:
> > 
> > >>>>>
> > expecting success of 1517.169 ''git instaweb -h' outside a repository':
> >                 test_expect_code 129 nongit git $cmd -h >usage &&
> >                 test_grep "[Uu]sage: git $cmd " usage
> > 
> > test_expect_code: command exited with 0, we wanted 129 nongit git instaweb -h
> > not ok 169 - 'git instaweb -h' outside a repository
> > <<<<<
> 
> I thought the tests passed, but I may have neglected to run them on the
> latest revision.  Go ahead and drop this for now and I'll send out a v2
> either tonight or later this week.  My apologies.

Hmm. Is this just a matter of tweaking the tests, or is there something
bigger going on?

These tests are _expecting_ the 129 exit code from most commands. And
indeed, "git log -h" produces 129 for example. So it is not just shell
scripts using "rev-parse --parseopt" that do this, and those scripts are
(currently) consistent with the rest of Git.

You'd need something like this (untested) to hit the non-shell commands:

diff --git a/parse-options.c b/parse-options.c
index 85e2f0ea7c..0ad43ca278 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1197,6 +1197,7 @@ int parse_options(int argc, const char **argv,
 	parse_options_start_1(&ctx, argc, argv, prefix, options, flags);
 	switch (parse_options_step(&ctx, options, usagestr)) {
 	case PARSE_OPT_HELP:
+		exit(0);
 	case PARSE_OPT_ERROR:
 		exit(129);
 	case PARSE_OPT_COMPLETE:
@@ -1495,11 +1496,11 @@ void show_usage_with_options_if_asked(int ac, const char **av,
 		if (!strcmp(av[1], "-h")) {
 			usage_with_options_internal(NULL, usagestr, opts,
 						    USAGE_NORMAL, USAGE_TO_STDOUT);
-			exit(129);
+			exit(0);
 		} else if (!strcmp(av[1], "--help-all")) {
 			usage_with_options_internal(NULL, usagestr, opts,
 						    USAGE_FULL, USAGE_TO_STDOUT);
-			exit(129);
+			exit(0);
 		}
 	}
 }

I agree with the general idea that "-h" usually should exit 0. But this
is not just a bug fix for --parseopt, but a change in overall intent. It
might be worth digging in the commit history or list archive to see if
there's any discussion on why we are using 129 in the first place.

-Peff