Am 21.07.23 um 16:37 schrieb Junio C Hamano:
> René Scharfe <l.s.r@xxxxxx> writes:
>
>> Overall I get the impression that having the negative form enabled by
>> default was not a good idea. For boolean options it makes sense, for
>> options with arguments perhaps as well, but for OPT_SET_INT we would
>> have less confusion if the negated form was opt-in.
>>
>> To make it easier discoverable we could let the short help include
>> the optional "no-" part, which would look like this:
>>
>> usage: git ls-tree [<options>] <tree-ish> [<path>...]
>>
>> -d only show trees
>> -r recurse into subtrees
>> -t show trees when recursing
>> -z terminate entries with NUL byte
>> -l, --long include object size
>> --name-only list only filenames
>> --name-status list only filenames
>> --object-only list only objects
>> --[no-]full-name use full path names
>> --[no-]full-tree list entire tree; not just current directory (implies --full-name)
>> --format <format> format to use for the output
>> --[no-]abbrev[=<n>] use <n> digits to display object names
>>
>> Thoughts?
>
> I like the "optional no- accepted" markings, but I suspect there may
> be quite a lot of fallouts.
Some test expectations in t0040 and t1502 would have to be adjusted.
This reveals, by the way, that t1502 doesn't yet exercise the "!" flag
of "git rev-parse --parseopt" that turns on PARSE_OPT_NONEG. I find
the "-h, --[no-]help" option strangely amusing..
--- >8 ----
Subject: [PATCH] parse-options: show negatability of options in short help
Add a "[no-]" prefix to options without the flag PARSE_OPT_NONEG to
document the fact that you can negate them.
This looks a bit strange for options that already start with "no-", e.g.
for the option --no-name of git show-branch:
--[no-]no-name suppress naming strings
You can actually use --no-no-name as an alias of --name, so the short
help is not wrong. If we strip off any of the "no-"s, we lose either
the ability to see if the remaining one belongs to the documented
variant or to see if it can be negated.
Signed-off-by: René Scharfe <l.s.r@xxxxxx>
---
parse-options.c | 10 ++++-
t/t0040-parse-options.sh | 44 ++++++++++---------
t/t1502-rev-parse-parseopt.sh | 80 ++++++++++++++++++++---------------
3 files changed, 77 insertions(+), 57 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index f8a155ee13..6323ca191d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1136,8 +1136,14 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
}
if (opts->long_name && opts->short_name)
pos += fprintf(outfile, ", ");
- if (opts->long_name)
- pos += fprintf(outfile, "--%s", opts->long_name);
+ if (opts->long_name) {
+ const char *long_name = opts->long_name;
+ if (opts->flags & PARSE_OPT_NONEG)
+ pos += fprintf(outfile, "--%s", long_name);
+ else
+ pos += fprintf(outfile, "--[no-]%s", long_name);
+ }
+
if (opts->type == OPTION_NUMBER)
pos += utf8_fprintf(outfile, _("-NUM"));
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 7d7ecfd571..f39758d2ef 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -13,29 +13,32 @@ usage: test-tool parse-options <options>
A helper function for the parse-options API.
- --yes get a boolean
- -D, --no-doubt begins with 'no-'
+ --[no-]yes get a boolean
+ -D, --[no-]no-doubt begins with 'no-'
-B, --no-fear be brave
- -b, --boolean increment by one
- -4, --or4 bitwise-or boolean with ...0100
- --neg-or4 same as --no-or4
+ -b, --[no-]boolean increment by one
+ -4, --[no-]or4 bitwise-or boolean with ...0100
+ --[no-]neg-or4 same as --no-or4
- -i, --integer <n> get a integer
+ -i, --[no-]integer <n>
+ get a integer
-j <n> get a integer, too
-m, --magnitude <n> get a magnitude
- --set23 set integer to 23
+ --[no-]set23 set integer to 23
--mode1 set integer to 1 (cmdmode option)
--mode2 set integer to 2 (cmdmode option)
- -L, --length <str> get length of <str>
- -F, --file <file> set file to <file>
+ -L, --[no-]length <str>
+ get length of <str>
+ -F, --[no-]file <file>
+ set file to <file>
String options
- -s, --string <string>
+ -s, --[no-]string <string>
get a string
- --string2 <str> get another string
- --st <st> get another string (pervert ordering)
+ --[no-]string2 <str> get another string
+ --[no-]st <st> get another string (pervert ordering)
-o <str> get another string
- --list <str> add str to list
+ --[no-]list <str> add str to list
Magic arguments
-NUM set integer to NUM
@@ -44,16 +47,17 @@ Magic arguments
--no-ambiguous negative ambiguity
Standard options
- --abbrev[=<n>] use <n> digits to display object names
- -v, --verbose be verbose
- -n, --dry-run dry run
- -q, --quiet be quiet
- --expect <string> expected output in the variable dump
+ --[no-]abbrev[=<n>] use <n> digits to display object names
+ -v, --[no-]verbose be verbose
+ -n, --[no-]dry-run dry run
+ -q, --[no-]quiet be quiet
+ --[no-]expect <string>
+ expected output in the variable dump
Alias
- -A, --alias-source <string>
+ -A, --[no-]alias-source <string>
get a string
- -Z, --alias-target <string>
+ -Z, --[no-]alias-target <string>
alias of --alias-source
EOF
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index dd811b7fb4..0a67e2dd4f 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -64,33 +64,38 @@ test_expect_success 'test --parseopt help output' '
|
| some-command does foo and bar!
|
-| -h, --help show the help
-| --foo some nifty option --foo
-| --bar ... some cool option --bar with an argument
-| -b, --baz a short and long option
+| -h, --[no-]help show the help
+| --[no-]foo some nifty option --foo
+| --[no-]bar ... some cool option --bar with an argument
+| -b, --[no-]baz a short and long option
|
|An option group Header
| -C[...] option C with an optional argument
-| -d, --data[=...] short and long option with an optional argument
+| -d, --[no-]data[=...]
+| short and long option with an optional argument
|
|Argument hints
| -B <arg> short option required argument
-| --bar2 <arg> long option required argument
-| -e, --fuz <with-space>
+| --[no-]bar2 <arg> long option required argument
+| -e, --[no-]fuz <with-space>
| short and long option required argument
| -s[<some>] short option optional argument
-| --long[=<data>] long option optional argument
-| -g, --fluf[=<path>] short and long option optional argument
-| --longest <very-long-argument-hint>
+| --[no-]long[=<data>] long option optional argument
+| -g, --[no-]fluf[=<path>]
+| short and long option optional argument
+| --[no-]longest <very-long-argument-hint>
| a very long argument hint
-| --pair <key=value> with an equals sign in the hint
-| --aswitch help te=t contains? fl*g characters!`
-| --bswitch <hint> hint has trailing tab character
-| --cswitch switch has trailing tab character
-| --short-hint <a> with a one symbol hint
+| --[no-]pair <key=value>
+| with an equals sign in the hint
+| --[no-]aswitch help te=t contains? fl*g characters!`
+| --[no-]bswitch <hint>
+| hint has trailing tab character
+| --[no-]cswitch switch has trailing tab character
+| --[no-]short-hint <a>
+| with a one symbol hint
|
|Extras
-| --extra1 line above used to cause a segfault but no longer does
+| --[no-]extra1 line above used to cause a segfault but no longer does
|
|EOF
END_EXPECT
@@ -131,7 +136,7 @@ test_expect_success 'test --parseopt help-all output hidden switches' '
|
| some-command does foo and bar!
|
-| --hidden1 A hidden switch
+| --[no-]hidden1 A hidden switch
|
|EOF
END_EXPECT
@@ -146,33 +151,38 @@ test_expect_success 'test --parseopt invalid switch help output' '
|
| some-command does foo and bar!
|
-| -h, --help show the help
-| --foo some nifty option --foo
-| --bar ... some cool option --bar with an argument
-| -b, --baz a short and long option
+| -h, --[no-]help show the help
+| --[no-]foo some nifty option --foo
+| --[no-]bar ... some cool option --bar with an argument
+| -b, --[no-]baz a short and long option
|
|An option group Header
| -C[...] option C with an optional argument
-| -d, --data[=...] short and long option with an optional argument
+| -d, --[no-]data[=...]
+| short and long option with an optional argument
|
|Argument hints
| -B <arg> short option required argument
-| --bar2 <arg> long option required argument
-| -e, --fuz <with-space>
+| --[no-]bar2 <arg> long option required argument
+| -e, --[no-]fuz <with-space>
| short and long option required argument
| -s[<some>] short option optional argument
-| --long[=<data>] long option optional argument
-| -g, --fluf[=<path>] short and long option optional argument
-| --longest <very-long-argument-hint>
+| --[no-]long[=<data>] long option optional argument
+| -g, --[no-]fluf[=<path>]
+| short and long option optional argument
+| --[no-]longest <very-long-argument-hint>
| a very long argument hint
-| --pair <key=value> with an equals sign in the hint
-| --aswitch help te=t contains? fl*g characters!`
-| --bswitch <hint> hint has trailing tab character
-| --cswitch switch has trailing tab character
-| --short-hint <a> with a one symbol hint
+| --[no-]pair <key=value>
+| with an equals sign in the hint
+| --[no-]aswitch help te=t contains? fl*g characters!`
+| --[no-]bswitch <hint>
+| hint has trailing tab character
+| --[no-]cswitch switch has trailing tab character
+| --[no-]short-hint <a>
+| with a one symbol hint
|
|Extras
-| --extra1 line above used to cause a segfault but no longer does
+| --[no-]extra1 line above used to cause a segfault but no longer does
|
END_EXPECT
test_expect_code 129 git rev-parse --parseopt -- --does-not-exist 1>/dev/null 2>output < optionspec &&
@@ -297,7 +307,7 @@ test_expect_success 'test --parseopt help output: "wrapped" options normal "or:"
| or: [--another-option]
| or: cmd [--yet-another-option]
|
- | -h, --help show the help
+ | -h, --[no-]help show the help
|
|EOF
END_EXPECT
@@ -334,7 +344,7 @@ test_expect_success 'test --parseopt help output: multi-line blurb after empty l
| line
| blurb
|
- | -h, --help show the help
+ | -h, --[no-]help show the help
|
|EOF
END_EXPECT
--
2.41.0
