From: Josh Steadmon <steadmon@google•com>
To: "René Scharfe" <l.s.r@web•de>
Cc: git@vger•kernel.org
Subject: Re: [PATCH v2 1/6] parse-options: recognize abbreviated negated option with arg
Date: Tue, 12 Mar 2024 14:42:50 -0700 [thread overview]
Message-ID: <ZfDMWoRuftBcg-19@google.com> (raw)
In-Reply-To: <20240303121944.20627-2-l.s.r@web.de>
On 2024.03.03 13:19, René Scharfe wrote:
> Giving an argument to an option that doesn't take one causes Git to
> report that error specifically:
>
> $ git rm --dry-run=bogus
> error: option `dry-run' takes no value
>
> The same is true when the option is negated or abbreviated:
>
> $ git rm --no-dry-run=bogus
> error: option `no-dry-run' takes no value
>
> $ git rm --dry=bogus
> error: option `dry-run' takes no value
>
> Not so when doing both, though:
>
> $ git rm --no-dry=bogus
> error: unknown option `no-dry=bogus'
> usage: git rm [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch]
>
> (Rest of the usage message omitted.)
>
> Improve consistency and usefulness of the error message by recognizing
> abbreviated negated options even if they have a (most likely bogus)
> argument. With this patch we get:
>
> $ git rm --no-dry=bogus
> error: option `no-dry-run' takes no value
>
> Signed-off-by: René Scharfe <l.s.r@web•de>
> ---
> parse-options.c | 5 +++--
> t/t0040-parse-options.sh | 16 ++++++++++++++++
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 63a99dea6e..e4ce33ea48 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -391,7 +391,7 @@ static enum parse_opt_result parse_long_opt(
> ambiguous_option = abbrev_option;
> ambiguous_flags = abbrev_flags;
> }
> - if (!(flags & OPT_UNSET) && *arg_end)
> + if (*arg_end)
> p->opt = arg_end + 1;
> abbrev_option = options;
> abbrev_flags = flags ^ opt_flags;
So this part allows us to recognize when we've attached an option onto a
negated flag (like "--no-foo=bar"). Looks good.
> @@ -412,7 +412,8 @@ static enum parse_opt_result parse_long_opt(
> if (!skip_prefix(arg + 3, long_name, &rest)) {
> /* abbreviated and negated? */
> if (allow_abbrev &&
> - starts_with(long_name, arg + 3))
> + !strncmp(long_name, arg + 3,
> + arg_end - arg - 3))
> goto is_abbreviated;
> else
> continue;
And now we fix the abbreviation match. So if arg="no-foo=bar", (arg+3) =
"foo=bar" and thus never matches a long_name. We fix that by only
comparing up to the first equal sign. Also looks good.
> diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
> index ec974867e4..8bb2a8b453 100755
> --- a/t/t0040-parse-options.sh
> +++ b/t/t0040-parse-options.sh
> @@ -210,6 +210,22 @@ test_expect_success 'superfluous value provided: boolean' '
> test_cmp expect actual
> '
>
> +test_expect_success 'superfluous value provided: boolean, abbreviated' '
> + cat >expect <<-\EOF &&
> + error: option `yes'\'' takes no value
> + EOF
> + test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
> + test-tool parse-options --ye=hi 2>actual &&
> + test_cmp expect actual &&
> +
> + cat >expect <<-\EOF &&
> + error: option `no-yes'\'' takes no value
> + EOF
> + test_expect_code 129 env GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
> + test-tool parse-options --no-ye=hi 2>actual &&
> + test_cmp expect actual
> +'
> +
> test_expect_success 'superfluous value provided: cmdmode' '
> cat >expect <<-\EOF &&
> error: option `mode1'\'' takes no value
> --
> 2.44.0
Test looks good as well.
next prev parent reply other threads:[~2024-03-12 21:42 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-24 21:29 [PATCH 0/6] parse-options: long option parsing fixes and cleanup René Scharfe
2024-02-24 21:29 ` [PATCH 1/6] parse-options: recognize abbreviated negated option with arg René Scharfe
2024-02-24 21:29 ` [PATCH 2/6] parse-options: set arg of abbreviated option lazily René Scharfe
2024-02-24 21:29 ` [PATCH 3/6] parse-options: factor out register_abbrev() and struct parsed_option René Scharfe
2024-02-24 23:13 ` Junio C Hamano
2024-02-24 21:29 ` [PATCH 4/6] parse-options: detect ambiguous self-negation René Scharfe
2024-02-24 21:29 ` [PATCH 5/6] parse-options: normalize arg and long_name before comparison René Scharfe
2024-02-24 21:29 ` [PATCH 6/6] parse-options: rearrange long_name matching code René Scharfe
2024-02-24 21:42 ` [PATCH 0/6] parse-options: long option parsing fixes and cleanup René Scharfe
2024-03-03 12:19 ` [PATCH v2 " René Scharfe
2024-03-03 12:19 ` [PATCH v2 1/6] parse-options: recognize abbreviated negated option with arg René Scharfe
2024-03-12 21:42 ` Josh Steadmon [this message]
2024-03-03 12:19 ` [PATCH v2 2/6] parse-options: set arg of abbreviated option lazily René Scharfe
2024-03-03 12:19 ` [PATCH v2 3/6] parse-options: factor out register_abbrev() and struct parsed_option René Scharfe
2024-03-12 21:43 ` Josh Steadmon
2024-03-13 17:48 ` Junio C Hamano
2024-03-13 18:47 ` René Scharfe
2024-03-03 12:19 ` [PATCH v2 4/6] parse-options: detect ambiguous self-negation René Scharfe
2024-03-03 12:19 ` [PATCH v2 5/6] parse-options: normalize arg and long_name before comparison René Scharfe
2024-03-12 21:43 ` Josh Steadmon
2024-03-03 12:19 ` [PATCH v2 6/6] parse-options: rearrange long_name matching code René Scharfe
2024-03-12 21:44 ` Josh Steadmon
2024-03-12 21:45 ` [PATCH v2 0/6] parse-options: long option parsing fixes and cleanup Josh Steadmon
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=ZfDMWoRuftBcg-19@google.com \
--to=steadmon@google$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=l.s.r@web$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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