Apply suggestions from code review

Author: lwronski

modules/cli/src/main/scala/scala/cli/ScalaCli.scala

@@ -31,7 +31,8 @@ object ScalaCli extends CommandsEntryPoint {
     Package,
     Run,
     SetupIde,
-    Test
+    Test,
+    Version
   )
 
   lazy val progName                 = (new Argv0).get("scala-cli")

modules/cli/src/main/scala/scala/cli/commands/About.scala

@@ -9,7 +9,6 @@ object About extends ScalaCommand[AboutOptions] {
   def run(options: AboutOptions, args: RemainingArgs): Unit = {
     val version            = Constants.version
     val detailedVersionOpt = Some(Constants.detailedVersion).filter(_ != version)
-    if (options.version) println(version)
-    else println(s"Scala CLI version $version" + detailedVersionOpt.fold("")(" (" + _ + ")"))
+    println(s"Scala CLI version $version" + detailedVersionOpt.fold("")(" (" + _ + ")"))
   }
 }

modules/cli/src/main/scala/scala/cli/commands/AboutOptions.scala

@@ -3,12 +3,7 @@ package scala.cli.commands
 import caseapp._
 
 @HelpMessage("Print details about this application")
-final case class AboutOptions(
-  @Group("About")
-  @Name("v")
-  @HelpMessage("Print only the scala-cli version")
-    version: Boolean = false
-)
+final case class AboutOptions()
 
 object AboutOptions {
   implicit lazy val parser: Parser[AboutOptions] = Parser.derive

modules/cli/src/main/scala/scala/cli/commands/InstallHome.scala

@@ -10,8 +10,42 @@ object InstallHome extends ScalaCommand[InstallHomeOptions] {
   override def hidden: Boolean = true
 
   private def isOutOfDate(newVersion: String, oldVersion: String): Boolean = {
-    val versionOrdering = Ordering.by { (_: String).split("""\.""").map(_.toInt).toIterable }
-    versionOrdering.gt(newVersion, oldVersion)
+    import coursier.core.Version
+
+    Version(newVersion) > Version(oldVersion)
+  }
+
+  private def loggingEqual(version: String) = {
+    System.err.println(
+      s"Scala-cli $version is already installed and up-to-date."
+    )
+    sys.exit(0)
+  }
+
+  private def loggingUpdate(env: Boolean, newVersion: String, oldVersion: String) = {
+    if (!env) println(
+      s"""scala-cli $oldVersion is already installed and out-of-date.
+         |scala-cli will be updated to version $newVersion
+         |""".stripMargin
+    )
+  }
+
+  private def loggingDowngrade(env: Boolean, newVersion: String, oldVersion: String) = {
+    if (!env && coursier.paths.Util.useAnsiOutput()) {
+      println(s"scala-cli $oldVersion is already installed and up-to-date.")
+      println(s"Do you want to downgrade scala-cli to version $newVersion [Y/n]")
+      val response = readLine()
+      if (response != "Y") {
+        System.err.println("Abort")
+        sys.exit(1)
+      }
+    }
+    else {
+      System.err.println(
+        s"Error: scala-cli is already installed $oldVersion and up-to-date. Downgrade to $newVersion pass -f or --force."
+      )
+      sys.exit(1)
+    }
   }
 
   def run(options: InstallHomeOptions, args: RemainingArgs): Unit = {
@@ -21,57 +55,30 @@ object InstallHome extends ScalaCommand[InstallHomeOptions] {
     val newScalaCliBinPath = os.Path(options.scalaCliBinaryPath, os.pwd)
 
     val newVersion: String =
-      os.proc(newScalaCliBinPath, "about", "--version").call(cwd = os.pwd).out.text.trim
+      os.proc(newScalaCliBinPath, "version").call(cwd = os.pwd).out.text.trim
 
     // Backward compatibility - previous versions not have the `--version` parameter
     val oldVersion: String = Try {
-      os.proc(binDirPath / options.binaryName, "about", "--version").call(cwd =
-        os.pwd).out.text.trim
+      os.proc(binDirPath / options.binaryName, "version").call(cwd = os.pwd).out.text.trim
     }.toOption.getOrElse("0.0.0")
 
     if (os.exists(binDirPath)) {
-      if (options.force) os.remove.all(binDirPath)
-      else if (newVersion == oldVersion) {
-        System.err.println(
-          s"Scala-cli $newVersion is already installed and up-to-date."
-        )
-        sys.exit(1)
-      }
-      else if (isOutOfDate(newVersion, oldVersion)) {
-        if (!options.env) println(
-          s"""scala-cli $oldVersion is already installed and out-of-date.
-             |scala-cli will be updated to version $newVersion
-             |""".stripMargin
-        )
-        os.remove.all(binDirPath)
-      }
-      else {
-        if (!options.env && coursier.paths.Util.useAnsiOutput()) {
-          println(s"scala-cli $oldVersion is already installed and up-to-date.")
-          println(s"Do you want to downgrade scala-cli to version $newVersion [Y/n]")
-          val response = readLine()
-          if (response != "Y") {
-            System.err.println("Abort")
-            sys.exit(1)
-          }
-          else
-            os.remove.all(binDirPath)
-        }
-        else {
-          System.err.println(
-            s"Error: scala-cli is already installed $oldVersion and up-to-date. Downgrade to $newVersion pass -f or --force."
-          )
-          sys.exit(1)
-        }
-      }
+      if (options.force) () // skip logging
+      else if (newVersion == oldVersion) loggingEqual(newVersion)
+      else if (isOutOfDate(newVersion, oldVersion))
+        loggingUpdate(options.env, newVersion, oldVersion)
+      else loggingDowngrade(options.env, newVersion, oldVersion)
+    }
+
+    os.remove.all(binDirPath)
 
     os.copy(
       from = newScalaCliBinPath,
       to = binDirPath / options.binaryName,
       createFolders = true
     )
     if (!Properties.isWin)
-      os.perms.set(binDirPath / options.binaryName, os.PermSet.fromString("rwxrwxr-x"))
+      os.perms.set(binDirPath / options.binaryName, os.PermSet.fromString("rwxr-xr-x"))
 
     if (options.env) {
       println(s""" export PATH="$$PATH:$binDirPath" """)

modules/cli/src/main/scala/scala/cli/commands/SharedCompilationServerOptions.scala

@@ -2,7 +2,6 @@ package scala.cli.commands
 
 import caseapp._
 import com.google.gson.Gson
-import coursier.core.Version
 
 import java.io.{BufferedReader, File, FileReader}
 import java.nio.file.{AtomicMoveNotSupportedException, FileAlreadyExistsException, Files}
@@ -168,7 +167,9 @@ final case class SharedCompilationServerOptions(
     parseDuration("connection server startup timeout", bloopStartupTimeout)
 
   def minimumBloopVersion = Constants.bloopVersion
-  def acceptBloopVersion  = Some((v: String) => Version(v) < Version(minimumBloopVersion))
+
+  import coursier.core.Version
+  def acceptBloopVersion = Some((v: String) => Version(v) < Version(minimumBloopVersion))
 
   def bloopDefaultJvmOptions(logger: Logger): List[String] = {
     val file = new File(bloopGlobalOptionsFile)

modules/integration/src/test/scala/scala/cli/integration/InstallHomeTests.scala

@@ -25,7 +25,6 @@ class InstallHomeTests extends munit.FunSuite {
            |}""".stripMargin
     )
   )
-  val binDirPath = os.home / ".scala" / "scala-cli"
 
   private def packageDummyScalaCli(root: os.Path, dummyScalaCliFileName: String, output: String) = {
     // format: off
@@ -40,10 +39,19 @@ class InstallHomeTests extends munit.FunSuite {
     )
   }
 
-  private def installScalaCli(root: os.Path, binVersion: String, force: Boolean) = {
+  private def installScalaCli(
+    root: os.Path,
+    binVersion: String,
+    binDirPath: os.Path,
+    force: Boolean
+  ) = {
     // format: off
     val cmdInstallVersion = Seq[os.Shellable](
-      TestUtil.cli, "install-home", "--scala-cli-binary-path", binVersion , "--binary-name", dummyScalaCliBinName, "--bin-dir", binDirPath
+      TestUtil.cli, "install-home", 
+      "--env", 
+      "--scala-cli-binary-path", binVersion , 
+      "--binary-name", dummyScalaCliBinName, 
+      "--bin-dir", binDirPath
     ) ++ (if(force) Seq[os.Shellable]("--force") else Seq.empty)
     // format: on
     os.proc(cmdInstallVersion).call(
@@ -53,46 +61,51 @@ class InstallHomeTests extends munit.FunSuite {
     )
   }
 
-  if (!Properties.isWin && TestUtil.isCI) {
-    test("updating and downgrading dummy scala-cli using install-home command") {
+  def runInstallHome(): Unit = {
 
-      testInputs.fromRoot { root =>
+    testInputs.fromRoot { root =>
+      val binDirPath = root / ".scala" / "scala-cli"
 
-        val binDummyScalaCliFirst  = dummyScalaCliFirstName.stripSuffix(".scala").toLowerCase
-        val binDummyScalaCliSecond = dummyScalaCliSecondName.stripSuffix(".scala").toLowerCase
+      val binDummyScalaCliFirst  = dummyScalaCliFirstName.stripSuffix(".scala").toLowerCase
+      val binDummyScalaCliSecond = dummyScalaCliSecondName.stripSuffix(".scala").toLowerCase
 
-        packageDummyScalaCli(root, dummyScalaCliFirstName, binDummyScalaCliFirst)
-        packageDummyScalaCli(root, dummyScalaCliSecondName, binDummyScalaCliSecond)
+      packageDummyScalaCli(root, dummyScalaCliFirstName, binDummyScalaCliFirst)
+      packageDummyScalaCli(root, dummyScalaCliSecondName, binDummyScalaCliSecond)
 
-        // install 1 version
-        installScalaCli(root, binDummyScalaCliFirst, force = true)
+      // install 1 version
+      installScalaCli(root, binDummyScalaCliFirst, binDirPath, force = true)
 
-        println(binDirPath / dummyScalaCliBinName)
+      println(binDirPath / dummyScalaCliBinName)
 
-        val v1Install = os.proc(binDirPath / dummyScalaCliBinName).call(
-          cwd = root,
-          stdin = os.Inherit
-        ).out.text.trim
-        expect(v1Install == firstVersion)
+      val v1Install = os.proc(binDirPath / dummyScalaCliBinName).call(
+        cwd = root,
+        stdin = os.Inherit
+      ).out.text.trim
+      expect(v1Install == firstVersion)
 
-        // update to 2 version
-        installScalaCli(root, binDummyScalaCliSecond, force = false)
+      // update to 2 version
+      installScalaCli(root, binDummyScalaCliSecond, binDirPath, force = false)
 
-        val v2Update = os.proc(binDirPath / dummyScalaCliBinName).call(
-          cwd = root,
-          stdin = os.Inherit
-        ).out.text.trim
-        expect(v2Update == secondVersion)
+      val v2Update = os.proc(binDirPath / dummyScalaCliBinName).call(
+        cwd = root,
+        stdin = os.Inherit
+      ).out.text.trim
+      expect(v2Update == secondVersion)
 
-        // downgrade to 1 version with force
-        installScalaCli(root, binDummyScalaCliFirst, force = true)
+      // downgrade to 1 version with force
+      installScalaCli(root, binDummyScalaCliFirst, binDirPath, force = true)
 
-        val v1Downgrade = os.proc(binDirPath / dummyScalaCliBinName).call(
-          cwd = root,
-          stdin = os.Inherit
-        ).out.text.trim
-        expect(v1Downgrade == firstVersion)
-      }
+      val v1Downgrade = os.proc(binDirPath / dummyScalaCliBinName).call(
+        cwd = root,
+        stdin = os.Inherit
+      ).out.text.trim
+      expect(v1Downgrade == firstVersion)
     }
   }
+
+  if (!Properties.isWin && TestUtil.isCI)
+    test("updating and downgrading dummy scala-cli using install-home command") {
+      runInstallHome()
+    }
+
 }

website/docs/reference/cli-options.md

@@ -3,20 +3,6 @@ title: Command-line options
 sidebar_position: 1
 ---
 
-## About options
-
-Available in commands:
-- [`about`](./commands#about)
-
-
-<!-- Automatically generated, DO NOT EDIT MANUALLY -->
-
-#### `--version`
-
-Aliases: `-v`
-
-Print only the scala-cli version
-
 ## Add path options
 
 Available in commands:
@@ -299,6 +285,7 @@ Available in commands:
 - [`run`](./commands#run)
 - [`setup-ide`](./commands#setup-ide)
 - [`test`](./commands#test)
+- [`version`](./commands#version)
 
 
 <!-- Automatically generated, DO NOT EDIT MANUALLY -->

website/docs/reference/commands.md

@@ -7,9 +7,6 @@ sidebar_position: 3
 
 Print details about this application
 
-Accepts options:
-- [about](./cli-options.md#about-options)
-
 ## `bsp`
 
 Accepts options:
@@ -219,6 +216,10 @@ Accepts options:
 - [test](./cli-options.md#test-options)
 - [watch](./cli-options.md#watch-options)
 
+## `version`
+
+Print scala-cli version
+
 ## Hidden commands
 
 ### `add-path`