git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 1, 2017, 05:54 UTC
Message-ID
<xmqq37bk6ynz.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<xmqq7f0w6z7u.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 7 quoted lines
> 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
Previous: Junio C HamanoNext: Jeff King
Message 32 of 38 in “[Bug] setup_git_env called without repository”
  1. Zero KingMay 29, 2017
  2. Ævar Arnfjörð BjarmasonMay 29, 2017
  3. Jeff KingMay 29, 2017
  4. 0/8 consistent "-h" handling in builtinsJeff King, May 30, 2017
  5. 1/8 am: handle "-h" argument earlierJeff King, May 30, 2017
  6. Junio C HamanoMay 30, 2017
  7. 2/8 credential: handle invalid arguments earlierJeff King, May 30, 2017
  8. 3/8 upload-archive: handle "-h" option earlyJeff King, May 30, 2017
  9. 4/8 remote-{ext,fd}: print usage message on invalid argumentsJeff King, May 30, 2017
  10. 5/8 submodule--helper: show usage for "-h"Jeff King, May 30, 2017
  11. 6/8 version: convert to parse-optionsJeff King, May 30, 2017
  12. version: move --build-options to a test helperÆvar Arnfjörð Bjarmason, May 30, 2017
  13. Jeff KingMay 30, 2017
  14. Johannes SchindelinMay 31, 2017
  15. Jeff KingMay 31, 2017
  16. Johannes SchindelinMay 31, 2017
  17. Ævar Arnfjörð BjarmasonMay 31, 2017
  18. Jeff KingMay 31, 2017
  19. 7/8 git: add hidden --list-builtins optionJeff King, May 30, 2017
  20. 8/8 t0012: test "-h" with builtinsJeff King, May 30, 2017
  21. Junio C HamanoMay 30, 2017
  22. Jeff KingMay 30, 2017
  23. Junio C HamanoMay 30, 2017
  24. Jeff KingMay 30, 2017
  25. Junio C HamanoMay 30, 2017
  26. Jeff KingMay 30, 2017
  27. Jeff KingMay 30, 2017
  28. Junio C HamanoMay 30, 2017
  29. Junio C HamanoJun 1, 2017
  30. Jeff KingJun 1, 2017
  31. Junio C HamanoJun 1, 2017
  32. Junio C HamanoJun 1, 2017
  33. Jeff KingJun 1, 2017
  34. Junio C HamanoJun 1, 2017
  35. Jeff KingJun 1, 2017
  36. Jonathan NiederJun 13, 2017
  37. Jeff KingJun 14, 2017
  38. Junio C HamanoMay 30, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.