Re: Re* [PATCH] push: comment on a funny unbalanced option help

["René Scharfe" <l.s.r@web.de>]

Am 02.08.2018 um 17:44 schrieb Junio C Hamano:
> Subject: [PATCH] push: use PARSE_OPT_LITERAL_ARGHELP instead of unbalanced brackets
> From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Date: Thu, 02 Aug 2018 00:31:33 +0200
> 
> The option help text for the force-with-lease option to "git push"
> reads like this:
> 
>      $ git push -h 2>&1 | grep -e force-with-lease
>         --force-with-lease[=<refname>:<expect>]
> 
> which comes from having N_("refname>:<expect") as the argument help
> text in the source code, with an aparent lack of "<" and ">" at both
> ends.
> 
> It turns out that parse-options machinery takes the whole string and
> encloses it inside a pair of "<>", to make it easier for majority
> cases that uses a single token placeholder.
> 
> The help string was written in a funnily unbalanced way knowing that
> the end result would balance out, by somebody who forgot the
> presence of PARSE_OPT_LITERAL_ARGHELP, which is the escape hatch
> mechanism designed to help such a case.  We just should use the
> official escape hatch instead.
> 
> Helped-by: René Scharfe <l.s.r@web.de>

I didn't do anything for this particular patch so far?  But...

> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   builtin/push.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/builtin/push.c b/builtin/push.c
> index 1c28427d82..ef4032a9ef 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -542,9 +542,9 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>   		OPT_BIT( 0,  "porcelain", &flags, N_("machine-readable output"), TRANSPORT_PUSH_PORCELAIN),
>   		OPT_BIT('f', "force", &flags, N_("force updates"), TRANSPORT_PUSH_FORCE),
>   		{ OPTION_CALLBACK,
> -		  0, CAS_OPT_NAME, &cas, N_("refname>:<expect"),
> +		  0, CAS_OPT_NAME, &cas, N_("<refname>:<expect>"),

... shouldn't we use this opportunity to document that "expect" is
optional?

+		  0, CAS_OPT_NAME, &cas, N_("<refname>[:<expect>]"),

>   		  N_("require old value of ref to be at this value"),
> -		  PARSE_OPT_OPTARG, parseopt_push_cas_option },
> +		  PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP, parseopt_push_cas_option },
>   		{ OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules, "check|on-demand|no",
>   			N_("control recursive pushing of submodules"),
>   			PARSE_OPT_OPTARG, option_parse_recurse_submodules },
>