On 2024.03.03 13:19, René Scharfe wrote:
> Strip "no-" from arg and long_name before comparing them. This way we
> no longer have to repeat the comparison with an offset of 3 for negated
> arguments.
>
> Note that we must not modify the "flags" value, which tracks whether arg
> is negated, inside the loop. When registering "--n", "--no" or "--no-"
> as abbreviation for any negative option, we used to OR it with OPT_UNSET
> and end the loop. We can simply hard-code OPT_UNSET and leave flags
> unchanged instead.
>
> Signed-off-by: René Scharfe <l.s.r@xxxxxx>
> ---
> parse-options.c | 44 ++++++++++++++++++++++----------------------
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 008c0f32cf..d45efa4e5c 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -382,28 +382,42 @@ static enum parse_opt_result parse_long_opt(
> const struct option *options)
> {
> const char *arg_end = strchrnul(arg, '=');
> + const char *arg_start = arg;
> + enum opt_parsed flags = OPT_LONG;
> + int arg_starts_with_no_no = 0;
> struct parsed_option abbrev = { .option = NULL, .flags = OPT_LONG };
> struct parsed_option ambiguous = { .option = NULL, .flags = OPT_LONG };
>
> + if (skip_prefix(arg_start, "no-", &arg_start)) {
> + if (skip_prefix(arg_start, "no-", &arg_start))
> + arg_starts_with_no_no = 1;
> + else
> + flags |= OPT_UNSET;
> + }
> +
> for (; options->type != OPTION_END; options++) {
> const char *rest, *long_name = options->long_name;
> - enum opt_parsed flags = OPT_LONG, opt_flags = OPT_LONG;
> + enum opt_parsed opt_flags = OPT_LONG;
> + int allow_unset = !(options->flags & PARSE_OPT_NONEG);
>
> if (options->type == OPTION_SUBCOMMAND)
> continue;
> if (!long_name)
> continue;
>
> - if (!starts_with(arg, "no-") &&
> - !(options->flags & PARSE_OPT_NONEG) &&
> - skip_prefix(long_name, "no-", &long_name))
> + if (skip_prefix(long_name, "no-", &long_name))
> opt_flags |= OPT_UNSET;
> + else if (arg_starts_with_no_no)
> + continue;
We only allow double negation ("--no-no-foo") if the canonical form
starts with "--no-". Makes sense.
> + if (((flags ^ opt_flags) & OPT_UNSET) && !allow_unset)
> + continue;
I don't think the "& OPT_UNSET" is required here, as we never set any
flags other than OPT_UNSET, but I think it's fine to keep it for
clarity.
> - if (!skip_prefix(arg, long_name, &rest))
> + if (!skip_prefix(arg_start, long_name, &rest))
> rest = NULL;
> if (!rest) {
> /* abbreviated? */
> - if (!strncmp(long_name, arg, arg_end - arg)) {
> + if (!strncmp(long_name, arg_start, arg_end - arg_start)) {
> register_abbrev(p, options, flags ^ opt_flags,
> &abbrev, &ambiguous);
> }
> @@ -412,24 +426,10 @@ static enum parse_opt_result parse_long_opt(
> continue;
> /* negated and abbreviated very much? */
> if (starts_with("no-", arg)) {
> - flags |= OPT_UNSET;
> - register_abbrev(p, options, flags ^ opt_flags,
> + register_abbrev(p, options, OPT_UNSET ^ opt_flags,
> &abbrev, &ambiguous);
> - continue;
> - }
> - /* negated? */
> - if (!starts_with(arg, "no-"))
> - continue;
> - flags |= OPT_UNSET;
> - if (!skip_prefix(arg + 3, long_name, &rest)) {
> - /* abbreviated and negated? */
> - if (!strncmp(long_name, arg + 3,
> - arg_end - arg - 3))
> - register_abbrev(p, options,
> - flags ^ opt_flags,
> - &abbrev, &ambiguous);
> - continue;
> }
> + continue;
> }
> if (*rest) {
> if (*rest != '=')
> --
> 2.44.0
>
>
