Re: [PATCH v3] help: make option --help open man pages only for Git commands
From: Junio C Hamano <hidden>
Date: 2016-08-16 17:27:43
Ralf Thielow [off-list ref] writes:
quoted hunk
builtin/help.c | 30 +++++++++++++++++++++++------- git.c | 15 ++++++++++++++- t/t0012-help.sh | 15 +++++++++++++++ 3 files changed, 52 insertions(+), 8 deletions(-) create mode 100755 t/t0012-help.shdiff --git a/builtin/help.c b/builtin/help.c index 8848013..76f07c7 100644 --- a/builtin/help.c +++ b/builtin/help.c@@ -37,7 +37,9 @@ static int show_all = 0; static int show_guides = 0; static unsigned int colopts; static enum help_format help_format = HELP_FORMAT_NONE; +static int swapped = 0;
This is not the first offender (show_guides above does so, too), but please do not initialize static explicitly to 0 or NULL.
quoted hunk
static struct option builtin_help_options[] = { + OPT_BOOL('s', "swapped", &swapped, "mark as being called by <cmd> --help"), OPT_BOOL('a', "all", &show_all, N_("print all available commands")), OPT_BOOL('g', "guides", &show_guides, N_("print list of useful guides")), OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),@@ -433,10 +435,29 @@ static void list_common_guides_help(void) putchar('\n'); } +static const char* check_git_cmd(const char* cmd)
Style: "static const char *check_git_cmd(const char *cmd)". The asterisk that turns the base type to a pointer to the base type sticks to the identifier, not to the type.
+{
+ char *alias;
+
+ if (is_git_command(cmd))
+ return cmd;
+
+ alias = alias_lookup(cmd);
+ if (alias) {
+ printf_ln(_("`git %s' is aliased to `%s'"), cmd, alias);
+ free(alias);
+ exit(0);
+ }
+
+ if (swapped)
+ return help_unknown_cmd(cmd);I am guilty of suggesting "swapped"; even if we are going to mark this as OPT_HIDDEN, I think we should be able to think of a better name. I think the meaning of this boolean is "we know that this is not a guide and is meant to be a command.", and I hope we can come up with a name that concisely expresses that (e.g. "--not-a-guide", "--must-be-a-command").
quoted hunk
+ return cmd; +} + int cmd_help(int argc, const char **argv, const char *prefix) { int nongit; - char *alias; enum help_format parsed_help_format; argc = parse_options(argc, argv, prefix, builtin_help_options,@@ -476,12 +497,7 @@ int cmd_help(int argc, const char **argv, const char *prefix) if (help_format == HELP_FORMAT_NONE) help_format = parse_help_format(DEFAULT_HELP_FORMAT); - alias = alias_lookup(argv[0]); - if (alias && !is_git_command(argv[0])) { - printf_ln(_("`git %s' is aliased to `%s'"), argv[0], alias); - free(alias); - return 0; - } + argv[0] = check_git_cmd(argv[0]); switch (help_format) { case HELP_FORMAT_NONE:diff --git a/git.c b/git.c index 0f1937f..71ea983 100644 --- a/git.c +++ b/git.c@@ -528,10 +528,23 @@ static void handle_builtin(int argc, const char **argv) strip_extension(argv); cmd = argv[0]; - /* Turn "git cmd --help" into "git help cmd" */ + /* Turn "git cmd --help" into "git help --swapped cmd" */ if (argc > 1 && !strcmp(argv[1], "--help")) { + struct argv_array args; + int i; + argv[1] = argv[0]; argv[0] = cmd = "help"; + + argv_array_init(&args); + for (i = 0; i < argc; i++) { + argv_array_push(&args, argv[i]); + if (i == 0)
It is more idiomatic to say if (!i) around here.
+ argv_array_push(&args, "--swapped");
+ } + + argc++; + argv = argv_array_detach(&args); } builtin = get_builtin(cmd);
The code does this after it:
if (builtin)
exit(run_builtin(...));
and returns. If we didn't get builtin, we risk leaking args.argv
here, but we assume argv[0] = cmd = "help" is always a builtin,
which I think is a safe assumption, so the code is OK. Static
checkers that are only half intelligent may yell at you for not
releasing the resources, though.
quoted hunk
diff --git a/t/t0012-help.sh b/t/t0012-help.sh new file mode 100755 index 0000000..6f700b1 --- /dev/null +++ b/t/t0012-help.sh@@ -0,0 +1,15 @@ +#!/bin/sh + +test_description='help' + +. ./test-lib.sh + +test_expect_success "pass --help to common guide" " + cat <<-EOF >expected && + git: 'revisions' is not a git command. See 'git --help'. + EOF + (git revisions --help 2>actual || true) && + test_i18ncmp expected actual +" + +test_done