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

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

Am 19.01.22 um 19:16 schrieb Junio C Hamano:
> René Scharfe <l.s.r@web.de> 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?

Building a multi-line regex or going through the output of --help-all of
all commands or adding a throwaway internal option to just print argh
would certainly be much harder.

Reducing the set of special characters would be part of a change to the
notation for describing options.  Perhaps we'd want to no longer use
grouping and thus get rid of parentheses.  That would require updating
all affected manpages and usage strings as well -- quite a big effort,
regardless of PARSE_OPT_LITERAL_ARGHELP's grepability.

Extending the set, e.g. to give special meaning to curly brackets, can't
rely on the explicit flag; all argh strings need to be examined to check
whether they become special.

>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>> 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.

Now that I read the whole comment, I think the right place to introduce
the automatic brackets is the description of argh some lines up.

--- >8 ---
Subject: [PATCH 5/5] parse-options: document bracketing of argh

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 parse-options.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/parse-options.h b/parse-options.h
index e22846d3b7..88d589d159 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -85,6 +85,11 @@ typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
  *   token to explain the kind of argument this option wants. Does not
  *   begin in capital letter, and does not end with a full stop.
  *   Should be wrapped by N_() for translation.
+ *   Is automatically enclosed in brackets when printed, unless it
+ *   contains any of the following characters: ()<>[]|
+ *   E.g. "name" is shown as "<name>" to indicate that a name value
+ *   needs to be supplied, not the literal string "name", but
+ *   "<start>,<end>" and "(this|that)" are printed verbatim.
  *
  * `help`::
  *   the short help associated to what the option does.
--
2.34.1