From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Jonathan Nieder" <jrnieder@gmail.com>,
"Zero King" <l2dy@macports.org>,
"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH 8/8] t0012: test "-h" with builtins
Date: Thu, 01 Jun 2017 14:54:08 +0900 [thread overview]
Message-ID: <xmqq37bk6ynz.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqq7f0w6z7u.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Thu, 01 Jun 2017 14:42:13 +0900")
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
next prev parent reply other threads:[~2017-06-01 5:54 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-29 11:45 [Bug] setup_git_env called without repository Zero King
2017-05-29 13:01 ` Ævar Arnfjörð Bjarmason
2017-05-29 15:32 ` Jeff King
2017-05-30 5:09 ` [PATCH 0/8] consistent "-h" handling in builtins Jeff King
2017-05-30 5:11 ` [PATCH 1/8] am: handle "-h" argument earlier Jeff King
2017-05-30 5:43 ` Junio C Hamano
2017-05-30 5:12 ` [PATCH 2/8] credential: handle invalid arguments earlier Jeff King
2017-05-30 5:13 ` [PATCH 3/8] upload-archive: handle "-h" option early Jeff King
2017-05-30 5:15 ` [PATCH 4/8] remote-{ext,fd}: print usage message on invalid arguments Jeff King
2017-05-30 5:16 ` [PATCH 5/8] submodule--helper: show usage for "-h" Jeff King
2017-05-30 5:17 ` [PATCH 6/8] version: convert to parse-options Jeff King
2017-05-30 20:45 ` [PATCH 6.5?/8] version: move --build-options to a test helper Ævar Arnfjörð Bjarmason
2017-05-30 21:05 ` Jeff King
2017-05-31 15:27 ` Johannes Schindelin
2017-05-31 15:31 ` Jeff King
2017-05-31 15:46 ` Johannes Schindelin
2017-05-31 17:51 ` Ævar Arnfjörð Bjarmason
2017-05-31 21:06 ` Jeff King
2017-05-30 5:18 ` [PATCH 7/8] git: add hidden --list-builtins option Jeff King
2017-05-30 5:19 ` [PATCH 8/8] t0012: test "-h" with builtins Jeff King
2017-05-30 6:03 ` Junio C Hamano
2017-05-30 6:05 ` Jeff King
2017-05-30 6:08 ` Junio C Hamano
2017-05-30 6:15 ` Jeff King
2017-05-30 13:23 ` Junio C Hamano
2017-05-30 15:27 ` Jeff King
2017-05-30 15:44 ` Jeff King
2017-05-30 22:39 ` Junio C Hamano
2017-06-01 4:17 ` Junio C Hamano
2017-06-01 5:35 ` Jeff King
2017-06-01 5:42 ` Junio C Hamano
2017-06-01 5:54 ` Junio C Hamano [this message]
2017-06-01 6:25 ` Jeff King
2017-06-01 7:51 ` Junio C Hamano
2017-06-01 6:10 ` Jeff King
2017-06-13 23:08 ` Jonathan Nieder
2017-06-14 10:03 ` Jeff King
2017-05-30 5:52 ` [PATCH 0/8] consistent "-h" handling in builtins Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq37bk6ynz.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=l2dy@macports.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).