From 3b09ff6af7c4e108152e8dfdb10f6a1f05c4ab34 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Wed, 4 Nov 2020 13:25:52 -0800 Subject: [PATCH 1/3] Support ansi by default if color is supported Scalatest will check the ansiCodesSupported value of a console appender to decide whether or not to colorize its output. We were setting isAnsiSupported to false by default in ci to prevent the ConsoleAppender from adding ansi codes to the output. The only place though where we were doing that was in adding a ClearScreenAfterCursor to the end of each log line. This was done for supershell but there is actually other logic in supershell processing that adds these anyway. --- .../src/main/scala/sbt/internal/util/ConsoleAppender.scala | 6 ++---- .../src/main/scala/sbt/internal/util/Terminal.scala | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleAppender.scala b/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleAppender.scala index 55b0fe5d8..7cfed50e6 100644 --- a/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleAppender.scala +++ b/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleAppender.scala @@ -460,8 +460,7 @@ trait Appender extends AutoCloseable { // according to https://github.com/sbt/sbt/issues/5608, sometimes we get a null message if (message == null) () else { - val len = - labelColor.length + label.length + messageColor.length + reset.length * 3 + ClearScreenAfterCursor.length + val len = labelColor.length + label.length + messageColor.length + reset.length * 3 val builder: StringBuilder = new StringBuilder(len) message.linesIterator.foreach { line => builder.ensureCapacity(len + line.length + 4) @@ -477,7 +476,6 @@ trait Appender extends AutoCloseable { fmted(labelColor, label) builder.append("] ") fmted(messageColor, line) - if (ansiCodesSupported) builder.append(ClearScreenAfterCursor) write(builder.toString) } } @@ -495,7 +493,7 @@ trait Appender extends AutoCloseable { // the output may have unwanted colors but it would still be legible. This should // only be relevant if the log message string itself contains ansi escape sequences // other than color codes which is very unlikely. - val toWrite = if (!ansiCodesSupported || !useFormat && msg.getBytes.contains(27.toByte)) { + val toWrite = if ((!ansiCodesSupported || !useFormat) && msg.getBytes.contains(27.toByte)) { val (bytes, len) = EscHelpers.strip(msg.getBytes, stripAnsi = !ansiCodesSupported, stripColor = !useFormat) new String(bytes, 0, len) 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 e037be9f1..8c4644b32 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 @@ -308,7 +308,7 @@ object Terminal { } private[this] lazy val superShellEnabled = sys.props.get("sbt.supershell").map(_ == "true") private[sbt] lazy val isAnsiSupported: Boolean = - logFormatEnabled.orElse(superShellEnabled).getOrElse(useColorDefault && !isCI) + logFormatEnabled.orElse(superShellEnabled).getOrElse(useColorDefault) private[this] val isDumbTerminal = "dumb" == System.getenv("TERM") private[this] val hasConsole = Option(java.lang.System.console).isDefined private[this] def useColorDefault: Boolean = { From 7e1384608ea021ab7e4ff8aec71b33d1b5561d61 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Wed, 4 Nov 2020 15:44:14 -0800 Subject: [PATCH 2/3] Fix console when supershell is disabled The sbt console didn't work with supershell disabled because setting that parameter was causing the terminal type to be dumb which only works in some very specific situations. When Terminal.isAnsiSupported was false, we were setting sbt to use a dumb terminal. We really only want to use a dumb terminal if virtual io is off. It also doesn't necessarily make sense to automatically disable general ansi codes even if supershell is disabled by system property. --- .../src/main/scala/sbt/internal/util/Terminal.scala | 6 ++---- server-test/src/test/scala/testpkg/TestServer.scala | 2 +- 2 files changed, 3 insertions(+), 5 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 8c4644b32..d4adac080 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 @@ -306,9 +306,7 @@ object Terminal { case _ => sys.props.get("sbt.log.format").flatMap(parseLogOption) } } - private[this] lazy val superShellEnabled = sys.props.get("sbt.supershell").map(_ == "true") - private[sbt] lazy val isAnsiSupported: Boolean = - logFormatEnabled.orElse(superShellEnabled).getOrElse(useColorDefault) + private[sbt] lazy val isAnsiSupported: Boolean = logFormatEnabled.getOrElse(useColorDefault) private[this] val isDumbTerminal = "dumb" == System.getenv("TERM") private[this] val hasConsole = Option(java.lang.System.console).isDefined private[this] def useColorDefault: Boolean = { @@ -754,7 +752,7 @@ object Terminal { private[this] def fixTerminalProperty(): Unit = { val terminalProperty = "jline.terminal" val newValue = - if (!isAnsiSupported) "none" + if (!isAnsiSupported && System.getProperty("sbt.io.virtual", "") == "false") "none" else System.getProperty(terminalProperty) match { case "jline.UnixTerminal" => "unix" diff --git a/server-test/src/test/scala/testpkg/TestServer.scala b/server-test/src/test/scala/testpkg/TestServer.scala index a5595c09e..b2c6a2f62 100644 --- a/server-test/src/test/scala/testpkg/TestServer.scala +++ b/server-test/src/test/scala/testpkg/TestServer.scala @@ -163,7 +163,7 @@ case class TestServer( val forkOptions = ForkOptions() .withOutputStrategy(OutputStrategy.StdoutOutput) - .withRunJVMOptions(Vector("-Dsbt.ci=true", "-Dsbt.io.virtual=false")) + .withRunJVMOptions(Vector("-Djline.terminal=none", "-Dsbt.io.virtual=false")) val process = RunFromSourceMain.fork(forkOptions, baseDirectory, scalaVersion, sbtVersion, classpath) From afbceb062fa4e6e777603abe548df28f27384995 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 3 Nov 2020 08:42:04 -0800 Subject: [PATCH 3/3] Only create console terminal if process has console Intellij import was broken in sbt 1.4.2 because we increased the scenarios in which virtual io is used. The problem wasn't actually virtual io but that we create a console terminal with jline3 and that could cause issues reading input from System.in. This can be fixed by only creating an instance of ConsoleTerminal if a console is actually available for the sbt process. When hasConsole is false, the consoleTerminalHolder will point to the SimpleTerminal whose inputStream method just reads directly from System.in. After making this change, I verified that intellij import worked again on windows. We also don't want to make a console terminal if it is a dumb terminal because jline 3 ends up having problems with the ConsoleTerminal. --- .../src/main/scala/sbt/internal/util/Terminal.scala | 10 ++++++---- 1 file changed, 6 insertions(+), 4 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 d4adac080..9d9d0cc2e 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 @@ -307,7 +307,9 @@ object Terminal { } } private[sbt] lazy val isAnsiSupported: Boolean = logFormatEnabled.getOrElse(useColorDefault) - private[this] val isDumbTerminal = "dumb" == System.getenv("TERM") + + private[this] val isDumb = "dumb" == System.getenv("TERM") + private[this] def isDumbTerminal = isDumb || System.getProperty("jline.terminal", "") == "none" private[this] val hasConsole = Option(java.lang.System.console).isDefined private[this] def useColorDefault: Boolean = { // This approximates that both stdin and stdio are connected, @@ -336,7 +338,7 @@ object Terminal { // In ci environments, don't touch the io streams unless run with -Dsbt.io.virtual=true if (System.getProperty("sbt.io.virtual", "") == "true" || !isCI) { hasProgress.set(isServer && isAnsiSupported) - consoleTerminalHolder.set(newConsoleTerminal()) + if (hasConsole && !isDumbTerminal) consoleTerminalHolder.set(newConsoleTerminal()) activeTerminal.set(consoleTerminalHolder.get) try withOut(withIn(f)) finally { @@ -744,7 +746,7 @@ object Terminal { private[sbt] def reset(): Unit = { jline.TerminalFactory.reset() console.close() - consoleTerminalHolder.set(newConsoleTerminal()) + if (hasConsole && !isDumbTerminal) consoleTerminalHolder.set(newConsoleTerminal()) } // translate explicit class names to type in order to support @@ -760,7 +762,7 @@ object Terminal { case "jline.WindowsTerminal" => "windows" case "jline.AnsiWindowsTerminal" => "windows" case "jline.UnsupportedTerminal" => "none" - case null if isDumbTerminal => "none" + case null if isDumb => "none" case x => x } if (newValue != null) {