Garrit Franke <garrit@slashdev.space> writes: > On 31.03.22 02:07, Ævar Arnfjörð Bjarmason wrote: > >> I think this is a good trade-off in this case. I.e. -v and -h are >> commonly understood. > > An interesting observation I just made is that curl [0] uses both > "--verbose" and "--version" on the top level [1][2] including > shorthands. "-v" corresponds to "verbose", "-V" corresponds to > "version. > > Not that I'm a fan of this clutter, but it's a possible path to go > down if we actually needed a second shorthand using this letter. Do you mean you want to use "-V" for version, instead of the "-v" used in the patch, so that "-v" can be left for "--verbose"? I am not sure consistency with whom we are aiming for anymore with that mixed to the proposal X-<. > const char git_usage_string[] = > - N_("git [--version] [--help] [-C <path>] [-c <name>=<value>]\n" > + N_("git [-v | --version] [-h | --help] [-C <path>] [-c <name>=<value>]\n" > " [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n" > " [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--bare]\n" > " [--git-dir=<path>] [--work-tree=<path>] [--namespace=<name>]\n" This is an improvement in that the first line that used to be unusually shorter now becomes comparable length to other lines ;-) > @@ -146,7 +146,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) > * commands can be written with "--" prepended > * to make them look like flags. > */ > - if (!strcmp(cmd, "--help") || !strcmp(cmd, "--version")) > + if (!strcmp(cmd, "--help") || !strcmp(cmd, "--version") || > + !strcmp(cmd, "-h") || !strcmp(cmd, "-v")) > break; OK. > @@ -893,7 +894,12 @@ int cmd_main(int argc, const char **argv) > handle_options(&argv, &argc, NULL); > if (argc > 0) { > /* translate --help and --version into commands */ > - skip_prefix(argv[0], "--", &argv[0]); > + if (!strcmp("-v", argv[0])) > + argv[0] = "version"; > + else if (!strcmp("-h", argv[0])) > + argv[0] = "help"; > + else > + skip_prefix(argv[0], "--", &argv[0]); I find this unnecessarily hard to read. (Side note) A tip for reviewers. Be suspicious of any change that adds new things _in front_ of existing sequence. Question if there is a good justification for it. If there isn't, see if it makes it better if you instead append the new stuff to existing sequence. If neither results in satisfying code, perhaps it is good time to totally rewrite it to make both existing and new stuff fit in the flow. The original was in a sense a bit sloppy to strip "--" from anything that begins with "--", when we only care about "--verbose" and "--help" and nothing else (granted, handle_options() would barf when argv[0] begins with "--" and is not one of these two, so the sloppiness does not make a difference in practice). If we accept the same kind of sloppiness, perhaps if (skip_prefix(argv[0], "--", &argv[0])) ; /* removed "--" from "--help" and "--version" */ else if (argv[0][0] == '-') argv[0] = argv[0][1] == 'v' ? "version" : "help"; would make the new side match the existing one better, but I would not necessarily recommend it. We may want to be a bit more explicit and readable, by spelling out the expectation, i.e. if (!strcmp("--version", argv[0]) || !strcmp("-v", argv[0])) argv[0] = "version"; else if (!strcmp("--help", argv[0]) || !strcmp("-h", argv[0])) argv[0] = "help"; This makes it clear that these two pseudo-commands, spelled with a dash in front and stand for other commands, are the only thing we care about and what their accepted spelling are. Hmm?