diff mbox series

[v3,6/6] builtin: send usage() help text to standard output

Message ID 20250116012524.1557441-7-gitster@pobox.com (mailing list archive)
State Superseded
Headers show
Series Send help text from "git cmd -h" to stdout | expand

Commit Message

Junio C Hamano Jan. 16, 2025, 1:25 a.m. UTC
Using the show_usage_and_exit_if_asked() helper we introduced
earlier, fix callers of usage() that want to show the help text when
explicitly asked by the end-user.  The help text now goes to the
standard output stream for them.

These are the bog standard "if we got only '-h', then that is a
request for help" callers.  Their

	if (argc == 2 && !strcmp(argv[1], "-h"))
		usage(message);

are simply replaced with

	show_usage_and_exit_if_asked(argc, argv, message);

With this, the built-ins tested by t0012 all send their help text to
their standard output stream, so the check in t0012 that was half
tightened earlier is now fully tightened to insist on standard error
stream being empty.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/check-ref-format.c  |  4 ++--
 builtin/diff-files.c        |  3 +--
 builtin/diff-index.c        |  3 +--
 builtin/diff-tree.c         |  3 +--
 builtin/fast-import.c       |  3 +--
 builtin/fetch-pack.c        |  3 +++
 builtin/get-tar-commit-id.c |  4 +++-
 builtin/index-pack.c        |  3 +--
 builtin/mailsplit.c         |  4 ++--
 builtin/merge-index.c       |  7 ++++++-
 builtin/merge-ours.c        |  3 +--
 builtin/merge-recursive.c   |  6 ++++++
 builtin/pack-redundant.c    |  3 +--
 builtin/remote-ext.c        |  2 ++
 builtin/remote-fd.c         |  1 +
 builtin/rev-list.c          |  3 +--
 builtin/rev-parse.c         |  2 ++
 builtin/unpack-objects.c    |  2 ++
 builtin/upload-archive.c    |  3 +--
 t/helper/test-simple-ipc.c  |  4 ++--
 t/t0012-help.sh             | 10 ++--------
 21 files changed, 42 insertions(+), 34 deletions(-)

Comments

Junio C Hamano Jan. 16, 2025, 5:30 p.m. UTC | #1
Junio C Hamano <gitster@pobox.com> writes:

> Using the show_usage_and_exit_if_asked() helper we introduced
> earlier, fix callers of usage() that want to show the help text when
> explicitly asked by the end-user.  The help text now goes to the
> standard output stream for them.
>
> These are the bog standard "if we got only '-h', then that is a
> request for help" callers.  Their
>
> 	if (argc == 2 && !strcmp(argv[1], "-h"))
> 		usage(message);
>
> are simply replaced with
>
> 	show_usage_and_exit_if_asked(argc, argv, message);
> ...

The above is a bit of a lie.  There is one strange thing I did,
which needs to be redone.

> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
> index bed2816c2d..9bd4b29c5b 100644
> --- a/builtin/fetch-pack.c
> +++ b/builtin/fetch-pack.c
> @@ -176,6 +176,9 @@ int cmd_fetch_pack(int argc,
>  			list_objects_filter_set_no_filter(&args.filter_options);
>  			continue;
>  		}
> +
> +		if (!strcmp(arg, "-h"))
> +			show_usage_and_exit_if_asked(2, &arg - 1, fetch_pack_usage);
>  		usage(fetch_pack_usage);
>  	}
>  	if (deepen_not.nr)

I think we should just call show_usage_and_exit_if_asked() before
entering the loop without changing anything else.
Junio C Hamano Jan. 16, 2025, 8:37 p.m. UTC | #2
Junio C Hamano <gitster@pobox.com> writes:

> The above is a bit of a lie.  There is one strange thing I did,
> which needs to be redone.
>
>> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
>> index bed2816c2d..9bd4b29c5b 100644
>> --- a/builtin/fetch-pack.c
>> +++ b/builtin/fetch-pack.c
>> @@ -176,6 +176,9 @@ int cmd_fetch_pack(int argc,
>>  			list_objects_filter_set_no_filter(&args.filter_options);
>>  			continue;
>>  		}
>> +
>> +		if (!strcmp(arg, "-h"))
>> +			show_usage_and_exit_if_asked(2, &arg - 1, fetch_pack_usage);
>>  		usage(fetch_pack_usage);
>>  	}
>>  	if (deepen_not.nr)
>
> I think we should just call show_usage_and_exit_if_asked() before
> entering the loop without changing anything else.

I have worked on a reroll and moved this piece to the "oddball"
pile.
diff mbox series

Patch

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index cef1ffe3ce..acc4366d85 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -64,8 +64,8 @@  int cmd_check_ref_format(int argc,
 
 	BUG_ON_NON_EMPTY_PREFIX(prefix);
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(builtin_check_ref_format_usage);
+	show_usage_and_exit_if_asked(argc, argv,
+				     builtin_check_ref_format_usage);
 
 	if (argc == 3 && !strcmp(argv[1], "--branch"))
 		return check_ref_format_branch(argv[2]);
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 604b04bb2c..f8816e0d9e 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -29,8 +29,7 @@  int cmd_diff_files(int argc,
 	int result;
 	unsigned options = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(diff_files_usage);
+	show_usage_and_exit_if_asked(argc, argv, diff_files_usage);
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index ebc824602e..a1759b8291 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -26,8 +26,7 @@  int cmd_diff_index(int argc,
 	int i;
 	int result;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(diff_cache_usage);
+	show_usage_and_exit_if_asked(argc, argv, diff_cache_usage);
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 40804e7b48..fc7c026555 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -122,8 +122,7 @@  int cmd_diff_tree(int argc,
 	int read_stdin = 0;
 	int merge_base = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(diff_tree_usage);
+	show_usage_and_exit_if_asked(argc, argv, diff_tree_usage);
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 0f86392761..e1f6bfdc73 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -3565,8 +3565,7 @@  int cmd_fast_import(int argc,
 {
 	unsigned int i;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(fast_import_usage);
+	show_usage_and_exit_if_asked(argc, argv, fast_import_usage);
 
 	reset_pack_idx_option(&pack_idx_opts);
 	git_pack_config();
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index bed2816c2d..9bd4b29c5b 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -176,6 +176,9 @@  int cmd_fetch_pack(int argc,
 			list_objects_filter_set_no_filter(&args.filter_options);
 			continue;
 		}
+
+		if (!strcmp(arg, "-h"))
+			show_usage_and_exit_if_asked(2, &arg - 1, fetch_pack_usage);
 		usage(fetch_pack_usage);
 	}
 	if (deepen_not.nr)
diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c
index 6bec0d1854..033a205ba4 100644
--- a/builtin/get-tar-commit-id.c
+++ b/builtin/get-tar-commit-id.c
@@ -13,7 +13,7 @@  static const char builtin_get_tar_commit_id_usage[] =
 #define HEADERSIZE (2 * RECORDSIZE)
 
 int cmd_get_tar_commit_id(int argc,
-			  const char **argv UNUSED,
+			  const char **argv,
 			  const char *prefix,
 			  struct repository *repo UNUSED)
 {
@@ -27,6 +27,8 @@  int cmd_get_tar_commit_id(int argc,
 
 	BUG_ON_NON_EMPTY_PREFIX(prefix);
 
+	show_usage_and_exit_if_asked(argc, argv, builtin_get_tar_commit_id_usage);
+
 	if (argc != 1)
 		usage(builtin_get_tar_commit_id_usage);
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 0b62b2589f..92ef59fad5 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1897,8 +1897,7 @@  int cmd_index_pack(int argc,
 	 */
 	fetch_if_missing = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(index_pack_usage);
+	show_usage_and_exit_if_asked(argc, argv, index_pack_usage);
 
 	disable_replace_refs();
 	fsck_options.walk = mark_link;
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 41dd304731..9e11bffdfb 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -284,6 +284,8 @@  int cmd_mailsplit(int argc,
 
 	BUG_ON_NON_EMPTY_PREFIX(prefix);
 
+	show_usage_and_exit_if_asked(argc, argv, git_mailsplit_usage);
+
 	for (argp = argv+1; *argp; argp++) {
 		const char *arg = *argp;
 
@@ -297,8 +299,6 @@  int cmd_mailsplit(int argc,
 			continue;
 		} else if ( arg[1] == 'f' ) {
 			nr = strtol(arg+2, NULL, 10);
-		} else if ( arg[1] == 'h' ) {
-			usage(git_mailsplit_usage);
 		} else if ( arg[1] == 'b' && !arg[2] ) {
 			allow_bare = 1;
 		} else if (!strcmp(arg, "--keep-cr")) {
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 342699edb7..f243a55d32 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -75,6 +75,9 @@  static void merge_all(void)
 	}
 }
 
+static const char usage_string[] =
+"git merge-index [-o] [-q] <merge-program> (-a | [--] [<filename>...])";
+
 int cmd_merge_index(int argc,
 		    const char **argv,
 		    const char *prefix UNUSED,
@@ -87,8 +90,10 @@  int cmd_merge_index(int argc,
 	 */
 	signal(SIGCHLD, SIG_DFL);
 
+	show_usage_and_exit_if_asked(argc, argv, usage_string);
+
 	if (argc < 3)
-		usage("git merge-index [-o] [-q] <merge-program> (-a | [--] [<filename>...])");
+		usage(usage_string);
 
 	repo_read_index(the_repository);
 
diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
index 3ecd9172f1..ab78fffb52 100644
--- a/builtin/merge-ours.c
+++ b/builtin/merge-ours.c
@@ -23,8 +23,7 @@  int cmd_merge_ours(int argc,
 		   const char *prefix UNUSED,
 		   struct repository *repo UNUSED)
 {
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(builtin_merge_ours_usage);
+	show_usage_and_exit_if_asked(argc, argv, builtin_merge_ours_usage);
 
 	/*
 	 * The contents of the current index becomes the tree we
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index 1dd295558b..b71c7e1c2f 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -38,6 +38,12 @@  int cmd_merge_recursive(int argc,
 	if (argv[0] && ends_with(argv[0], "-subtree"))
 		o.subtree_shift = "";
 
+	if (argc == 2 && !strcmp(argv[1], "-h")) {
+		struct strbuf msg = STRBUF_INIT;
+		strbuf_addf(&msg, builtin_merge_recursive_usage, argv[0]);
+		show_usage_and_exit_if_asked(argc, argv, msg.buf);
+	}
+
 	if (argc < 4)
 		usagef(builtin_merge_recursive_usage, argv[0]);
 
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index e046575871..ce2e1fde8d 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -595,8 +595,7 @@  int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s
 	struct strbuf idx_name = STRBUF_INIT;
 	char buf[GIT_MAX_HEXSZ + 2]; /* hex hash + \n + \0 */
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(pack_redundant_usage);
+	show_usage_and_exit_if_asked(argc, argv, pack_redundant_usage);
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index 33c8ae0fc7..f7591e7701 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -202,6 +202,8 @@  int cmd_remote_ext(int argc,
 {
 	BUG_ON_NON_EMPTY_PREFIX(prefix);
 
+	show_usage_and_exit_if_asked(argc, argv, usage_msg);
+
 	if (argc != 3)
 		usage(usage_msg);
 
diff --git a/builtin/remote-fd.c b/builtin/remote-fd.c
index ae896eda57..1a1aa65273 100644
--- a/builtin/remote-fd.c
+++ b/builtin/remote-fd.c
@@ -64,6 +64,7 @@  int cmd_remote_fd(int argc,
 
 	BUG_ON_NON_EMPTY_PREFIX(prefix);
 
+	show_usage_and_exit_if_asked(argc, argv, usage_msg);
 	if (argc != 3)
 		usage(usage_msg);
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 3196da7b2d..5a21d57c43 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -542,8 +542,7 @@  int cmd_rev_list(int argc,
 	const char *show_progress = NULL;
 	int ret = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(rev_list_usage);
+	show_usage_and_exit_if_asked(argc, argv, rev_list_usage);
 
 	git_config(git_default_config, NULL);
 	repo_init_revisions(the_repository, &revs, prefix);
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 949747a6b6..f4c2b1b000 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -713,6 +713,8 @@  int cmd_rev_parse(int argc,
 	int seen_end_of_options = 0;
 	enum format_type format = FORMAT_DEFAULT;
 
+	show_usage_and_exit_if_asked(argc, argv, builtin_rev_parse_usage);
+
 	if (argc > 1 && !strcmp("--parseopt", argv[1]))
 		return cmd_parseopt(argc - 1, argv + 1, prefix);
 
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 2197d6d933..1f1fb8e682 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -619,6 +619,8 @@  int cmd_unpack_objects(int argc,
 
 	quiet = !isatty(2);
 
+	show_usage_and_exit_if_asked(argc, argv, unpack_usage);
+
 	for (i = 1 ; i < argc; i++) {
 		const char *arg = argv[i];
 
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 3b282d41e6..42d8f67174 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -93,8 +93,7 @@  struct repository *repo UNUSED)
 
 	BUG_ON_NON_EMPTY_PREFIX(prefix);
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(upload_archive_usage);
+	show_usage_and_exit_if_asked(argc, argv, upload_archive_usage);
 
 	/*
 	 * Set up sideband subprocess.
diff --git a/t/helper/test-simple-ipc.c b/t/helper/test-simple-ipc.c
index fb5927775d..d7a41cd053 100644
--- a/t/helper/test-simple-ipc.c
+++ b/t/helper/test-simple-ipc.c
@@ -612,8 +612,8 @@  int cmd__simple_ipc(int argc, const char **argv)
 	if (argc < 2)
 		usage_with_options(simple_ipc_usage, options);
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(simple_ipc_usage, options);
+	show_usage_help_and_exit_if_asked(argc, argv,
+					  simple_ipc_usage, options);
 
 	if (argc == 2 && !strcmp(argv[1], "SUPPORTS_SIMPLE_IPC"))
 		return 0;
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 9c7ae9fd36..d3a0967e9d 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -257,14 +257,8 @@  do
 			export GIT_CEILING_DIRECTORIES &&
 			test_expect_code 129 git -C sub $builtin -h >output 2>err
 		) &&
-		if test -n "$GIT_TEST_HELP_MUST_BE_STDOUT"
-		then
-			test_must_be_empty err &&
-			test_grep usage output
-		else
-			test_grep usage output ||
-			test_grep usage err
-		fi
+		test_must_be_empty err &&
+		test_grep usage output
 	'
 done <builtins