SI-8525 MultiChoice takes a helper function

Clean-up code review comments.

MultiChoice takes a helper function for outputting help text.

5 files changed, 40 insertions, 20 deletions

diff --git a/src/compiler/scala/tools/nsc/CompilerCommand.scala b/src/compiler/scala/tools/nsc/CompilerCommand.scala
index 95f7a70980..3ded456378 100644
--- a/src/compiler/scala/tools/nsc/CompilerCommand.scala
+++ b/src/compiler/scala/tools/nsc/CompilerCommand.scala
@@ -107,7 +107,7 @@ class CompilerCommand(arguments: List[String], val settings: Settings) {
     else {
       val sb = new StringBuilder
       allSettings foreach {
-        case s: MultiChoiceSetting if s.isHelping => sb append s.help append "\n"
+        case s: MultiChoiceSetting if s.isHelping => sb append s.help
         case _ =>
       }
       sb.toString
diff --git a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala
index ef9695a594..c73e7ce00e 100644
--- a/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/AbsScalaSettings.scala
@@ -29,7 +29,7 @@ trait AbsScalaSettings {
   def ChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: String): ChoiceSetting
   def IntSetting(name: String, descr: String, default: Int, range: Option[(Int, Int)], parser: String => Option[Int]): IntSetting
   def MultiStringSetting(name: String, helpArg: String, descr: String): MultiStringSetting
-  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit): MultiChoiceSetting
+  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit)(helper: MultiChoiceSetting => String): MultiChoiceSetting
   def OutputSetting(outputDirs: OutputDirs, default: String): OutputSetting
   def PathSetting(name: String, descr: String, default: String): PathSetting
   def PhasesSetting(name: String, descr: String, default: String): PhasesSetting
diff --git a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
index 8c69b49b98..3f41ede3ad 100644
--- a/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/MutableSettings.scala
@@ -201,7 +201,7 @@ class MutableSettings(val errorFn: String => Unit)
     }
 
   // a wrapper for all Setting creators to keep our list up to date
-  private[nsc] def add[T <: Setting](s: T): T = {
+  private def add[T <: Setting](s: T): T = {
     allSettings += s
     s
   }
@@ -211,8 +211,10 @@ class MutableSettings(val errorFn: String => Unit)
     add(new ChoiceSetting(name, helpArg, descr, choices, default))
   def IntSetting(name: String, descr: String, default: Int, range: Option[(Int, Int)], parser: String => Option[Int]) = add(new IntSetting(name, descr, default, range, parser))
   def MultiStringSetting(name: String, arg: String, descr: String) = add(new MultiStringSetting(name, arg, descr))
-  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ()) =
-    add(new MultiChoiceSetting(name, helpArg, descr, choices, default))
+  def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: () => Unit = () => ())(
+      helper: MultiChoiceSetting => String = _ => choices.mkString(f"$descr:%n", f"%n  ", f"%n")
+  ) =
+    add(new MultiChoiceSetting(name, helpArg, descr, choices, default, helper))
   def OutputSetting(outputDirs: OutputDirs, default: String) = add(new OutputSetting(outputDirs, default))
   def PhasesSetting(name: String, descr: String, default: String = "") = add(new PhasesSetting(name, descr, default))
   def StringSetting(name: String, arg: String, descr: String, default: String) = add(new StringSetting(name, arg, descr, default))
@@ -561,7 +563,8 @@ class MutableSettings(val errorFn: String => Unit)
     arg: String,
     descr: String,
     override val choices: List[String],
-    val default: () => Unit
+    val default: () => Unit,
+    helper: MultiChoiceSetting => String
   ) extends MultiStringSetting(name, s"_,$arg,-$arg", s"$descr: `_' for all, `$name:help' to list") {
 
     private def badChoice(s: String, n: String) = errorFn(s"'$s' is not a valid choice for '$name'")
@@ -590,7 +593,7 @@ class MutableSettings(val errorFn: String => Unit)
     }
 
     def isHelping: Boolean = sawHelp
-    def help: String = s"$descr${ choices.mkString(":\n", "    \n", "\n") }"
+    def help: String = helper(this)
   }
 
   /** A setting that accumulates all strings supplied to it,
diff --git a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
index 6675ad96e6..5c3eadbd60 100644
--- a/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
+++ b/src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
@@ -70,8 +70,27 @@ trait ScalaSettings extends AbsScalaSettings
   // Would be nice to build this dynamically from scala.languageFeature.
   // The two requirements: delay error checking until you have symbols, and let compiler command build option-specific help.
   val language      = {
-    val features = List("dynamics", "postfixOps", "reflectiveCalls", "implicitConversions", "higherKinds", "existentials", "experimental.macros")
-    MultiChoiceSetting("-language", "feature", "Enable or disable language features", features)
+    val features = List(
+      "dynamics"            -> "Allow direct or indirect subclasses of scala.Dynamic",
+      "postfixOps"          -> "Allow postfix operator notation, such as `1 to 10 toList'",
+      "reflectiveCalls"     -> "Allow reflective access to members of structural types",
+      "implicitConversions" -> "Allow definition of implicit functions called views",
+      "higherKinds"         -> "Allow higher-kinded types",  // "Ask Adriaan, but if you have to ask..."
+      "existentials"        -> "Existential types (besides wildcard types) can be written and inferred",
+      "experimental.macros" -> "Allow macro defintion (besides implementation and application)"
+    )
+    val description = "Enable or disable language features"
+    MultiChoiceSetting(
+      name    = "-language",
+      helpArg = "feature",
+      descr   = description,
+      choices = features map (_._1)
+    ) { s =>
+      val helpline: ((String, String)) => String = {
+        case (name, text) => f"  $name%-25s $text%n"
+      }
+      features map helpline mkString (f"$description:%n", "", f"%n")
+    }
   }
 
   /*
diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala
index 3accd9fdaa..9989d1d188 100644
--- a/src/compiler/scala/tools/nsc/settings/Warnings.scala
+++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala
@@ -109,18 +109,18 @@ trait Warnings {
   private val xlint = new BooleanSetting("-Zunused", "True if -Xlint or -Xlint:_")
   // On -Xlint or -Xlint:_, set xlint, otherwise set the lint warning unless already set true
   val lint = {
-    val d = "Enable or disable specific warnings"
-    val s = new MultiChoiceSetting(
+    val description = "Enable or disable specific warnings"
+    val choices     = (lintWarnings map (_.name)).sorted
+    MultiChoiceSetting(
       name    = "-Xlint",
-      arg     = "warning",
-      descr   = d,
-      choices = (lintWarnings map (_.name)).sorted,
+      helpArg = "warning",
+      descr   = description,
+      choices = choices,
       default = () => xlint.value = true
-    ) {
+    ) { s =>
       def helpline(n: String) = lintWarnings.find(_.name == n).map(w => f"  ${w.name}%-25s ${w.helpDescription}%n")
-      override def help = s"$d${ choices flatMap (helpline(_)) mkString (":\n", "", "\n") }"
-    }
-    val t = s withPostSetHook { x =>
+      choices flatMap (helpline(_)) mkString (f"$description:%n", "", f"%n")
+    } withPostSetHook { x =>
       val Neg = "-"
       def setPolitely(b: BooleanSetting, v: Boolean) = if (!b.isSetByUser || !b) b.value = v
       def set(w: String, v: Boolean) = lintWarnings find (_.name == w) foreach (setPolitely(_, v))
@@ -130,8 +130,6 @@ trait Warnings {
       }
       propagate(x.value)
     }
-    add(t)
-    t
   }
 
   // Lint warnings that are currently -Y, but deprecated in that usage