Re: [PATCH] branch,checkout: fix --track usage strings

From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Josh Steadmon <steadmon@google.com>, git@vger.kernel.org
Subject: Re: [PATCH] branch,checkout: fix --track usage strings
Date: Fri, 21 Jan 2022 13:12:15 -0800	[thread overview]
Message-ID: <xmqqilucg5e8.fsf@gitster.g> (raw)
In-Reply-To: <220121.86ee51l3jd.gmgdl@evledraar.gmail.com> (=?utf-8?B?IsOG?= =?utf-8?B?dmFyIEFybmZqw7Zyw7A=?= Bjarmason"'s message of "Fri, 21 Jan 2022 12:27:33 +0100")

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> Also, with the sh completion if you do "git add --chm<TAB>" it expands
> it to "git add --chmod=", i.e. the cursor is left after the "=" that's
> not shown in the "git add -h". So always including it would be
> consistent with that.
>     
>     diff --git a/parse-options.c b/parse-options.c
>     index a8283037be9..75c345bb738 100644
>     --- a/parse-options.c
>     +++ b/parse-options.c
>     @@ -916,7 +916,7 @@ static int usage_argh(const struct option *opts, FILE *outfile)
>                     else
>                             s = literal ? "[%s]" : "[<%s>]";
>             else
>     -               s = literal ? " %s" : " <%s>";
>     +               s = literal ? "=%s" : "=<%s>";

If the option has a long name, I think it is a good change.  I do
not offhand know if it is a good change for a short option, though.

    $ git diff -B=20/60
    error: break-rewrites expects <n>/<m> form
    $ git diff -B 20/60
    fatal: ambiguous argument '20/60': unknown revision or path not in the working tree.
    $ git diff -B20/60

gitcli.txt has this (I didn't check with the parse-options
implementation, though):

 * when a command-line option takes an argument, use the 'stuck' form.  In
   other words, write `git foo -oArg` instead of `git foo -o Arg` for short
   options, and `git foo --long-opt=Arg` instead of `git foo --long-opt Arg`
   for long options.  An option that takes optional option-argument must be
   written in the 'stuck' form.

So probably you'd need the same "show differently depending on which
between short and long we will show" on this side of the if/else.

	else {
		if (opts->long_name)
			s = literal ? "=%s" : "=<%s>";
		else
			s = literal ? "%s" : "<%s>";
	}

perhaps?