Re: [PATCH 8/8] t0012: test "-h" with builtins

[Junio C Hamano <gitster@pobox.com>]

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

> For now, I will mix this in when queuing the whole thing in 'pu', as
> I hate to push out something that does not even work for me to the
> general public.
> 
> -- >8 --
> Subject: [PATCH] diff- and log- family: handle "git cmd -h" early
> ...

And then the check_help_option() thing may look like this.  

I am not proud of the way it "unifies" the two styles of usage
strings, obviously.

One benefit this patch has is that it makes it easier to highlight
what it does *not* touch.

    $ git grep -A2 -E -e 'a(rg)?c [!=]= 2 .*strcmp.*-h'

shows there are somewhat curious construct

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

left in the code.  Upon closer inspection, they all happen to be
doing the right thing for their current set of options and
arguments, but they are somewhat ugly.


 builtin/am.c               |  3 +--
 builtin/branch.c           |  3 +--
 builtin/check-ref-format.c |  3 +--
 builtin/checkout-index.c   |  7 ++++---
 builtin/commit.c           |  6 ++----
 builtin/diff-files.c       |  3 +--
 builtin/diff-index.c       |  3 +--
 builtin/diff-tree.c        |  3 +--
 builtin/gc.c               |  3 +--
 builtin/index-pack.c       |  3 +--
 builtin/ls-files.c         |  3 +--
 builtin/merge-ours.c       |  3 +--
 builtin/merge.c            |  3 +--
 builtin/pack-redundant.c   |  3 +--
 builtin/rev-list.c         |  3 +--
 builtin/update-index.c     |  3 +--
 builtin/upload-archive.c   |  3 +--
 fast-import.c              |  3 +--
 git-compat-util.h          |  3 +++
 usage.c                    | 11 +++++++++++
 20 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 8881d73615..12b7298907 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2307,8 +2307,7 @@ int cmd_am(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(usage, options);
+	check_help_option(argc, argv, usage, options);
 
 	git_config(git_am_config, NULL);
 
diff --git a/builtin/branch.c b/builtin/branch.c
index 83fcda43dc..8c4465f0e4 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -599,8 +599,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	filter.kind = FILTER_REFS_BRANCHES;
 	filter.abbrev = -1;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_branch_usage, options);
+	check_help_option(argc, argv, builtin_branch_usage, options);
 
 	git_config(git_branch_config, NULL);
 
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index eac499450f..aab5776dd5 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -55,8 +55,7 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 	int flags = 0;
 	const char *refname;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(builtin_check_ref_format_usage);
+	check_help_option(argc, argv, builtin_check_ref_format_usage, NULL);
 
 	if (argc == 3 && !strcmp(argv[1], "--branch"))
 		return check_ref_format_branch(argv[2]);
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 07631d0c9c..8dd28ae8ba 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -179,9 +179,10 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_checkout_index_usage,
-				   builtin_checkout_index_options);
+	check_help_option(argc, argv,
+			  builtin_checkout_index_usage,
+			  builtin_checkout_index_options);
+
 	git_config(git_default_config, NULL);
 	prefix_length = prefix ? strlen(prefix) : 0;
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 66c9ac587b..05c2f61e33 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1376,8 +1376,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_status_usage, builtin_status_options);
+	check_help_option(argc, argv, builtin_status_usage, builtin_status_options);
 
 	status_init_config(&s, git_status_config);
 	argc = parse_options(argc, argv, prefix,
@@ -1661,8 +1660,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_commit_usage, builtin_commit_options);
+	check_help_option(argc, argv, builtin_commit_usage, builtin_commit_options);
 
 	status_init_config(&s, git_commit_config);
 	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index c97069a714..ff52edb46c 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -20,8 +20,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	int result;
 	unsigned options = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(diff_files_usage);
+	check_help_option(argc, argv, diff_files_usage, NULL);
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	init_revisions(&rev, prefix);
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index d59bf6cf5f..518482850e 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -17,8 +17,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	int i;
 	int result;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(diff_cache_usage);
+	check_help_option(argc, argv, diff_cache_usage, NULL);
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	init_revisions(&rev, prefix);
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index a4e7398b4b..aa12c02203 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -105,8 +105,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	struct setup_revision_opt s_r_opt;
 	int read_stdin = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(diff_tree_usage);
+	check_help_option(argc, argv, diff_tree_usage, NULL);
 
 	git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 	init_revisions(opt, prefix);
diff --git a/builtin/gc.c b/builtin/gc.c
index f484eda43c..b1a6163347 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -363,8 +363,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_gc_usage, builtin_gc_options);
+	check_help_option(argc, argv, builtin_gc_usage, builtin_gc_options);
 
 	argv_array_pushl(&pack_refs_cmd, "pack-refs", "--all", "--prune", NULL);
 	argv_array_pushl(&reflog, "reflog", "expire", "--all", NULL);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 04b9dcaf0f..2be24276d6 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1640,8 +1640,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	unsigned foreign_nr = 1;	/* zero is a "good" value, assume bad */
 	int report_end_of_input = 0;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(index_pack_usage);
+	check_help_option(argc, argv, index_pack_usage, NULL);
 
 	check_replace_refs = 0;
 	fsck_options.walk = mark_link;
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index b376afc312..6d5334aae5 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -587,8 +587,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 		OPT_END()
 	};
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(ls_files_usage, builtin_ls_files_options);
+	check_help_option(argc, argv, ls_files_usage, builtin_ls_files_options);
 
 	memset(&dir, 0, sizeof(dir));
 	prefix = cmd_prefix;
diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
index 684411694f..52be2fa2f4 100644
--- a/builtin/merge-ours.c
+++ b/builtin/merge-ours.c
@@ -20,8 +20,7 @@ static const char *diff_index_args[] = {
 
 int cmd_merge_ours(int argc, const char **argv, const char *prefix)
 {
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(builtin_merge_ours_usage);
+	check_help_option(argc, argv, builtin_merge_ours_usage, NULL);
 
 	/*
 	 * We need to exit with 2 if the index does not match our HEAD tree,
diff --git a/builtin/merge.c b/builtin/merge.c
index eab03a026d..446eb0f3fb 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1108,8 +1108,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	void *branch_to_free;
 	int orig_argc = argc;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(builtin_merge_usage, builtin_merge_options);
+	check_help_option(argc, argv, builtin_merge_usage, builtin_merge_options);
 
 	/*
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index cb1df1c761..80603b9b47 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -601,8 +601,7 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
 	unsigned char *sha1;
 	char buf[42]; /* 40 byte sha1 + \n + \0 */
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(pack_redundant_usage);
+	check_help_option(argc, argv, pack_redundant_usage, NULL);
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index b250c515b1..ce6acf18c7 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -277,8 +277,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	int use_bitmap_index = 0;
 	const char *show_progress = NULL;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(rev_list_usage);
+	check_help_option(argc, argv, rev_list_usage, NULL);
 
 	git_config(git_default_config, NULL);
 	init_revisions(&revs, prefix);
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 32fd977b43..e6df968056 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1009,8 +1009,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage_with_options(update_index_usage, options);
+	check_help_option(argc, argv, update_index_usage, options);
 
 	git_config(git_default_config, NULL);
 
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 84532ae9a9..0e097969db 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -76,8 +76,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
 	struct child_process writer = { argv };
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(upload_archive_usage);
+	check_help_option(argc, argv, upload_archive_usage, NULL);
 
 	/*
 	 * Set up sideband subprocess.
diff --git a/fast-import.c b/fast-import.c
index 9a22fc92c0..fd7a4fb472 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3447,8 +3447,7 @@ int cmd_main(int argc, const char **argv)
 {
 	unsigned int i;
 
-	if (argc == 2 && !strcmp(argv[1], "-h"))
-		usage(fast_import_usage);
+	check_help_option(argc, argv, fast_import_usage, NULL);
 
 	setup_git_directory();
 	reset_pack_idx_option(&pack_idx_opts);
diff --git a/git-compat-util.h b/git-compat-util.h
index 22b756ed51..c30b6b9a72 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -418,6 +418,9 @@ extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2
 extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
 extern void warning_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
 
+struct option;
+extern void check_help_option(int, const char **, const void *, struct option *);
+
 #ifndef NO_OPENSSL
 #ifdef APPLE_COMMON_CRYPTO
 #include "compat/apple-common-crypto.h"
diff --git a/usage.c b/usage.c
index 2f87ca69a8..007d732094 100644
--- a/usage.c
+++ b/usage.c
@@ -5,6 +5,7 @@
  */
 #include "git-compat-util.h"
 #include "cache.h"
+#include "parse-options.h"
 
 void vreportf(const char *prefix, const char *err, va_list params)
 {
@@ -225,3 +226,13 @@ NORETURN void BUG(const char *fmt, ...)
 	va_end(ap);
 }
 #endif
+
+void check_help_option(int argc, const char **argv, const void *help, struct option *opt)
+{
+	if (argc == 2 && !strcmp(argv[1], "-h")) {
+		if (opt)
+			usage_with_options((const char * const *)help, opt);
+		else
+			usage((const char *)help);
+	}
+}
-- 
2.13.0-513-g1c0955652f