options: change handling of "no-" options yet again

Commit 4a40eed "options: change handling of "no-" options" generally
improved the handling of automatically added negation options
(recognizing "--no-opt", even though only "--opt" is declared in the
option list).

Unfortunately, one corner case was missed, which broke the option
"--input=no-default-bindings" (other suboptions, e.g. VO suboptions,
were not affected, and this is the only option where this mattered).

Instead of increasing the complexity further, use a completely different
approach: add the "--no-" options at runtime, and make them behave like
real options. This approach could be considered slightly less elegant,
because the code now has to worry about some option implementation
details rather than leaving it to the parser, but all in all the new
code is simpler and there are less weird corner cases to worry about.

4 files changed, 55 insertions, 45 deletions

diff --git a/core/m_config.c b/core/m_config.c
index 2d994bb802..6ecfa13a39 100644
--- a/core/m_config.c
+++ b/core/m_config.c
@@ -257,6 +257,43 @@ void m_config_leave_file_local(struct m_config *config)
     }
 }
 
+// Given an option --opt, add --no-opt (if applicable).
+static void add_negation_option(struct m_config *config,
+                                struct m_config_option *parent,
+                                const struct m_option *opt)
+{
+    int value;
+    if (opt->type == CONF_TYPE_FLAG) {
+        value = opt->min;
+    } else if (opt->type == CONF_TYPE_CHOICE) {
+        // Find out whether there's a "no" choice.
+        // m_option_parse() should be used for this, but it prints
+        // unsilenceable error messages.
+        struct m_opt_choice_alternatives *alt = opt->priv;
+        for ( ; alt->name; alt++) {
+            if (strcmp(alt->name, "no") == 0)
+                break;
+        }
+        if (!alt->name)
+            return;
+        value = alt->value;
+    } else {
+        return;
+    }
+    struct m_option *no_opt = talloc_ptrtype(config, no_opt);
+    *no_opt = (struct m_option) {
+        .name = talloc_asprintf(no_opt, "no-%s", opt->name),
+        .type = CONF_TYPE_STORE,
+        .flags = opt->flags & (M_OPT_NOCFG | M_OPT_GLOBAL | M_OPT_LOCAL |
+                               M_OPT_PRE_PARSE | M_OPT_PREFIXED | M_OPT_MERGE),
+        .new = opt->new,
+        .p = opt->p,
+        .offset = opt->offset,
+        .max = value,
+    };
+    m_config_add_option(config, parent, no_opt);
+}
+
 static void add_options(struct m_config *config,
                         struct m_config_option *parent,
                         const struct m_option *defs)
@@ -352,6 +389,8 @@ static void m_config_add_option(struct m_config *config,
         co->next = config->opts;
         config->opts = co;
     }
+
+    add_negation_option(config, parent, arg);
 }
 
 int m_config_register_options(struct m_config *config,
@@ -393,10 +432,6 @@ static int m_config_parse_option(struct m_config *config, void *optstruct,
     assert(name.len != 0);
     bool set = !(flags & M_SETOPT_CHECK_ONLY);
 
-    int r = m_config_map_option(config, &name, &param, false);
-    if (r < 0)
-        return r;
-
     struct m_config_option *co = m_config_get_co(config, name);
     if (!co)
         return M_OPT_UNKNOWN;
@@ -516,35 +551,18 @@ const struct m_option *m_config_get_option(const struct m_config *config,
         return NULL;
 }
 
-int m_config_map_option(struct m_config *config, bstr *name, bstr *param,
-                        bool ambiguous)
+int m_config_option_requires_param(struct m_config *config, bstr name)
 {
-    bstr s = *name;
-    const struct m_option *opt = m_config_get_option(config, s);
+    const struct m_option *opt = m_config_get_option(config, name);
     if (opt) {
-        if (bstr_endswith0(s, "-clr"))
-            return (ambiguous || !param->len) ? 0 : M_OPT_DISALLOW_PARAM;
-        if (ambiguous && ((opt->flags & M_OPT_OPTIONAL_PARAM) ||
-                          (opt->type->flags & M_OPT_TYPE_OPTIONAL_PARAM)))
+        if (bstr_endswith0(name, "-clr"))
+            return 0;
+        if (((opt->flags & M_OPT_OPTIONAL_PARAM) ||
+             (opt->type->flags & M_OPT_TYPE_OPTIONAL_PARAM)))
             return 0;
         return 1;
     }
-
-    if (!bstr_eatstart0(&s, "no-"))
-        return M_OPT_UNKNOWN;
-
-    opt = m_config_get_option(config, s);
-    if (!opt || (opt->type != &m_option_type_flag
-                 && opt->type != &m_option_type_choice))
-        return M_OPT_UNKNOWN;
-    // Avoid allowing "--no-no-opt".
-    if (bstr_startswith(bstr0(opt->name), bstr0("no-")))
-        return M_OPT_UNKNOWN;
-    if (param->len)
-        return M_OPT_DISALLOW_PARAM;
-    *name = s;
-    *param = bstr0("no");
-    return 0;
+    return M_OPT_UNKNOWN;
 }
 
 void m_config_print_option_list(const struct m_config *config)
diff --git a/core/m_config.h b/core/m_config.h
index 57170cf48a..86376e4849 100644
--- a/core/m_config.h
+++ b/core/m_config.h
@@ -147,20 +147,12 @@ const struct m_option *m_config_get_option(const struct m_config *config,
 struct m_config_option *m_config_get_co(const struct m_config *config,
                                         struct bstr name);
 
-/*  Map options like "no-opt=" to "opt=no".
- *  config:     config object.
- *  name:       option's name. May be set to a new name.
- *  value:      option value. May be set to a new value.
- *  ambiguous:  if true, "value" may be either an option value, or a separate,
- *              unrelated option following the current option.
- *  returns the following error codes:
- *          < 0: one of the M_OPT_ error codes
- *          0: the option is valid, *value is set implicitly
- *             ("--foo bar" maps to "--foo=yes" + "bar", *value == "yes")
- *          1: the option is valid, *value is a proper parameter
- */
-int m_config_map_option(struct m_config *config, bstr *name, bstr *value,
-                        bool ambiguous);
+// Return a hint to the option parser whether a parameter is/may be required.
+// The option may still accept empty/non-empty parameters independent from
+// this, and this function is useful only for handling ambiguous options like
+// flags (e.g. "--a" is ok, "--a=yes" is also ok).
+// Returns: error code (<0), or number of expected params (0, 1)
+int m_config_option_requires_param(struct m_config *config, bstr name);
 
 /*  Print a list of all registered options.
  *  \param config The config object.
diff --git a/core/parser-cfg.c b/core/parser-cfg.c
index e5bf6eb3b5..5d63d27c52 100644
--- a/core/parser-cfg.c
+++ b/core/parser-cfg.c
@@ -200,7 +200,7 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile)
         bstr bopt = bstr0(opt);
         bstr bparam = bstr0(param);
 
-        tmp = m_config_map_option(config, &bopt, &bparam, false);
+        tmp = m_config_option_requires_param(config, bopt);
         if (tmp > 0 && !param_set)
             tmp = M_OPT_MISSING_PARAM;
         if (tmp < 0) {
@@ -213,7 +213,7 @@ int m_config_parse_config_file(m_config_t *config, const char *conffile)
 
         if (profile) {
             if (!strcmp(opt, "profile-desc"))
-                m_profile_set_desc(profile, param), tmp = 1;
+                m_profile_set_desc(profile, param);
             else
                 tmp = m_config_set_profile_option(config, profile,
                                                   bopt, bparam);
diff --git a/core/parser-mpcmd.c b/core/parser-mpcmd.c
index 1a7a4dcbc9..fd22a1e054 100644
--- a/core/parser-mpcmd.c
+++ b/core/parser-mpcmd.c
@@ -82,7 +82,7 @@ static int split_opt_silent(struct parse_state *p)
 
     bool ambiguous = !bstr_split_tok(p->arg, "=", &p->arg, &p->param);
 
-    int r = m_config_map_option(p->config, &p->arg, &p->param, ambiguous);
+    int r = m_config_option_requires_param(p->config, p->arg);
     if (r < 0)
         return r;