From 21664be3f76d90efefa229a175860aa8f958b924 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Fri, 26 Jun 2020 16:42:02 -0700 Subject: [PATCH] Set terminal properties during boot Supershell does not work correctly when the sbt server is started by the remote client on windows because it incorrectly calculates the terminal dimensions. To work around this, we can pass in the dimensions from the remote client as an environment variable. I tried to do this as a system property but had all kinds of problems with windows stripping delimeters from the command. It was much easier to get working with an environment variable and should really only be set by the sbtc client anyway. --- .../scala/sbt/internal/util/Terminal.scala | 56 ++++++++++++++++--- .../sbt/internal/client/NetworkClient.scala | 26 +++++---- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/internal/util-logging/src/main/scala/sbt/internal/util/Terminal.scala b/internal/util-logging/src/main/scala/sbt/internal/util/Terminal.scala index 83c3c62f8..5972cf623 100644 --- a/internal/util-logging/src/main/scala/sbt/internal/util/Terminal.scala +++ b/internal/util-logging/src/main/scala/sbt/internal/util/Terminal.scala @@ -19,6 +19,7 @@ import sbt.internal.util.ConsoleAppender.{ ClearScreenAfterCursor, CursorLeft100 import scala.annotation.tailrec import scala.collection.mutable.ArrayBuffer +import scala.util.Try trait Terminal extends AutoCloseable { @@ -430,6 +431,38 @@ object Terminal { } } + /* + * When the server is booted by a remote client, it may not be able to accurately + * calculate the terminal properties. To work around this, we can set the + * properties via an environment property. It was too difficult to get system + * properties working correctly with windows. + */ + private class Props( + val width: Int, + val height: Int, + val ansi: Boolean, + val color: Boolean, + val supershell: Boolean + ) + private[sbt] val TERMINAL_PROPS = "SBT_TERMINAL_PROPS" + private val props = System.getenv(TERMINAL_PROPS) match { + case null => None + case p => + p.split(",") match { + case Array(width, height, ansi, color, supershell) => + Try( + new Props( + width.toInt, + height.toInt, + ansi.toBoolean, + color.toBoolean, + supershell.toBoolean + ) + ).toOption + case _ => None + } + } + /** * Creates an instance of [[Terminal]] that delegates most of its methods to an underlying * jline.Terminal2 instance. In the long run, sbt should upgrade to jline3, which has a @@ -452,9 +485,9 @@ object Terminal { override def restore(): Unit = if (alive) terminal.restore() override def reset(): Unit = if (alive) terminal.reset() override def isSupported: Boolean = terminal.isSupported - override def getWidth: Int = terminal.getWidth - override def getHeight: Int = terminal.getHeight - override def isAnsiSupported: Boolean = terminal.isAnsiSupported + override def getWidth: Int = props.map(_.width).getOrElse(terminal.getWidth) + override def getHeight: Int = props.map(_.height).getOrElse(terminal.getHeight) + override def isAnsiSupported: Boolean = props.map(_.ansi).getOrElse(terminal.isAnsiSupported) override def wrapOutIfNeeded(out: OutputStream): OutputStream = terminal.wrapOutIfNeeded(out) override def wrapInIfNeeded(in: InputStream): InputStream = terminal.wrapInIfNeeded(in) override def hasWeirdWrap: Boolean = terminal.hasWeirdWrap @@ -549,13 +582,18 @@ object Terminal { term.setEchoEnabled(true) } } - override def isColorEnabled: Boolean = ConsoleAppender.formatEnabledInEnv + override def isColorEnabled: Boolean = + props.map(_.color).getOrElse(ConsoleAppender.formatEnabledInEnv) - override def isSupershellEnabled: Boolean = System.getProperty("sbt.supershell") match { - case null => !(sys.env.contains("BUILD_NUMBER") || sys.env.contains("CI")) && isColorEnabled - case "true" => true - case _ => false - } + override def isSupershellEnabled: Boolean = + props + .map(_.supershell) + .getOrElse(System.getProperty("sbt.supershell") match { + case null => + !(sys.env.contains("BUILD_NUMBER") || sys.env.contains("CI")) && isColorEnabled + case "true" => true + case _ => false + }) } private[sbt] abstract class TerminalImpl private[sbt] ( val in: InputStream, diff --git a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala index c9d104d25..1d52eb8b7 100644 --- a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala +++ b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala @@ -189,23 +189,25 @@ class NetworkClient( */ def forkServer(portfile: File, log: Boolean): Unit = { if (log) console.appendLog(Level.Info, "server was not detected. starting an instance") - val color = - if (!arguments.sbtArguments.exists(_.startsWith("-Dsbt.color="))) - s"-Dsbt.color=${Terminal.console.isColorEnabled}" :: Nil - else Nil - val superShell = - if (!arguments.sbtArguments.exists(_.startsWith("-Dsbt.supershell="))) - s"-Dsbt.supershell=${Terminal.console.isColorEnabled}" :: Nil - else Nil + val term = Terminal.console + val props = + Seq( + term.getWidth, + term.getHeight, + term.isAnsiSupported, + term.isColorEnabled, + term.isSupershellEnabled + ).mkString(",") - val args = color ++ superShell ++ arguments.sbtArguments - val cmd = arguments.sbtScript +: args :+ BasicCommandStrings.CloseIOStreams - val process = + val cmd = arguments.sbtScript +: arguments.sbtArguments :+ BasicCommandStrings.CloseIOStreams + val processBuilder = new ProcessBuilder(cmd: _*) .directory(arguments.baseDirectory) .redirectInput(Redirect.PIPE) - .start() + processBuilder.environment.put(Terminal.TERMINAL_PROPS, props) + val process = processBuilder.start() sbtProcess.set(process) + val hook = new Thread(() => Option(sbtProcess.get).foreach(_.destroyForcibly())) Runtime.getRuntime.addShutdownHook(hook) val stdout = process.getInputStream