Re: Re* [PATCH] push: comment on a funny unbalanced option help
- From
René Scharfe <l.s.r@web.de>
- Date
- Aug 2, 2018, 15:59 UTC
- Message-ID
- <b72e5848-a51d-a62d-2a3c-36ea6b8582d0@web.de>
- In-Reply-To
- <xmqq36vw4vu5.fsf_-_@gitster-ct.c.googlers.com>
Am 02.08.2018 um 17:44 schrieb Junio C Hamano:
Show 25 quoted lines
> 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...
Show 15 quoted lines
> 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>]"),Show 7 quoted lines
> 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 },
>