Support Better Common Options Parsing (#176)

* support show/hide/suppress labels in config files * ensure suppress/hide always override show * Don't support overrides from the top level of config any more * Fix a bug in computing the value to set * Make sure the default value is always the last choice * Fix tests * Upgrade to 0.13.+ * Fix a copy paste bug
ossuminc · Sep 9, 2022 · 420c245 · 420c245
1 parent 3e37000
commit 420c245
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 45 deletions.
diff --git a/commands/src/main/scala/com/reactific/riddl/commands/CommandOptions.scala b/commands/src/main/scala/com/reactific/riddl/commands/CommandOptions.scala
@@ -78,23 +78,21 @@ object CommandOptions {
     (cur: ConfigCursor) =>
       {
         for {
-          objCur <- cur.asObjectCursor
-          showTimes <- optional(objCur, "show-times", noBool)(c =>
-            c.asBoolean.map(Option(_))
-          )
-          verbose <- optional(objCur, "verbose", noBool)(cc =>
-            cc.asBoolean.map(Option(_))
-          )
-          dryRun <- optional(objCur, "dry-run", noBool)(cc =>
-            cc.asBoolean.map(Option(_))
-          )
+          topCur <- cur.asObjectCursor
+          topRes <- topCur.atKey("common")
+          objCur <- topRes.asObjectCursor
+          showTimes <- optional[Boolean](objCur, "show-times", false)(
+            c => c.asBoolean)
+          verbose <- optional(objCur, "verbose", false)(
+            cc => cc.asBoolean)
+          dryRun <- optional(objCur, "dry-run", false)(
+            cc => cc.asBoolean)
           quiet <-
-            optional(objCur, "quiet", noBool)(cc => cc.asBoolean.map(Option(_)))
-          debug <-
-            optional(objCur, "debug", noBool)(cc => cc.asBoolean.map(Option(_)))
-          suppressWarnings <- optional(objCur, "suppress-warnings", noBool) {
-            cc => cc.asBoolean.map(Option(_))
-          }
+            optional(objCur, "quiet", false)(cc => cc.asBoolean)
+          debug <- optional(objCur, "debug", false)(
+            cc => cc.asBoolean)
+          suppressWarnings <- optional(objCur, "suppress-warnings", noBool)(
+            cc => cc.asBoolean.map(Option(_)))
           suppressStyleWarnings <-
             optional(objCur, "suppress-style-warnings", noBool) { cc =>
               cc.asBoolean.map(Option(_))
@@ -103,25 +101,51 @@ object CommandOptions {
             optional(objCur, "suppress-missing-warnings", noBool) { cc =>
               cc.asBoolean.map(Option(_))
             }
+          hideWarnings <- optional(objCur, "hide-warnings", noBool)(
+            cc => cc.asBoolean.map(Option(_)))
+          hideStyleWarnings <-
+            optional(objCur, "hide-style-warnings", noBool) { cc =>
+              cc.asBoolean.map(Option(_))
+            }
+          hideMissingWarnings <-
+            optional(objCur, "hide-missing-warnings", noBool) { cc =>
+              cc.asBoolean.map(Option(_))
+            }
+          showWarnings <- optional(objCur, "show-warnings", noBool) {
+            cc => cc.asBoolean.map(Option(_))
+          }
+          showStyleWarnings <-
+            optional(objCur, "show-style-warnings", noBool) { cc =>
+              cc.asBoolean.map(Option(_))
+            }
+          showMissingWarnings <-
+            optional(objCur, "show-missing-warnings", noBool) { cc =>
+              cc.asBoolean.map(Option(_))
+            }
           pluginsDir <- optional(objCur, "plugins-dir", Option.empty[Path]) {
             cc => cc.asString.map(f => Option(Path.of(f)))
           }
-          common <- optional[CommonOptions](objCur, "common", CommonOptions()) {
-            cur => commonOptionsReader.from(cur)
-          }
         } yield {
+          val default = CommonOptions()
+          val shouldShowWarnings = suppressWarnings.map(!_).getOrElse(
+            hideWarnings.map(!_).getOrElse(
+              showWarnings.getOrElse(default.showWarnings)))
+          val shouldShowMissing = suppressMissingWarnings.map(!_).getOrElse(
+            hideMissingWarnings.map(!_).getOrElse(
+              showMissingWarnings.getOrElse(default.showMissingWarnings)))
+          val shouldShowStyle = suppressStyleWarnings.map(!_).getOrElse(
+            hideStyleWarnings.map(!_).getOrElse(
+            showStyleWarnings.getOrElse(default.showStyleWarnings)))
+
           CommonOptions(
-            showTimes.getOrElse(common.showTimes),
-            verbose.getOrElse(common.verbose),
-            dryRun.getOrElse(common.dryRun),
-            quiet.getOrElse(common.quiet),
-            showWarnings = suppressWarnings.map(!_)
-              .getOrElse(common.showWarnings),
-            showMissingWarnings = suppressMissingWarnings.map(!_)
-              .getOrElse(common.showMissingWarnings),
-            showStyleWarnings = suppressStyleWarnings.map(!_)
-              .getOrElse(common.showStyleWarnings),
-            debug.getOrElse(common.debug),
+            showTimes,
+            verbose,
+            dryRun,
+            quiet,
+            shouldShowWarnings,
+            shouldShowMissing,
+            shouldShowStyle,
+            debug,
             pluginsDir
           )
         }

diff --git a/commands/src/main/scala/com/reactific/riddl/commands/FromCommand.scala b/commands/src/main/scala/com/reactific/riddl/commands/FromCommand.scala
@@ -1,7 +1,7 @@
 package com.reactific.riddl.commands
 import com.reactific.riddl.language.CommonOptions
 import com.reactific.riddl.language.Messages.Messages
-import com.reactific.riddl.utils.Logger
+import com.reactific.riddl.utils.{Logger, StringHelpers}
 import pureconfig.{ConfigCursor, ConfigReader}
 import scopt.OParser
 
@@ -58,9 +58,24 @@ class FromCommand extends CommandPlugin[FromCommand.Options]("from") {
     commonOptions: CommonOptions,
     log: Logger
   ): Either[Messages, Unit] = {
+    val loadedCO =
+      CommandOptions.loadCommonOptions(options.inputFile.get) match {
+        case Right(newCO: CommonOptions) =>
+          if (commonOptions.verbose) {
+            println(s"Read new common options from ${
+              options.inputFile.get} as:\n" +
+              StringHelpers.toPrettyString(newCO))
+          }
+          newCO
+        case Left(messages) =>
+          if (commonOptions.debug) {
+            println(s"Failed to read common options from ${
+              options.inputFile.get} because:\n" ++ messages.format)
+          }
+          commonOptions
+    }
     val result = CommandPlugin.runFromConfig(
-      options.inputFile, options.targetCommand, commonOptions, log, pluginName
-    )
+      options.inputFile, options.targetCommand, loadedCO, log, pluginName)
     result
   }
 

diff --git a/commands/src/test/scala/com/reactific/riddl/commands/CommonOptionsTest.scala b/commands/src/test/scala/com/reactific/riddl/commands/CommonOptionsTest.scala
@@ -43,14 +43,14 @@ class CommonOptionsTest extends AnyWordSpec with Matchers {
       }
     }
 
-    "common options override properly" in {
+    "options at top level do not override in common object" in {
       val optionFile = Path.of("riddlc/src/test/input/common-overrides.conf")
       CommandOptions.loadCommonOptions(optionFile) match {
         case Left(messages) => fail(messages.format)
         case Right(opts) =>
-          opts.showWarnings mustBe true
-          opts.showStyleWarnings mustBe true
-          opts.showMissingWarnings mustBe true
+          opts.showWarnings mustBe false
+          opts.showStyleWarnings mustBe false
+          opts.showMissingWarnings mustBe false
       }
     }
 

diff --git a/commands/src/test/scala/com/reactific/riddl/commands/PluginCommandTest.scala b/commands/src/test/scala/com/reactific/riddl/commands/PluginCommandTest.scala
@@ -30,7 +30,7 @@ class PluginCommandTest
       val (parser, default) = plugin.getOptions
       OParser.parse(parser, args, default) match {
         case Some(to) => to.arg1 must be("Success!")
-        case None     => fail("No options returned from OParser.parse")
+        case None => fail("No options returned from OParser.parse")
       }
     }
     "get options from config file" in {
@@ -45,7 +45,7 @@ class PluginCommandTest
       ConfigSource.file(path.toFile)
         .load[ASimpleTestCommand.Options](reader) match {
         case Right(loadedOptions) => loadedOptions.arg1 mustBe "Success!"
-        case Left(failures)       => fail(failures.prettyPrint())
+        case Left(failures) => fail(failures.prettyPrint())
       }
     }
 
@@ -74,8 +74,8 @@ class PluginCommandTest
         "--suppress-style-warnings",
         "--suppress-missing-warnings",
         "from",
-        "commands/src/test/input/repeat-options.conf", // wrong file!
-        "flumox"
+        "commands/src/test/input/repeat-options.conf",
+        "flumox" // unknown command
       )
       val rc = CommandPlugin.runMain(args)
       rc must not(be(0))

diff --git a/riddlc/src/test/input/common-overrides.conf b/riddlc/src/test/input/common-overrides.conf
@@ -1,5 +1,10 @@
 command = parse
-suppress-warnings = false
+show-warnings = true
+show-style-warnings = true
+show-missing-warnings = true
+
 common {
-  suppress-warnings = true
+  hide-warnings = true
+  hide-style-warnings = true
+  hide-missing-warnings = true
 }
diff --git a/riddlc/src/test/scala/com/reactific/riddl/RiddlCommandsTest.scala b/riddlc/src/test/scala/com/reactific/riddl/RiddlCommandsTest.scala
@@ -7,7 +7,7 @@ import org.scalatest.Assertion
 class RiddlCommandsTest extends RunCommandSpecBase {
 
   val inputFile = "testkit/src/test/input/rbbq.riddl"
-  val hugoConfig = "testkit/src/test/input/hugo.config"
+  val hugoConfig = "testkit/src/test/input/hugo.conf"
   val validateConfig = "testkit/src/test/input/validate.conf"
   val outputDir: String => String =
     (name: String) => s"riddlc/target/test/$name"

diff --git a/testkit/src/test/input/hugo.config → testkit/src/test/input/hugo.conf b/testkit/src/test/input/hugo.config → testkit/src/test/input/hugo.conf
@@ -1,4 +1,4 @@
-common = {
+common {
   show-times = true
   verbose = false
   quiet = false