Subject: Re: [PATCH] parse-options: document automatic PARSE_OPT_LITERAL_ARGHELP

From: Junio C Hamano <[email-masked]>
Date: Wed, 19 Jan 2022 10:16:34 -0800

René Scharfe <[email-masked]> writes:

> The help strings for arguments are enclosed in angle brackets
> automatically.  E.g. if argh is specified as "name", "--option <name>"
> is printed, to indicate that users need to supply an actual name.  The
> flag PARSE_OPT_LITERAL_ARGHELP turns this off, so that "--option name"
> is printed instead, to indicate that the literal string needs to be
> supplied -- a rare case.
>
> This flag is enabled automatically if argh contains special characters
> like brackets.  The developer is supposed to provide any required angle
> brackets for more complicated cases.  E.g. if argh is "<start>,<end>"
> then "--option <start>,<end>" is printed.

The above does explain why we want to have this change very well,
but at least some of it would help those who are reading the comment
we touch.

> Add a comment to mention this PARSE_OPT_LITERAL_ARGHELP behavior.

But instead, the addition is only about when the flag bit is turned
on automatically.  Without understanding your

    E.g. if argh is specified as "name", "--option <name>" is
    printed, to indicate that users need to supply an actual name.

readers would not quite get from the current description "says that
argh shouldn't be enclosed in brackets" when/why it is a good idea
to add the option, I am afraid.

> Also remove the flag from option definitions for which it's inferred
> automatically.

I am not sure if this is a good move.

Because these places explicitly gave PARSE_OPT_LITERAL_ARGHELP, it
was easy to grep for them when I was trying to find an existing
practice.

Imagine, after we remove these redundant flags, we see a patch that
wants to change the set of characters that automatically flips the
flag bit on.  It is clear from the patch text why it helps one
particular OPT_STRING() or whatever the same patch adds, but how
would you make sure it will not regress _existing_ OPT_WHATEVER()
that do not use PARSE_OPT_LITERAL_ARGHELP because their argh happens
to use the character that wasn't special before?

> Signed-off-by: René Scharfe <[email-masked]>
> ---
> Somehow I feel this is not enough, but I can't pin down what's
> missing.

Let me try.

>   *   PARSE_OPT_LITERAL_ARGHELP: says that argh shouldn't be enclosed in brackets
>   *				(i.e. '<argh>') in the help message.
>   *				Useful for options with multiple parameters.
> + *				Automatically enabled if argh contains any
> + *				of the following characters: ()<>[]|
>   *   PARSE_OPT_NOCOMPLETE: by default all visible options are completable
>   *			   by git-completion.bash. This option suppresses that.
>   *   PARSE_OPT_COMP_ARG: this option forces to git-completion.bash to

PARSE_OPT_LITERAL_ARGHELP: in short-help given by "git cmd -h", the
			   "argh" member of the struct option is
			   shown in a pair of angle brackets (e.g.
			   "--option=<argh>"); this flag tells the
			   machinery not to add these brackets, to
			   give more control to the developer.  E.g.
			   "<start>,<end>" given to argh is shown as
			   "--option=<start>,<end>".

That may be a bit too much, but on the other hand, among PARSE_OPT_X
descriptions, this is the only one that needs to talk about help
text on the argument to the option.

Or we can flip it the other way, perhaps?

	Tell the machinery to give "argh" member literally in the
	short-help in "git cmd -h" output for the option.  E.g. {
	.argh = "(diff|raw)", .long_name = "show" } would give
	"--show=(diff|raw)".  Without the flag, "argh" is enclosed
	in a pair of angle brackets.

I dunno.