parse-options: exit 0 on -h

The standard philosophy for Unix software when a help option (such as
--help) is specified is that the software should exit 0, printing the
help output to standard output, since the standard output is for
user-requested output and the program performed the requested task
successfully.  If the user specifies an incorrect option, then the help
output should be printed to standard error (since the user has made a
mistake) and it should exit unsuccessfully.

Most of our commands currently exit 129 on receiving the -h option to
print the short help, which does not line up with the standard
philosophy above.  Let's change that to exit 0 instead.

This requires changes to a variety of tests which previously wanted the
129 exit code, so update them.  Note that because git diff does its own
option parsing, it still exits with 129, so update some of the tests to
expect either exit status.

Some commands also now pass with -h but not --help-all, so handle those
cases differently for those commands.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin/blame.c

@@ -1013,6 +1013,7 @@ int cmd_blame(int argc,
 		case PARSE_OPT_UNKNOWN:
 			break;
 		case PARSE_OPT_HELP:
+			exit(0);
 		case PARSE_OPT_HELP_ERROR:
 		case PARSE_OPT_ERROR:
 		case PARSE_OPT_SUBCOMMAND:

builtin/shortlog.c

@@ -433,6 +433,7 @@ int cmd_shortlog(int argc,
 		case PARSE_OPT_UNKNOWN:
 			break;
 		case PARSE_OPT_HELP:
+			exit(0);
 		case PARSE_OPT_HELP_ERROR:
 		case PARSE_OPT_ERROR:
 		case PARSE_OPT_SUBCOMMAND:

builtin/update-index.c

@@ -1133,6 +1133,7 @@ int cmd_update_index(int argc,
 			break;
 		switch (parseopt_state) {
 		case PARSE_OPT_HELP:
+			exit(0);
 		case PARSE_OPT_HELP_ERROR:
 		case PARSE_OPT_ERROR:
 			exit(129);

contrib/subtree/t/t7900-subtree.sh

@@ -99,7 +99,7 @@ test_create_subtree_add () {
 }
 
 test_expect_success 'shows short help text for -h' '
-	test_expect_code 129 git subtree -h >out 2>err &&
+	git subtree -h >out 2>err &&
 	test_must_be_empty err &&
 	grep -e "^ *or: git subtree pull" out &&
 	grep -F -e "--[no-]annotate" out

parse-options.c

@@ -1135,8 +1135,9 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
 		case PARSE_OPT_UNKNOWN:
 			goto unknown;
 		case PARSE_OPT_HELP:
-		case PARSE_OPT_HELP_ERROR:
 			goto show_usage;
+		case PARSE_OPT_HELP_ERROR:
+			goto show_usage_stderr;
 		case PARSE_OPT_NON_OPTION:
 		case PARSE_OPT_SUBCOMMAND:
 		case PARSE_OPT_COMPLETE:
@@ -1170,6 +1171,9 @@ unknown:
  show_usage:
 	return usage_with_options_internal(ctx, usagestr, options,
 					   USAGE_NORMAL, USAGE_TO_STDOUT);
+ show_usage_stderr:
+	return usage_with_options_internal(ctx, usagestr, options,
+					   USAGE_NORMAL, USAGE_TO_STDERR);
 }
 
 int parse_options_end(struct parse_opt_ctx_t *ctx)
@@ -1201,6 +1205,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_HELP_ERROR:
 	case PARSE_OPT_ERROR:
 		exit(129);
@@ -1500,11 +1505,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);
 		}
 	}
 }

t/for-each-ref-tests.sh

@@ -522,7 +522,7 @@ test_expect_success 'Verify descending sort' '
 '
 
 test_expect_success 'Give help even with invalid sort atoms' '
-	test_expect_code 129 ${git_for_each_ref} --sort=bogus -h >actual 2>&1 &&
+	${git_for_each_ref} --sort=bogus -h >actual 2>&1 &&
 	grep "^usage: ${git_for_each_ref}" actual
 '
 

t/t0012-help.sh

@@ -260,7 +260,7 @@ do
 		(
 			GIT_CEILING_DIRECTORIES=$(pwd) &&
 			export GIT_CEILING_DIRECTORIES &&
-			test_expect_code 129 git -C sub $builtin -h >output 2>err
+			git -C sub $builtin -h >output 2>err
 		) &&
 		test_must_be_empty err &&
 		test_grep usage output

t/t0040-parse-options.sh

@@ -68,7 +68,7 @@ Alias
 EOF
 
 test_expect_success 'test help' '
-	test_must_fail test-tool parse-options -h >output 2>output.err &&
+	test-tool parse-options -h >output 2>output.err &&
 	test_must_be_empty output.err &&
 	test_cmp expect output
 '

t/t0450-txt-doc-vs-help.sh

@@ -29,7 +29,7 @@ help_to_synopsis () {
 		return 0
 	fi &&
 	mkdir -p "$out_dir" &&
-	test_expect_code 129 git $builtin -h >"$out.raw" 2>&1 &&
+	test_might_fail git $builtin -h >"$out.raw" 2>&1 &&
 	sed -n \
 		-e '1,/^$/ {
 			/^$/d;

t/t0610-reftable-basics.sh

@@ -15,9 +15,9 @@ export GIT_TEST_DEFAULT_REF_FORMAT
 INVALID_OID=$(test_oid 001)
 
 test_expect_success 'pack-refs does not crash with -h' '
-	test_expect_code 129 git pack-refs -h >usage &&
+	git pack-refs -h >usage &&
 	test_grep "[Uu]sage: git pack-refs " usage &&
-	test_expect_code 129 nongit git pack-refs -h >usage &&
+	nongit git pack-refs -h >usage &&
 	test_grep "[Uu]sage: git pack-refs " usage
 '
 

t/t1403-show-ref.sh

@@ -165,7 +165,7 @@ test_expect_success 'show-ref --branches, --tags, --head, pattern' '
 '
 
 test_expect_success 'show-ref --heads is deprecated and hidden' '
-	test_expect_code 129 git show-ref -h >short-help &&
+	git show-ref -h >short-help &&
 	test_grep ! -e --heads short-help &&
 	git show-ref --heads >actual 2>warning &&
 	test_grep ! deprecated warning &&

t/t1410-reflog.sh

@@ -107,12 +107,12 @@ test_expect_success setup '
 '
 
 test_expect_success 'correct usage on sub-command -h' '
-	test_expect_code 129 git reflog expire -h >err &&
+	git reflog expire -h >err &&
 	grep "git reflog expire" err
 '
 
 test_expect_success 'correct usage on "git reflog show -h"' '
-	test_expect_code 129 git reflog show -h >err &&
+	git reflog show -h >err &&
 	grep -F "git reflog [show]" err
 '
 

t/t1418-reflog-exists.sh

@@ -12,7 +12,7 @@ test_expect_success 'setup' '
 
 test_expect_success 'usage' '
 	test_expect_code 129 git reflog exists &&
-	test_expect_code 129 git reflog exists -h
+	git reflog exists -h
 '
 
 test_expect_success 'usage: unknown option' '

t/t1502-rev-parse-parseopt.sh

@@ -75,7 +75,7 @@ EOF
 '
 
 test_expect_success 'test --parseopt help output' '
-	test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec &&
+	git rev-parse --parseopt -- -h > output < optionspec &&
 	test_cmp "$TEST_DIRECTORY/t1502/optionspec.help" output
 '
 
@@ -89,7 +89,7 @@ test_expect_success 'test --parseopt help output no switches' '
 |EOF
 |exit 0
 END_EXPECT
-	test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec_no_switches &&
+	git rev-parse --parseopt -- -h > output < optionspec_no_switches &&
 	test_cmp expect output
 '
 
@@ -103,7 +103,7 @@ test_expect_success 'test --parseopt help output hidden switches' '
 |EOF
 |exit 0
 END_EXPECT
-	test_expect_code 129 git rev-parse --parseopt -- -h > output < optionspec_only_hidden_switches &&
+	git rev-parse --parseopt -- -h > output < optionspec_only_hidden_switches &&
 	test_cmp expect output
 '
 
@@ -119,7 +119,7 @@ test_expect_success 'test --parseopt help-all output hidden switches' '
 |EOF
 |exit 0
 END_EXPECT
-	test_expect_code 129 git rev-parse --parseopt -- --help-all > output < optionspec_only_hidden_switches &&
+	git rev-parse --parseopt -- --help-all > output < optionspec_only_hidden_switches &&
 	test_cmp expect output
 '
 
@@ -258,7 +258,7 @@ test_expect_success 'test --parseopt help output: "wrapped" options normal "or:"
 	|exit 0
 	END_EXPECT
 
-	test_must_fail git rev-parse --parseopt -- -h <spec >actual &&
+	git rev-parse --parseopt -- -h <spec >actual &&
 	test_cmp expect actual
 '
 
@@ -296,12 +296,12 @@ test_expect_success 'test --parseopt help output: multi-line blurb after empty l
 	|exit 0
 	END_EXPECT
 
-	test_must_fail git rev-parse --parseopt -- -h <spec >actual &&
+	git rev-parse --parseopt -- -h <spec >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'test --parseopt help output for optionspec-neg' '
-	test_expect_code 129 git rev-parse --parseopt -- \
+	git rev-parse --parseopt -- \
 		-h >output <"$TEST_DIRECTORY/t1502/optionspec-neg" &&
 	test_cmp "$TEST_DIRECTORY/t1502/optionspec-neg.help" output
 '

t/t1517-outside-repo.sh

@@ -129,18 +129,25 @@ do
 	archimport | citool | credential-netrc | credential-libsecret | \
 	credential-osxkeychain | cvsexportcommit | cvsimport | cvsserver | \
 	daemon | \
-	difftool--helper | filter-branch | format-rev | fsck-objects | \
-	get-tar-commit-id | \
+	difftool--helper | format-rev | fsck-objects |  get-tar-commit-id | \
 	gui | gui--askpass | \
-	http-backend | http-fetch | http-push | init-db | instaweb | \
-	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 | \
-	sh-i18n--envsubst | shell | show | stage | submodule | svn | \
-	upload-archive--writer | upload-pack | web--browse | whatchanged)
-		expect_outcome=expect_failure ;;
+	http-backend | http-fetch | http-push | init-db | \
+	mktag | p4 | p4.py | pickaxe | remote-ftp | remote-ftps | \
+	remote-http | remote-https | replay | send-email | \
+	sh-i18n--envsubst | shell | show | stage | \
+	upload-archive--writer | upload-pack | whatchanged)
+		h_expect_outcome=expect_failure
+		all_expect_outcome=expect_failure
+		;;
+	filter-branch | merge-octopus | merge-one-file | merge-resolve | \
+	mergetool | submodule | svn | web--browse)
+		h_expect_outcome=expect_success
+		all_expect_outcome=expect_failure
+		;;
 	*)
-		expect_outcome=expect_success ;;
+		h_expect_outcome=expect_success
+		all_expect_outcome=expect_success
+		;;
 	esac
 	case "$cmd" in
 	instaweb)
@@ -150,20 +157,20 @@ do
 	*)
 		prereq= ;;
 	esac
-	test_$expect_outcome $prereq "'git $cmd -h' outside a repository" '
-		test_expect_code 129 nongit git $cmd -h >usage &&
+	test_$h_expect_outcome $prereq "'git $cmd -h' outside a repository" '
+		nongit git $cmd -h >usage &&
 		test_grep "[Uu]sage: git $cmd " usage
 	'
-	test_$expect_outcome $prereq "'git $cmd --help-all' outside a repository" '
-		test_expect_code 129 nongit git $cmd --help-all >usage &&
+	test_$all_expect_outcome $prereq "'git $cmd --help-all' outside a repository" '
+		nongit git $cmd --help-all >usage &&
 		test_grep "[Uu]sage: git $cmd " usage
 	'
 done
 
 test_expect_success 'fmt-merge-msg does not crash with -h' '
-	test_expect_code 129 git fmt-merge-msg -h >usage &&
+	git fmt-merge-msg -h >usage &&
 	test_grep "[Uu]sage: git fmt-merge-msg " usage &&
-	test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
+	nongit git fmt-merge-msg -h >usage &&
 	test_grep "[Uu]sage: git fmt-merge-msg " usage
 '
 

t/t1800-hook.sh

@@ -75,10 +75,10 @@ sentinel_detector () {
 test_expect_success 'git hook usage' '
 	test_expect_code 129 git hook &&
 	test_expect_code 129 git hook run &&
-	test_expect_code 129 git hook run -h &&
+	git hook run -h &&
 	test_expect_code 129 git hook run --unknown 2>err &&
 	test_expect_code 129 git hook list &&
-	test_expect_code 129 git hook list -h &&
+	git hook list -h &&
 	grep "unknown option" err
 '
 

t/t1900-repo-info.sh

@@ -150,7 +150,7 @@ test_expect_success 'git repo info --keys uses lines as its default output forma
 '
 
 test_expect_success 'git repo info -h shows only repo info usage' '
-	test_must_fail git repo info -h >actual &&
+	git repo info -h >actual &&
 	test_grep "git repo info" actual &&
 	test_grep ! "git repo structure" actual
 '

t/t1901-repo-structure.sh

@@ -225,7 +225,7 @@ test_expect_success 'progress meter option' '
 '
 
 test_expect_success 'git repo structure -h shows only repo structure usage' '
-	test_must_fail git repo structure -h >actual &&
+	git repo structure -h >actual &&
 	test_grep "git repo structure" actual &&
 	test_grep ! "git repo info" actual
 '

t/t2006-checkout-index-basic.sh

@@ -16,15 +16,15 @@ test_expect_success 'checkout-index -h in broken repository' '
 		cd broken &&
 		git init &&
 		>.git/index &&
-		test_expect_code 129 git checkout-index -h >usage 2>&1
+		git checkout-index -h >usage 2>&1
 	) &&
 	test_grep "[Uu]sage" broken/usage
 '
 
 test_expect_success 'checkout-index does not crash with -h' '
-	test_expect_code 129 git checkout-index -h >usage &&
+	git checkout-index -h >usage &&
 	test_grep "[Uu]sage: git checkout-index " usage &&
-	test_expect_code 129 nongit git checkout-index -h >usage &&
+	nongit git checkout-index -h >usage &&
 	test_grep "[Uu]sage: git checkout-index " usage
 '
 

t/t2107-update-index-basic.sh

@@ -23,7 +23,7 @@ test_expect_success 'update-index -h with corrupt index' '
 		cd broken &&
 		git init &&
 		>.git/index &&
-		test_expect_code 129 git update-index -h >usage 2>&1
+		git update-index -h >usage 2>&1
 	) &&
 	test_grep "[Uu]sage: git update-index" broken/usage
 '

t/t3004-ls-files-basic.sh

@@ -29,15 +29,15 @@ test_expect_success 'ls-files -h in corrupt repository' '
 		cd broken &&
 		git init &&
 		>.git/index &&
-		test_expect_code 129 git ls-files -h >usage 2>&1
+		git ls-files -h >usage 2>&1
 	) &&
 	test_grep "[Uu]sage: git ls-files " broken/usage
 '
 
 test_expect_success 'ls-files does not crash with -h' '
-	test_expect_code 129 git ls-files -h >usage &&
+	git ls-files -h >usage &&
 	test_grep "[Uu]sage: git ls-files " usage &&
-	test_expect_code 129 nongit git ls-files -h >usage &&
+	nongit git ls-files -h >usage &&
 	test_grep "[Uu]sage: git ls-files " usage
 '
 

t/t3200-branch.sh

@@ -33,7 +33,7 @@ test_expect_success REFFILES 'branch -h in broken repository' '
 		cd broken &&
 		git init -b main &&
 		>.git/refs/heads/main &&
-		test_expect_code 129 git branch -h >usage 2>&1
+		git branch -h >usage 2>&1
 	) &&
 	test_grep "[Uu]sage" broken/usage
 '

t/t3903-stash.sh

@@ -27,13 +27,13 @@ test_expect_success 'usage on cmd and subcommand invalid option' '
 '
 
 test_expect_success 'usage on main command -h emits a summary of subcommands' '
-	test_expect_code 129 git stash -h >usage &&
+	git stash -h >usage &&
 	grep -F "usage: git stash list" usage &&
 	grep -F "or: git stash show" usage
 '
 
 test_expect_success 'usage for subcommands should emit subcommand usage' '
-	test_expect_code 129 git stash push -h >usage &&
+	git stash push -h >usage &&
 	grep -F "usage: git stash [push" usage
 '
 

t/t4200-rerere.sh

@@ -438,7 +438,7 @@ test_expect_success 'rerere --no-no-rerere-autoupdate' '
 '
 
 test_expect_success 'rerere -h' '
-	test_must_fail git rerere -h >help &&
+	git rerere -h >help &&
 	test_grep [Uu]sage help
 '
 

t/t5200-update-server-info.sh

@@ -47,7 +47,7 @@ test_expect_success 'midx does not create duplicate pack entries' '
 '
 
 test_expect_success 'update-server-info does not crash with -h' '
-	test_expect_code 129 git update-server-info -h >usage &&
+	git update-server-info -h >usage &&
 	test_grep "[Uu]sage: git update-server-info " usage
 '
 

t/t5304-prune.sh

@@ -365,7 +365,7 @@ test_expect_success 'gc.recentObjectsHook' '
 '
 
 test_expect_success 'prune does not crash with -h' '
-	test_expect_code 129 git prune -h >usage &&
+	git prune -h >usage &&
 	test_grep "[Uu]sage: git prune " usage
 '
 

t/t5400-send-pack.sh

@@ -56,9 +56,9 @@ test_expect_success setup '
 	git log'
 
 test_expect_success 'send-pack does not crash with -h' '
-	test_expect_code 129 git send-pack -h >usage &&
+	git send-pack -h >usage &&
 	test_grep "[Uu]sage: git send-pack " usage &&
-	test_expect_code 129 nongit git send-pack -h >usage &&
+	nongit git send-pack -h >usage &&
 	test_grep "[Uu]sage: git send-pack " usage
 '
 

t/t5512-ls-remote.sh

@@ -86,7 +86,7 @@ test_expect_success 'ls-remote -h is deprecated w/o warning' '
 '
 
 test_expect_success 'ls-remote --heads is deprecated and hidden w/o warning' '
-	test_expect_code 129 git ls-remote -h >short-help &&
+	git ls-remote -h >short-help &&
 	test_grep ! -e --head short-help &&
 	git ls-remote --heads self >actual 2>warning &&
 	test_cmp expected.branches actual &&

t/t6300-for-each-ref.sh

@@ -8,9 +8,9 @@ test_description='for-each-ref test'
 . ./test-lib.sh
 
 test_expect_success "for-each-ref does not crash with -h" '
-	test_expect_code 129 git for-each-ref -h >usage &&
+	git for-each-ref -h >usage &&
 	test_grep "[Uu]sage: git for-each-ref " usage &&
-	test_expect_code 129 nongit git for-each-ref -h >usage &&
+	nongit git for-each-ref -h >usage &&
 	test_grep "[Uu]sage: git for-each-ref " usage
 '
 

t/t6500-gc.sh

@@ -35,7 +35,7 @@ test_expect_success 'gc -h with invalid configuration' '
 		cd broken &&
 		git init &&
 		echo "[gc] pruneexpire = CORRUPT" >>.git/config &&
-		test_expect_code 129 git gc -h >usage 2>&1
+		git gc -h >usage 2>&1
 	) &&
 	test_grep "[Uu]sage" broken/usage
 '

t/t7030-verify-tag.sh

@@ -8,9 +8,9 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . "$TEST_DIRECTORY/lib-gpg.sh"
 
 test_expect_success GPG 'verify-tag does not crash with -h' '
-	test_expect_code 129 git verify-tag -h >usage &&
+	git verify-tag -h >usage &&
 	test_grep "[Uu]sage: git verify-tag " usage &&
-	test_expect_code 129 nongit git verify-tag -h >usage &&
+	nongit git verify-tag -h >usage &&
 	test_grep "[Uu]sage: git verify-tag " usage
 '
 

t/t7508-status.sh

@@ -16,7 +16,7 @@ test_expect_success 'status -h in broken repository' '
 		cd broken &&
 		git init &&
 		echo "[status] showuntrackedfiles = CORRUPT" >>.git/config &&
-		test_expect_code 129 git status -h >usage 2>&1
+		git status -h >usage 2>&1
 	) &&
 	test_grep "[Uu]sage" broken/usage
 '
@@ -28,7 +28,7 @@ test_expect_success 'commit -h in broken repository' '
 		cd broken &&
 		git init &&
 		echo "[status] showuntrackedfiles = CORRUPT" >>.git/config &&
-		test_expect_code 129 git commit -h >usage 2>&1
+		git commit -h >usage 2>&1
 	) &&
 	test_grep "[Uu]sage" broken/usage
 '

t/t7510-signed-commit.sh

@@ -9,9 +9,9 @@ GNUPGHOME_NOT_USED=$GNUPGHOME
 . "$TEST_DIRECTORY/lib-gpg.sh"
 
 test_expect_success GPG 'verify-commit does not crash with -h' '
-	test_expect_code 129 git verify-commit -h >usage &&
+	git verify-commit -h >usage &&
 	test_grep "[Uu]sage: git verify-commit " usage &&
-	test_expect_code 129 nongit git verify-commit -h >usage &&
+	nongit git verify-commit -h >usage &&
 	test_grep "[Uu]sage: git verify-commit " usage
 '
 

t/t7600-merge.sh

@@ -173,7 +173,7 @@ test_expect_success 'merge -h with invalid index' '
 		cd broken &&
 		git init &&
 		>.git/index &&
-		test_expect_code 129 git merge -h >usage
+		git merge -h >usage
 	) &&
 	test_grep "[Uu]sage: git merge" broken/usage
 '

t/t7800-difftool.sh

@@ -27,12 +27,11 @@ prompt_given ()
 }
 
 test_expect_success 'basic usage requires no repo' '
-	test_expect_code 129 git difftool -h >output &&
+	git difftool -h >output &&
 	test_grep ^usage: output &&
 	# create a ceiling directory to prevent Git from finding a repo
 	mkdir -p not/repo &&
 	test_when_finished rm -r not &&
-	test_expect_code 129 \
 	env GIT_CEILING_DIRECTORIES="$(pwd)/not" \
 	git -C not/repo difftool -h >output &&
 	test_grep ^usage: output

t/t7900-maintenance.sh

@@ -35,7 +35,7 @@ test_systemd_analyze_verify () {
 }
 
 test_expect_success 'help text' '
-	test_expect_code 129 git maintenance -h >actual &&
+	git maintenance -h >actual &&
 	test_grep "usage: git maintenance <subcommand>" actual &&
 	test_expect_code 129 git maintenance barf 2>err &&
 	test_grep "unknown subcommand: \`barf'\''" err &&

usage.c

@@ -188,7 +188,7 @@ static void show_usage_if_asked_helper(const char *err, ...)
 	va_start(params, err);
 	vfreportf(stdout, _("usage: "), err, params);
 	va_end(params);
-	exit(129);
+	exit(0);
 }
 
 void show_usage_if_asked(int ac, const char **av, const char *err)