From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH] ls-tree: fix --no-full-name
Date: Fri, 21 Jul 2023 13:09:45 -0700 [thread overview]
Message-ID: <xmqq4jlxuiuu.fsf@gitster.g> (raw)
In-Reply-To: <43ca3f01-ba11-6c29-a8e8-4e6c262a68cc@web.de> ("René Scharfe"'s message of "Fri, 21 Jul 2023 21:29:35 +0200")
René Scharfe <l.s.r@web.de> writes:
> 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@web.de>
> ---
> 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);
> + }
This is a good starting point, but we should at least exempt
OPT_BOOL from this exercise, I would think, because ...
> A helper function for the parse-options API.
>
> - --yes get a boolean
> + --[no-]yes get a boolean
... they are designed to be prefixed with an optional "no-".
> - -D, --no-doubt begins with 'no-'
> + -D, --[no-]no-doubt begins with 'no-'
Hmph, I really really loved the neat trick to allow "no-doubt"
option to be "positivised" by _dropping_ the leading "no-" at around
0f1930c5 (parse-options: allow positivation of options starting,
with no-, 2012-02-25).
> EOF
Many of the above are amusing and served as good demonstration to
show the blast radius, but it seems that most of them should be
marked with PARSE_OPT_NONEG.
> 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
Indeed it is amusing, but we probably should give PARSE_OPT_NONEG
appropriately, instead of changing the expectations, for many of the
changes we see here, I think.
next prev parent reply other threads:[~2023-07-21 20:09 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-18 15:44 [PATCH] ls-tree: fix --no-full-name René Scharfe
2023-07-18 16:37 ` Junio C Hamano
2023-07-18 20:49 ` Junio C Hamano
2023-07-21 12:41 ` René Scharfe
2023-07-21 12:41 ` René Scharfe
2023-07-21 14:37 ` Junio C Hamano
2023-07-21 19:29 ` René Scharfe
2023-07-21 20:09 ` Junio C Hamano [this message]
2023-07-21 20:14 ` Junio C Hamano
2023-07-24 12:29 ` René Scharfe
2023-07-24 18:51 ` Junio C Hamano
2023-07-24 20:09 ` René Scharfe
2023-07-24 20:50 ` Junio C Hamano
2023-07-28 6:12 ` René Scharfe
2023-07-28 9:45 ` Phillip Wood
2023-07-29 20:40 ` René Scharfe
2023-07-31 15:31 ` Junio C Hamano
2023-08-04 16:40 ` Junio C Hamano
2023-08-04 19:48 ` Phillip Wood
2023-08-05 10:40 ` René Scharfe
2023-07-24 12:29 ` [PATCH v2 0/5] show negatability of options in short help René Scharfe
2023-07-24 12:34 ` [PATCH v2 1/5] subtree: disallow --no-{help,quiet,debug,branch,message} René Scharfe
2023-07-24 12:36 ` [PATCH v2 2/5] t1502, docs: disallow --no-help René Scharfe
2023-07-24 12:38 ` [PATCH v2 3/5] t1502: move optionspec help output to a file René Scharfe
2023-07-24 12:39 ` [PATCH v2 4/5] t1502: test option negation René Scharfe
2023-07-24 12:40 ` [PATCH v2 5/5] parse-options: show negatability of options in short help René Scharfe
2023-08-05 14:33 ` [PATCH v3 0/8] " René Scharfe
2023-08-05 14:37 ` [PATCH v3 1/8] subtree: disallow --no-{help,quiet,debug,branch,message} René Scharfe
2023-08-05 14:37 ` [PATCH v3 2/8] t1502, docs: disallow --no-help René Scharfe
2023-08-05 14:38 ` [PATCH v3 3/8] t1502: move optionspec help output to a file René Scharfe
2023-08-05 14:39 ` [PATCH v3 4/8] t1502: test option negation René Scharfe
2023-08-05 14:40 ` [PATCH v3 5/8] parse-options: show negatability of options in short help René Scharfe
2023-08-05 14:43 ` [PATCH v3 6/8] parse-options: factor out usage_indent() and usage_padding() René Scharfe
2023-08-05 14:44 ` [PATCH v3 7/8] parse-options: no --[no-]no- René Scharfe
2023-08-05 14:52 ` [PATCH v3 8/8] parse-options: simplify usage_padding() René Scharfe
2023-08-05 23:04 ` Junio C Hamano
2023-07-21 12:41 ` [PATCH] show-branch: fix --no-sparse René Scharfe
2023-07-21 14:42 ` Junio C Hamano
2023-07-21 16:30 ` René Scharfe
2023-07-21 12:41 ` [PATCH] show-branch: disallow --no-{date,topo}-order René Scharfe
2023-07-21 12:41 ` [PATCH] reset: disallow --no-{mixed,soft,hard,merge,keep} René Scharfe
2023-07-21 12:41 ` [PATCH] pack-objects: fix --no-quiet René Scharfe
2023-07-21 12:41 ` [PATCH] pack-objects: fix --no-keep-true-parents René Scharfe
2023-07-21 17:03 ` Junio C Hamano
2023-07-21 12:42 ` [PATCH] branch: disallow --no-{all,remotes} René Scharfe
2023-07-21 12:42 ` [PATCH] am: unify definition of --keep-cr and --no-keep-cr René Scharfe
2023-07-21 13:41 ` [PATCH] describe: fix --no-exact-match René Scharfe
2023-07-21 14:10 ` Junio C Hamano
2023-07-21 17:00 ` Junio C Hamano
2023-08-08 21:27 ` Jeff King
2023-08-08 21:28 ` Jeff King
2023-08-09 1:43 ` Junio C Hamano
2023-08-09 14:09 ` Jeff King
2023-08-09 16:41 ` René Scharfe
2023-08-09 19:07 ` Junio C Hamano
2023-08-10 0:26 ` Jeff King
2023-08-10 1:00 ` Junio C Hamano
2023-08-10 19:45 ` René Scharfe
2023-08-10 0:41 ` Jeff King
2023-08-10 19:10 ` René Scharfe
2023-08-11 15:11 ` Jeff King
2023-08-11 17:59 ` René Scharfe
2023-08-11 18:24 ` Jeff King
2023-08-12 5:11 ` René Scharfe
2023-08-11 15:13 ` Jeff King
2023-08-11 17:59 ` René Scharfe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq4jlxuiuu.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=l.s.r@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).