Re: [PATCH] cli: add -v and -h shorthands

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 30, 2022, 17:17 UTC
Message-ID: <220330.86r16jnydf.gmgdl@evledraar.gmail.com>
In-Reply-To: <20220330170059.5075-1-garrit@slashdev.space>

On Wed, Mar 30 2022, Garrit Franke wrote:

> If "-v" or "-h" is passed as a flag, they will be interpreted as
> a "version" or "help" command respectively.

They won't, try e.g. "./git -h" before/after this change. On a second reading I see that there's an implied "change the behavior to ..." there.

But it would really help if the commit message discussed what we did before, what we do now, and why the change is safe/OK.

I *think* we just emit the message about an unknown option before, and this makes it synonymous with --verbose/verbose and --help/(but not "help"), but saying so would be useful :)

> Since this is my first patch for this project, I'm very happy to take
> your feedback!

Welcome to git development!

Show 20 quoted lines

>  Documentation/git.txt |  4 +++-
>  git.c                 | 15 ++++++++++++---
>  po/bg.po              |  4 ++--
>  po/ca.po              |  4 ++--
>  po/de.po              |  4 ++--
>  po/el.po              |  4 ++--
>  po/es.po              |  4 ++--
>  po/fr.po              |  4 ++--
>  po/git.pot            |  2 +-
>  po/id.po              |  4 ++--
>  po/it.po              |  4 ++--
>  po/ko.po              |  4 ++--
>  po/pl.po              |  4 ++--
>  po/pt_PT.po           |  2 +-
>  po/ru.po              |  4 ++--
>  po/sv.po              |  4 ++--
>  po/tr.po              |  4 ++--
>  po/vi.po              |  4 ++--
>  po/zh_CN.po           |  4 ++--
>  po/zh_TW.po           |  4 ++--

This *.po stuff is a well-meaning change, but please leave this out of the patch.

The translation files are updated by a process that the translation teams(s) use, see po/README.md. I think changing this from under them is probably more disruptive than not.

Show 5 quoted lines

> -		if (!strcmp(cmd, "--help") || !strcmp(cmd, "--version"))
> +		if (!strcmp(cmd, "-h") ||
> +			!strcmp(cmd, "--help") ||
> +			!strcmp(cmd, "-v") ||
> +			!strcmp(cmd, "--version"))

nit: minimize the diff here perhaps by keeping the existing line, and doing -h or -v on a new line? Your indentation here is also off.

Show 12 quoted lines

> @@ -893,7 +896,13 @@ 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]);
> +		}

Don't use {} braces here, see CodingGudielines.

FWIW I have an existing branch (unsubmitted) at https://github.com/avar/git/tree/avar/parse-options-h-3 where I added some tests for "git cmd -h" behavior, which seems to pass with this change (not unexpected, as this is for the top-level command).