Sorry for being late in responding. It's been busy days. 2016-08-18 21:51 GMT+02:00 Junio C Hamano <gitster@xxxxxxxxx>: > Ralf Thielow <ralf.thielow@xxxxxxxxx> writes: > > The same comment applies to 1/2, too, in that the word "command" > will be interpreted differently by different people. For example, > "git co --help" and "git help co" would work, with or without 1/2 in > place when you have "[alias] co = checkout", so we are calling "Git > subcommands that we ship, custom commands 'git-$foo' the users have > in their $PATH, and aliases the users create" collectively "command". > > As long as the reader understands that definition, both the log > messages of 1/2 and 2/2 _and_ the updated description for "git help" > we have in 1/2 are all very clear. I do not care too much about the > commit log message, but we may want to think about the documentation > a bit more. > > Here is what 1/2 adds to "git help" documentation: > > +Note that `git --help ...` is almost identical to `git help ...` because > +the former is internally converted into the latter with option --command-only > +being added. > > To display the linkgit:git[1] man page, use `git help git`. > > @@ -43,6 +44,10 @@ OPTIONS > Prints all the available commands on the standard output. This > option overrides any given command or guide name. > > +-c:: > +--command-only:: > + Display help information only for commands. > + > > First, I do not think a short form is unnecessary; the users are not > expected to use that form, once they started typing "git help...". > If we flip the polarity and call it --exclude-guides or something, > would it make it less ambiguous? > Sure. Since "command" is understood as both Git command and guide in this context, the name --exclude-guides would describe the behaviour of that option less ambiguous. I'll rename it. >> This breaks "git <concept> --help" while "git help <concept>" still works. > > I wouldn't call that a breakage; "git everyday --help" shouldn't > have worked in the first place. It did something useful merely by > accident ;-). > OK, I'll call it "doesn't work anymore". >> diff --git a/git.c b/git.c >> index 0f1937f..2cd2e06 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 --command-only 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) >> + argv_array_push(&args, "--command-only"); >> + } >> + >> + 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. I wonder if it is worth doing > > static void handle_builtin(int argc, const char **argv) > { > struct argv_array args = ARGV_ARRAY_INIT; > ... > if (argc > 1 && !strcmp(argv[1], "--help")) { > ... > argv = args.argv; > } > builtin = get_builtin(cmd); > if (builtin) > exit(run_builtin(...)); > argv_array_clear(&args); > } > > to help unconfuse them. > I'll do it this way. Thanks! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html