Refactor commandLine code a bit (#242)

* get rid of toColoredCommandLine

* remove unused run configuration parameter

* refactor AptosCommandLine

* add proper ignoreExitCode

Author: mkurnikov

src/main/kotlin/org/move/cli/runConfigurations/AptosCommandLine.kt

@@ -4,43 +4,16 @@ import com.intellij.execution.configuration.EnvironmentVariablesData
 import com.intellij.execution.configurations.GeneralCommandLine
 import com.intellij.execution.configurations.PtyCommandLine
 import com.intellij.util.execution.ParametersListUtil
+import org.move.cli.tools.MvCommandLine
 import java.nio.file.Path
 
-data class AptosCommandLine(
+class AptosCommandLine(
     val subCommand: String?,
-    val arguments: List<String> = emptyList(),
-    val workingDirectory: Path? = null,
-    val environmentVariables: EnvironmentVariablesData = EnvironmentVariablesData.DEFAULT
-) {
-    val commandLineString: String
-        get() = ParametersListUtil.join(subCommand?.split(" ").orEmpty() + arguments)
-
-    fun toGeneralCommandLine(cliExePath: Path): GeneralCommandLine {
-        val generalCommandLine = GeneralCommandLine()
-            .withExePath(cliExePath.toString())
-            // subcommand can be null
-            .withParameters(subCommand?.split(" ").orEmpty())
-            .withParameters(this.arguments)
-            .withWorkingDirectory(this.workingDirectory)
-            .withCharset(Charsets.UTF_8)
-            // disables default coloring for stderr
-            .withRedirectErrorStream(true)
-        this.environmentVariables.configureCommandLine(generalCommandLine, true)
-        return generalCommandLine
-    }
-
-    fun toColoredCommandLine(cliExePath: Path): GeneralCommandLine {
-        // preudo-tty emulation makes aptos-cli recognize console as tty and show ANSI colors
-        val generalCommandLine = PtyCommandLine()
-            .withExePath(cliExePath.toString())
-            // subcommand can be null
-            .withParameters(subCommand?.split(" ").orEmpty())
-            .withParameters(this.arguments)
-            .withWorkingDirectory(this.workingDirectory)
-            .withCharset(Charsets.UTF_8)
-            // disables default coloring for stderr
-            .withRedirectErrorStream(true)
-        this.environmentVariables.configureCommandLine(generalCommandLine, true)
-        return generalCommandLine
-    }
-}
+    arguments: List<String> = emptyList(),
+    workingDirectory: Path? = null,
+    environmentVariables: EnvironmentVariablesData = EnvironmentVariablesData.DEFAULT
+): MvCommandLine(
+    subCommand?.split(" ").orEmpty() + arguments,
+    workingDirectory,
+    environmentVariables
+)

src/main/kotlin/org/move/cli/runConfigurations/AptosRunState.kt

@@ -7,18 +7,19 @@ import com.intellij.execution.process.ProcessHandler
 import com.intellij.execution.process.ProcessTerminatedListener
 import com.intellij.execution.runners.ExecutionEnvironment
 import org.move.cli.MoveFileHyperlinkFilter
+import org.move.cli.runConfigurations.CommandConfigurationBase.CleanConfiguration
 
 abstract class AptosRunStateBase(
-    environment: ExecutionEnvironment,
-    val runConfiguration: CommandConfigurationBase,
-    val config: CommandConfigurationBase.CleanConfiguration.Ok
+    environment: ExecutionEnvironment, val cleanConfiguration: CleanConfiguration.Ok
 ): CommandLineState(environment) {
 
     val project = environment.project
-    val commandLine: AptosCommandLine = config.cmd
+    val commandLine: AptosCommandLine = cleanConfiguration.cmd
 
     override fun startProcess(): ProcessHandler {
-        val generalCommandLine = commandLine.toColoredCommandLine(config.aptosPath)
+        // emulateTerminal=true allows for the colored output
+        val generalCommandLine =
+            commandLine.toGeneralCommandLine(cleanConfiguration.aptosPath, emulateTerminal = true)
         val handler = KillableColoredProcessHandler(generalCommandLine)
         consoleBuilder.console.attachToProcess(handler)
         ProcessTerminatedListener.attach(handler)  // shows exit code upon termination
@@ -36,11 +37,8 @@ abstract class AptosRunStateBase(
 }
 
 class AptosRunState(
-    environment: ExecutionEnvironment,
-    runConfiguration: CommandConfigurationBase,
-    config: CommandConfigurationBase.CleanConfiguration.Ok
-):
-    AptosRunStateBase(environment, runConfiguration, config) {
+    environment: ExecutionEnvironment, config: CleanConfiguration.Ok
+): AptosRunStateBase(environment, config) {
 
     init {
         createFilters().forEach { consoleBuilder.addFilter(it) }

src/main/kotlin/org/move/cli/runConfigurations/CommandConfigurationBase.kt

@@ -56,9 +56,9 @@ abstract class CommandConfigurationBase(
     override fun getState(executor: Executor, environment: ExecutionEnvironment): AptosRunStateBase? {
         val config = clean().ok ?: return null
         return if (showTestToolWindow(config.cmd)) {
-            AptosTestRunState(environment, this, config)
+            AptosTestRunState(environment, config)
         } else {
-            AptosRunState(environment, this, config)
+            AptosRunState(environment, config)
         }
     }
 
@@ -79,16 +79,14 @@ abstract class CommandConfigurationBase(
                 workingDirectory,
                 environmentVariables
             )
-        return CleanConfiguration.Ok(aptosPath, commandLine)
+        return CleanConfiguration.
+        Ok(aptosPath, commandLine)
     }
 
     protected fun showTestToolWindow(commandLine: AptosCommandLine): Boolean =
         when {
             !AdvancedSettings.getBoolean(TEST_TOOL_WINDOW_SETTING_KEY) -> false
             commandLine.subCommand != "move test" -> false
-//            "--nocapture" in commandLine.additionalArguments -> false
-//            Cargo.TEST_NOCAPTURE_ENABLED_KEY.asBoolean() -> false
-//            else -> !hasRemoteTarget
             else -> true
         }
 

src/main/kotlin/org/move/cli/runConfigurations/aptos/Aptos.kt

@@ -178,11 +178,8 @@ data class Aptos(val cliLocation: Path, val parentDisposable: Disposable?): Disp
         listener: ProcessListener? = null,
         runner: CapturingProcessHandler.() -> ProcessOutput = { runProcessWithGlobalProgress() }
     ): RsProcessResult<ProcessOutput> {
-        val generalCommandLine = if (colored) {
-            commandLine.toColoredCommandLine(this.cliLocation)
-        } else {
-            commandLine.toGeneralCommandLine(this.cliLocation)
-        }
+        val generalCommandLine =
+            commandLine.toGeneralCommandLine(this.cliLocation, emulateTerminal = colored)
         return generalCommandLine.execute(innerDisposable, runner, listener)
     }
 
@@ -193,11 +190,9 @@ data class Aptos(val cliLocation: Path, val parentDisposable: Disposable?): Disp
         runner: CapturingProcessHandler.() -> ProcessOutput = { runProcessWithGlobalProgress() }
     ): AptosProcessResult<Unit> {
         val processOutput = executeCommandLine(commandLine, colored, listener, runner)
+            .ignoreNonZeroExitCode()
             .unwrapOrElse {
-                if (it !is RsProcessExecutionException.FailedWithNonZeroExitCode) {
-                    return Err(it)
-                }
-                it.output
+                return Err(it)
             }
 
         val json = processOutput.stdout

src/main/kotlin/org/move/cli/runConfigurations/test/AptosTestRunState.kt

@@ -8,9 +8,8 @@ import org.move.cli.runConfigurations.aptos.cmd.AptosCommandConfiguration
 
 class AptosTestRunState(
     environment: ExecutionEnvironment,
-    runConfiguration: CommandConfigurationBase,
-    config: CommandConfigurationBase.CleanConfiguration.Ok
-): AptosRunStateBase(environment, runConfiguration, config) {
+    cleanConfiguration: CommandConfigurationBase.CleanConfiguration.Ok
+): AptosRunStateBase(environment, cleanConfiguration) {
 
     init {
         consoleBuilder =

src/main/kotlin/org/move/cli/tools/MvCommandLine.kt

@@ -0,0 +1,30 @@
+package org.move.cli.tools
+
+import com.intellij.execution.configuration.EnvironmentVariablesData
+import com.intellij.execution.configurations.GeneralCommandLine
+import com.intellij.execution.configurations.PtyCommandLine
+import com.intellij.util.execution.ParametersListUtil
+import java.nio.file.Path
+
+open class MvCommandLine(
+    val arguments: List<String> = emptyList(),
+    val workingDirectory: Path? = null,
+    val environmentVariables: EnvironmentVariablesData = EnvironmentVariablesData.DEFAULT
+) {
+    val commandLineString: String get() = ParametersListUtil.join(arguments)
+
+    fun toGeneralCommandLine(cliExePath: Path, emulateTerminal: Boolean = false): GeneralCommandLine {
+        var commandLine = GeneralCommandLine()
+            .withExePath(cliExePath.toString())
+            .withParameters(this.arguments)
+            .withWorkingDirectory(this.workingDirectory)
+            .withCharset(Charsets.UTF_8)
+            // disables default coloring for stderr
+            .withRedirectErrorStream(true)
+        this.environmentVariables.configureCommandLine(commandLine, true)
+        if (emulateTerminal) {
+            commandLine = PtyCommandLine(commandLine)
+        }
+        return commandLine
+    }
+}

src/main/kotlin/org/move/openapiext/MvProcessResult.kt

@@ -12,41 +12,41 @@ import org.move.stdext.RsResult
 
 typealias RsProcessResult<T> = RsResult<T, RsProcessExecutionException>
 
-sealed class RsProcessExecutionOrDeserializationException : RuntimeException {
-    constructor(cause: Throwable) : super(cause)
-    constructor(message: String) : super(message)
+sealed class RsProcessExecutionOrDeserializationException: RuntimeException {
+    constructor(cause: Throwable): super(cause)
+    constructor(message: String): super(message)
 }
 
-class RsDeserializationException(cause: JacksonException) :
+class RsDeserializationException(cause: JacksonException):
     RsProcessExecutionOrDeserializationException(cause)
 
-sealed class RsProcessExecutionException : RsProcessExecutionOrDeserializationException {
-    constructor(message: String) : super(message)
-    constructor(cause: Throwable) : super(cause)
+sealed class RsProcessExecutionException: RsProcessExecutionOrDeserializationException {
+    constructor(message: String): super(message)
+    constructor(cause: Throwable): super(cause)
 
     abstract val commandLineString: String
 
     class Start(
         override val commandLineString: String,
         cause: ExecutionException,
-    ) : RsProcessExecutionException(cause)
+    ): RsProcessExecutionException(cause)
 
     class Canceled(
         override val commandLineString: String,
         val output: ProcessOutput,
         message: String = errorMessage(commandLineString, output),
-    ) : RsProcessExecutionException(message)
+    ): RsProcessExecutionException(message)
 
     class Timeout(
         override val commandLineString: String,
         val output: ProcessOutput,
-    ) : RsProcessExecutionException(errorMessage(commandLineString, output))
+    ): RsProcessExecutionException(errorMessage(commandLineString, output))
 
     /** The process exited with non-zero exit code */
     class FailedWithNonZeroExitCode(
         override val commandLineString: String,
         val output: ProcessOutput,
-    ) : RsProcessExecutionException(errorMessage(commandLineString, output))
+    ): RsProcessExecutionException(errorMessage(commandLineString, output))
 
     companion object {
         fun errorMessage(commandLineString: String, output: ProcessOutput): String = """
@@ -68,3 +68,12 @@ fun RsProcessResult<ProcessOutput>.ignoreExitCode(): RsResult<ProcessOutput, RsP
             is RsProcessExecutionException.FailedWithNonZeroExitCode -> RsResult.Ok(err.output)
         }
     }
+
+fun RsProcessResult<ProcessOutput>.ignoreNonZeroExitCode(): RsResult<ProcessOutput, RsProcessExecutionException> =
+    when (this) {
+        is RsResult.Ok -> this
+        is RsResult.Err -> when (err) {
+            is RsProcessExecutionException.FailedWithNonZeroExitCode -> RsResult.Ok(err.output)
+            else -> this
+        }
+    }