Solver options: space for '=' #220

Allow 'outlev 1' instead of 'outlev=1', 'outlev ?', etc
ampl · Aug 14, 2023 · 05d9536 · 05d9536
1 parent acaf531
commit 05d9536
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 48 deletions.
diff --git a/include/mp/solver-opt.h b/include/mp/solver-opt.h
@@ -225,25 +225,43 @@ class SolverOption {
     SetValue(long_value);
   }
 
-  /// Formats the option value. Throws OptionError in case of error.
+  /// Echo option name(s) [= value].
+  virtual std::string echo_with_value() {
+    auto s = echo();
+    if (!is_flag()) {
+      fmt::MemoryWriter w;
+      w << " = ";
+      this->Write(w);
+      s += w.c_str();
+    }
+    return s;
+  }
+
+  /// Formats the option value.
+  /// Throws OptionError in case of error.
+  /// Not called for flags.
   virtual void Write(fmt::Writer &w) = 0;
 
   /// Parses a string and sets the option value. Throws InvalidOptionValue
   /// if the value is invalid or OptionError in case of another error.
   virtual void Parse(const char *&s, bool splitString=false) = 0;
 
+  /// Echo option name(s).
   virtual std::string echo() {
     if (is_wildcard())
       return wc_key_last__std_form();
     return name();
   }
 
+  /// Option type classifier, used in AMPLS.
   enum Option_Type {
     BOOL,
     INT,
     DBL,
     STRING
   };
+
+  /// Return type classifier.
   virtual Option_Type type() = 0;
 
 private:

diff --git a/src/solver.cc b/src/solver.cc
@@ -843,7 +843,6 @@ SolverOption *SolverOptionManager::FindOption(
 
 void BasicSolver::ParseOptionString(
     const char *s, unsigned flags) {
-  bool skip = false;
   for (;;) {
     if (!*(s = SkipSpaces(s)))
       return;
@@ -859,77 +858,57 @@ void BasicSolver::ParseOptionString(
       name[i] = name_start[i];
     name[name_size] = 0;
 
-    // Parse the option value.
+    // Check if we have an '=' sign,
+    // skip all spaces intil next token.
     bool equal_sign = false;
     s = SkipSpaces(s);
     if (*s == '=') {
       s = SkipSpaces(s + 1);
       equal_sign = true;
     }
 
+    // Parse option name.
     SolverOption *opt = FindOption(&name[0], true);
     if (!opt) {
-      if (!skip)
-        HandleUnknownOption(&name[0]);
-      if (equal_sign) {
-        s = SkipNonSpaces(s);
-      } else {
-        // Skip everything until the next known option if there is no "="
-        // because it is impossible to know whether the next token is an
-        // option name or a value.
-        // For example, if "a" in "a b c" is an unknown option, then "b"
-        // can be either a value of option "a" or another option.
-        skip = true;
-      }
-      continue;
+      HandleUnknownOption(&name[0]);
+      continue;       // in case it does not throw
     }
 
-    skip = false;
+    // If user asks the default/current value.
     if (*s == '?') {
       char next = s[1];
       if (!next || std::isspace(next)) {
         ++s;
         if ((flags & NO_OPTION_ECHO) == 0) {
-          fmt::MemoryWriter w;
-          w << opt->echo() << '=';
-          opt->Write(w);
-          w << '\n';
-          Print("{}", w.c_str());
+          Print("{}", opt->echo_with_value() + '\n');
         }
         continue;
       }
     }
-    if (opt->is_flag() && equal_sign) {
-      MP_RAISE(
-            fmt::format("Option \"{}\" doesn't accept an argument",
-                        &name[0]));
-      s = SkipNonSpaces(s);
-      continue;
-    }
-    if (!equal_sign) {
+
+    /// Parse value if needed.
+    if (equal_sign) {
+      // No '=' for flags.
       if (opt->is_flag()) {
-        opt->Parse(s, flags & FROM_COMMAND_LINE);
-      } else // Emulate flag options for integer options
-      if (SolverOption::Option_Type::INT==opt->type()) {
-        auto s_ = s;
-        s = "1";
-        opt->Parse(s, flags & FROM_COMMAND_LINE);
-        s = s_;
+        ReportError(
+              "Option \"{}\" doesn't accept an argument",
+              &name[0]);
+        s = SkipNonSpaces(s);    // In case we go on
+        continue;
       } else {
-        MP_RAISE(
-              fmt::format("Option \"{}\" requires an argument",
-                          &name[0]));
+        opt->Parse(s, flags & FROM_COMMAND_LINE);
       }
     } else {
-      opt->Parse(s, flags & FROM_COMMAND_LINE);
+      if (opt->is_flag()) {         // Might set some flag
+        opt->Parse(s, flags & FROM_COMMAND_LINE);
+      } else {                      // Even w/o '=' sign
+        opt->Parse(s, flags & FROM_COMMAND_LINE);
+      }
     }
-    if ((flags & NO_OPTION_ECHO) == 0)
-    {
-      fmt::MemoryWriter w;
-      w << opt->echo() << '=';
-      opt->Write(w);
-      w << '\n';
-      Print("{}", w.c_str());
+
+    // Echo name [= value].
+    if ((flags & NO_OPTION_ECHO) == 0) {
+      Print("{}", opt->echo_with_value() + '\n');
     }
   }
 }

diff --git a/test/end2end/cases/categorized/fast/tech/modellist.json b/test/end2end/cases/categorized/fast/tech/modellist.json
@@ -1,4 +1,13 @@
 [
+  {
+    "name" : "Check_Standard_Options",
+    "files" : ["diet.mod", "diet.dat"],
+    "tags" : [ ],
+    "comment": "Check standard options: version, outlev",
+    "comment": "Check acceptance of '?' and space/= for assignment",
+    "options": { "ANYSOLVER_options": "version outlev ? outlev=? outlev 1 outlev=1 outlev 1" },
+    "objective": 88.2
+  },
   {
     "name" : "Check_Wrong_Options",
     "tags" : ["linear", "continuous"],