From 78620cd9026e582116f43336c2f67ed5a89dbe11 Mon Sep 17 00:00:00 2001 From: Ethan Atkins Date: Tue, 20 Oct 2020 11:39:08 -0700 Subject: [PATCH] Manage ansi codes and color codes separately The ConsoleAppender formatEnabledInEnv field was being used both as an indicator that ansi codes were supported and that color codes are enabled. There are cases in which general ansi codes are not supported but color codes are and these use cases need to be handled separately. To make things more explicit, this commit adds isColorEnabled and isAnsiSupported to the Terminal companion object so that we can be more specific about what the requirements are (general ansi escape codes or just colors). There are a few cases in ConsoleAppender itself where formatEnabledInEnv was used to set flags for both color and ansi codes. When that is the case, we use Terminal.isAnsiSupported because when that is true, colors should at least work but there are terminals that support color but not general ansi escape codes. --- .../sbt/internal/util/ConsoleAppender.scala | 17 ++++++++++------- .../scala/sbt/internal/util/ConsoleOut.scala | 4 ++-- .../main/scala/sbt/internal/util/JLine3.scala | 2 +- .../scala/sbt/internal/util/MainLogging.scala | 4 ++-- .../scala/sbt/internal/util/Terminal.scala | 19 +++++++++++-------- .../src/main/scala/sbt/ForkTests.scala | 4 ++-- .../src/main/scala/sbt/Highlight.scala | 6 +++--- main-settings/src/main/scala/sbt/Def.scala | 4 ++-- .../main/scala/sbt/std/TaskLinterDSL.scala | 10 +++++----- main/src/main/scala/sbt/EvaluateTask.scala | 4 ++-- main/src/main/scala/sbt/Main.scala | 2 +- .../main/scala/sbt/internal/LogManager.scala | 4 ++-- .../src/main/scala/sbt/internal/SysProp.scala | 4 ++-- .../sbt/internal/testing/TestLogger.scala | 4 ++-- 14 files changed, 47 insertions(+), 41 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 fcf092f81..c248eb484 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 @@ -64,8 +64,8 @@ object ConsoleLogger { */ def apply( out: ConsoleOut = ConsoleOut.systemOut, - ansiCodesSupported: Boolean = ConsoleAppender.formatEnabledInEnv, - useFormat: Boolean = ConsoleAppender.formatEnabledInEnv, + ansiCodesSupported: Boolean = Terminal.isAnsiSupported, + useFormat: Boolean = Terminal.isColorEnabled, suppressedMessage: SuppressedTraceContext => Option[String] = ConsoleAppender.noSuppressedMessage ): ConsoleLogger = @@ -148,7 +148,8 @@ object ConsoleAppender { * 3. -Dsbt.colour=always/auto/never/true/false * 4. -Dsbt.log.format=always/auto/never/true/false */ - lazy val formatEnabledInEnv: Boolean = Terminal.formatEnabledInEnv + @deprecated("Use Terminal.isAnsiSupported or Terminal.isColorEnabled", "1.4.0") + lazy val formatEnabledInEnv: Boolean = Terminal.isAnsiSupported private[sbt] def parseLogOption(s: String): LogOption = Terminal.parseLogOption(s) match { case Some(true) => LogOption.Always @@ -204,7 +205,7 @@ object ConsoleAppender { * @param out Where to write messages. * @return A new `ConsoleAppender` that writes to `out`. */ - def apply(name: String, out: ConsoleOut): Appender = apply(name, out, formatEnabledInEnv) + def apply(name: String, out: ConsoleOut): Appender = apply(name, out, Terminal.isAnsiSupported) /** * A new `ConsoleAppender` identified by `name`, and that writes to `out`. @@ -218,8 +219,10 @@ object ConsoleAppender { name: String, out: ConsoleOut, suppressedMessage: SuppressedTraceContext => Option[String] - ): Appender = - apply(name, out, formatEnabledInEnv, formatEnabledInEnv, suppressedMessage) + ): Appender = { + val ansi = Terminal.isAnsiSupported + apply(name, out, ansi, ansi, suppressedMessage) + } /** * A new `ConsoleAppender` identified by `name`, and that writes to `out`. @@ -230,7 +233,7 @@ object ConsoleAppender { * @return A new `ConsoleAppender` that writes to `out`. */ def apply(name: String, out: ConsoleOut, useFormat: Boolean): Appender = - apply(name, out, useFormat || formatEnabledInEnv, useFormat, noSuppressedMessage) + apply(name, out, useFormat || Terminal.isAnsiSupported, useFormat, noSuppressedMessage) /** * A new `ConsoleAppender` identified by `name`, and that writes to `out`. diff --git a/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleOut.scala b/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleOut.scala index 1f3e8e273..b946a0cc8 100644 --- a/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleOut.scala +++ b/internal/util-logging/src/main/scala/sbt/internal/util/ConsoleOut.scala @@ -64,7 +64,7 @@ object ConsoleOut { def println(s: String): Unit = synchronized { current.append(s); println() } def println(): Unit = synchronized { val s = current.toString - if (ConsoleAppender.formatEnabledInEnv && last.exists(lmsg => f(s, lmsg))) + if (Terminal.isAnsiSupported && last.exists(lmsg => f(s, lmsg))) lockObject.print(OverwriteLine) lockObject.println(s) last = Some(s) @@ -72,7 +72,7 @@ object ConsoleOut { } def flush(): Unit = synchronized { val s = current.toString - if (ConsoleAppender.formatEnabledInEnv && last.exists(lmsg => f(s, lmsg))) + if (Terminal.isAnsiSupported && last.exists(lmsg => f(s, lmsg))) lockObject.print(OverwriteLine) lockObject.print(s) last = Some(s) diff --git a/internal/util-logging/src/main/scala/sbt/internal/util/JLine3.scala b/internal/util-logging/src/main/scala/sbt/internal/util/JLine3.scala index d3c4bfd41..f74326eaa 100644 --- a/internal/util-logging/src/main/scala/sbt/internal/util/JLine3.scala +++ b/internal/util-logging/src/main/scala/sbt/internal/util/JLine3.scala @@ -73,7 +73,7 @@ private[sbt] object JLine3 { term } private[sbt] def apply(term: Terminal): JTerminal = { - if (System.getProperty("jline.terminal", "") == "none" || !Terminal.formatEnabledInEnv) + if (System.getProperty("jline.terminal", "") == "none" || !Terminal.isAnsiSupported) new DumbTerminal(term.inputStream, term.outputStream) else wrapTerminal(term) } diff --git a/internal/util-logging/src/main/scala/sbt/internal/util/MainLogging.scala b/internal/util-logging/src/main/scala/sbt/internal/util/MainLogging.scala index d5573b6dc..db5d2e25e 100644 --- a/internal/util-logging/src/main/scala/sbt/internal/util/MainLogging.scala +++ b/internal/util-logging/src/main/scala/sbt/internal/util/MainLogging.scala @@ -88,10 +88,10 @@ object MainAppender { ConsoleAppender(name, console, suppressedMessage = suppressedMessage) def defaultBacked: PrintWriter => Appender = - defaultBacked(generateGlobalBackingName, ConsoleAppender.formatEnabledInEnv) + defaultBacked(generateGlobalBackingName, Terminal.isAnsiSupported) def defaultBacked(loggerName: String): PrintWriter => Appender = - defaultBacked(loggerName, ConsoleAppender.formatEnabledInEnv) + defaultBacked(loggerName, Terminal.isAnsiSupported) def defaultBacked(useFormat: Boolean): PrintWriter => Appender = defaultBacked(generateGlobalBackingName, useFormat) 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 03ed8290c..775cb1f07 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,7 +306,8 @@ object Terminal { case _ => sys.props.get("sbt.log.format").flatMap(parseLogOption) } } - private[sbt] lazy val formatEnabledInEnv: Boolean = logFormatEnabled.getOrElse(useColorDefault) + private[sbt] lazy val isAnsiSupported: Boolean = + logFormatEnabled.getOrElse(useColorDefault && !isCI) private[this] val isDumbTerminal = "dumb" == System.getenv("TERM") private[this] val hasConsole = Option(java.lang.System.console).isDefined private[this] def useColorDefault: Boolean = { @@ -316,9 +317,10 @@ object Terminal { } private[this] lazy val isColorEnabledProp: Option[Boolean] = sys.props.get("sbt.color").orElse(sys.props.get("sbt.colour")).flatMap(parseLogOption) + private[sbt] lazy val isColorEnabled = useColorDefault private[sbt] def red(str: String, doRed: Boolean): String = - if (formatEnabledInEnv && doRed) Console.RED + str + Console.RESET + if (isColorEnabled && doRed) Console.RED + str + Console.RESET else str /** @@ -331,7 +333,7 @@ object Terminal { private[sbt] def withStreams[T](isServer: Boolean)(f: => T): T = // In ci environments, don't touch the io streams unless run with -Dsbt.io.virtual=true if (System.getProperty("sbt.io.virtual", "") == "true" || (logFormatEnabled.getOrElse(true) && !isCI)) { - hasProgress.set(isServer && formatEnabledInEnv) + hasProgress.set(isServer && isAnsiSupported) consoleTerminalHolder.set(newConsoleTerminal()) activeTerminal.set(consoleTerminalHolder.get) try withOut(withIn(f)) @@ -745,7 +747,7 @@ object Terminal { private[this] def fixTerminalProperty(): Unit = { val terminalProperty = "jline.terminal" val newValue = - if (!formatEnabledInEnv) "none" + if (!isAnsiSupported) "none" else System.getProperty(terminalProperty) match { case "jline.UnixTerminal" => "unix" @@ -794,7 +796,8 @@ object Terminal { val size = system.getSize (size.getColumns, size.getRows) } - override lazy val isAnsiSupported: Boolean = !isDumbTerminal && formatEnabledInEnv && !isCI + override lazy val isAnsiSupported: Boolean = + !isDumbTerminal && Terminal.isAnsiSupported && !isCI override private[sbt] def progressState: ProgressState = consoleProgressState.get override def isEchoEnabled: Boolean = try system.echo() @@ -839,7 +842,7 @@ object Terminal { override def isColorEnabled: Boolean = props .map(_.color) - .getOrElse(isColorEnabledProp.getOrElse(formatEnabledInEnv)) + .getOrElse(isColorEnabledProp.getOrElse(Terminal.isColorEnabled)) override def isSupershellEnabled: Boolean = props @@ -963,8 +966,8 @@ object Terminal { override def getStringCapability(capability: String): String = null override def getWidth: Int = 0 override def inputStream: InputStream = nullInputStream - override def isAnsiSupported: Boolean = formatEnabledInEnv - override def isColorEnabled: Boolean = isColorEnabledProp.getOrElse(formatEnabledInEnv) + override def isAnsiSupported: Boolean = Terminal.isAnsiSupported + override def isColorEnabled: Boolean = isColorEnabledProp.getOrElse(Terminal.isColorEnabled) override def isEchoEnabled: Boolean = false override def isSuccessEnabled: Boolean = true override def isSupershellEnabled: Boolean = false diff --git a/main-actions/src/main/scala/sbt/ForkTests.scala b/main-actions/src/main/scala/sbt/ForkTests.scala index 39a9c35c3..b342e03f1 100755 --- a/main-actions/src/main/scala/sbt/ForkTests.scala +++ b/main-actions/src/main/scala/sbt/ForkTests.scala @@ -18,7 +18,7 @@ import sbt.util.Logger import sbt.ConcurrentRestrictions.Tag import sbt.protocol.testing._ import sbt.internal.util.Util.{ AnyOps, none } -import sbt.internal.util.{ ConsoleAppender, RunningProcesses } +import sbt.internal.util.{ RunningProcesses, Terminal } private[sbt] object ForkTests { def apply( @@ -97,7 +97,7 @@ private[sbt] object ForkTests { val is = new ObjectInputStream(socket.getInputStream) try { - val config = new ForkConfiguration(ConsoleAppender.formatEnabledInEnv, parallel) + val config = new ForkConfiguration(Terminal.isAnsiSupported, parallel) os.writeObject(config) val taskdefs = opts.tests.map { t => diff --git a/main-command/src/main/scala/sbt/Highlight.scala b/main-command/src/main/scala/sbt/Highlight.scala index 3163fda28..140ad554a 100644 --- a/main-command/src/main/scala/sbt/Highlight.scala +++ b/main-command/src/main/scala/sbt/Highlight.scala @@ -10,13 +10,13 @@ package sbt import java.util.regex.Pattern import scala.Console.{ BOLD, RESET } -import sbt.internal.util.ConsoleAppender +import sbt.internal.util.Terminal object Highlight { def showMatches(pattern: Pattern)(line: String): Option[String] = { val matcher = pattern.matcher(line) - if (ConsoleAppender.formatEnabledInEnv) { + if (Terminal.isColorEnabled) { // ANSI codes like \033[39m (normal text color) don't work on Windows val highlighted = matcher.replaceAll(scala.Console.RED + "$0" + RESET) if (highlighted == line) None else Some(highlighted) @@ -26,5 +26,5 @@ object Highlight { None } def bold(s: String) = - if (ConsoleAppender.formatEnabledInEnv) BOLD + s.replace(RESET, RESET + BOLD) + RESET else s + if (Terminal.isColorEnabled) BOLD + s.replace(RESET, RESET + BOLD) + RESET else s } diff --git a/main-settings/src/main/scala/sbt/Def.scala b/main-settings/src/main/scala/sbt/Def.scala index 68af91992..df78134ea 100644 --- a/main-settings/src/main/scala/sbt/Def.scala +++ b/main-settings/src/main/scala/sbt/Def.scala @@ -15,7 +15,7 @@ import sbt.KeyRanks.{ DTask, Invisible } import sbt.Scope.{ GlobalScope, ThisScope } import sbt.internal.util.Types.const import sbt.internal.util.complete.Parser -import sbt.internal.util._ +import sbt.internal.util.{ Terminal => ITerminal, _ } import Util._ import sbt.util.Show import xsbti.VirtualFile @@ -173,7 +173,7 @@ object Def extends Init[Scope] with TaskMacroExtra with InitializeImplicits { Scope.displayMasked(scoped.scope, scoped.key.label, mask, showZeroConfig) def withColor(s: String, color: Option[String]): String = - withColor(s, color, useColor = ConsoleAppender.formatEnabledInEnv) + withColor(s, color, useColor = ITerminal.isColorEnabled) def withColor(s: String, color: Option[String], useColor: Boolean): String = color match { case Some(c) if useColor => c + s + scala.Console.RESET case _ => s diff --git a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala index db2cfe204..6a9b78aa7 100644 --- a/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala +++ b/main-settings/src/main/scala/sbt/std/TaskLinterDSL.scala @@ -10,7 +10,7 @@ package sbt.std import sbt.SettingKey import sbt.dsl.LinterLevel import sbt.dsl.LinterLevel.{ Abort, Warn } -import sbt.internal.util.ConsoleAppender +import sbt.internal.util.Terminal import sbt.internal.util.appmacro.{ Convert, LinterDSL } import scala.io.AnsiColor @@ -191,10 +191,10 @@ object OnlyTaskDynLinterDSL extends BaseTaskLinterDSL { } object TaskLinterDSLFeedback { - private final val startBold = if (ConsoleAppender.formatEnabledInEnv) AnsiColor.BOLD else "" - private final val startRed = if (ConsoleAppender.formatEnabledInEnv) AnsiColor.RED else "" - private final val startGreen = if (ConsoleAppender.formatEnabledInEnv) AnsiColor.GREEN else "" - private final val reset = if (ConsoleAppender.formatEnabledInEnv) AnsiColor.RESET else "" + private final val startBold = if (Terminal.isColorEnabled) AnsiColor.BOLD else "" + private final val startRed = if (Terminal.isColorEnabled) AnsiColor.RED else "" + private final val startGreen = if (Terminal.isColorEnabled) AnsiColor.GREEN else "" + private final val reset = if (Terminal.isColorEnabled) AnsiColor.RESET else "" private final val ProblemHeader = s"${startRed}problem$reset" private final val SolutionHeader = s"${startGreen}solution$reset" diff --git a/main/src/main/scala/sbt/EvaluateTask.scala b/main/src/main/scala/sbt/EvaluateTask.scala index 4ff043af3..cc3c4dc26 100644 --- a/main/src/main/scala/sbt/EvaluateTask.scala +++ b/main/src/main/scala/sbt/EvaluateTask.scala @@ -17,7 +17,7 @@ import sbt.Scope.Global import sbt.internal.Aggregation.KeyValue import sbt.internal.TaskName._ import sbt.internal._ -import sbt.internal.util._ +import sbt.internal.util.{ Terminal => ITerminal, _ } import sbt.librarymanagement.{ Resolver, UpdateReport } import sbt.std.Transform.DummyTaskMap import sbt.util.{ Logger, Show } @@ -368,7 +368,7 @@ object EvaluateTask { for ((key, msg, ex) <- keyed if (msg.isDefined || ex.isDefined)) { val msgString = (msg.toList ++ ex.toList.map(ErrorHandling.reducedToString)).mkString("\n\t") val log = getStreams(key, streams).log - val display = contextDisplay(state, ConsoleAppender.formatEnabledInEnv) + val display = contextDisplay(state, ITerminal.isColorEnabled) log.error("(" + display.show(key) + ") " + msgString) } } diff --git a/main/src/main/scala/sbt/Main.scala b/main/src/main/scala/sbt/Main.scala index 8f1617c02..26fa0b00b 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/main/src/main/scala/sbt/Main.scala @@ -105,7 +105,7 @@ private[sbt] object xMain { } finally { // Clear any stray progress lines ShutdownHooks.close() - if (ITerminal.formatEnabledInEnv) { + if (ITerminal.isAnsiSupported) { System.out.print(ConsoleAppender.ClearScreenAfterCursor) System.out.flush() } diff --git a/main/src/main/scala/sbt/internal/LogManager.scala b/main/src/main/scala/sbt/internal/LogManager.scala index 315989bfa..b643b277e 100644 --- a/main/src/main/scala/sbt/internal/LogManager.scala +++ b/main/src/main/scala/sbt/internal/LogManager.scala @@ -14,7 +14,7 @@ import sbt.Def.ScopedKey import sbt.Keys._ import sbt.Scope.GlobalScope import sbt.internal.util.MainAppender._ -import sbt.internal.util._ +import sbt.internal.util.{ Terminal => ITerminal, _ } import sbt.util.{ Level, LogExchange, Logger, LoggerContext } import org.apache.logging.log4j.core.{ Appender => XAppender } @@ -319,7 +319,7 @@ object LogManager { private[this] def slog: Logger = Option(ref.get) getOrElse sys.error("Settings logger used after project was loaded.") - override val ansiCodesSupported = ConsoleAppender.formatEnabledInEnv + override val ansiCodesSupported = ITerminal.isAnsiSupported override def trace(t: => Throwable) = slog.trace(t) override def success(message: => String) = slog.success(message) override def log(level: Level.Value, message: => String) = slog.log(level, message) diff --git a/main/src/main/scala/sbt/internal/SysProp.scala b/main/src/main/scala/sbt/internal/SysProp.scala index 51d7a1d6e..9c9d99dfb 100644 --- a/main/src/main/scala/sbt/internal/SysProp.scala +++ b/main/src/main/scala/sbt/internal/SysProp.scala @@ -12,7 +12,7 @@ import java.util.Locale import scala.util.control.NonFatal import scala.concurrent.duration._ -import sbt.internal.util.ConsoleAppender +import sbt.internal.util.{ Terminal => ITerminal } import sbt.internal.util.complete.SizeParser // See also BuildPaths.scala @@ -106,7 +106,7 @@ object SysProp { * 3. -Dsbt.colour=always/auto/never/true/false * 4. -Dsbt.log.format=always/auto/never/true/false */ - lazy val color: Boolean = ConsoleAppender.formatEnabledInEnv + lazy val color: Boolean = ITerminal.isColorEnabled def closeClassLoaders: Boolean = getOrFalse("sbt.classloader.close") diff --git a/testing/src/main/scala/sbt/internal/testing/TestLogger.scala b/testing/src/main/scala/sbt/internal/testing/TestLogger.scala index 0b094eaad..86ca21de2 100644 --- a/testing/src/main/scala/sbt/internal/testing/TestLogger.scala +++ b/testing/src/main/scala/sbt/internal/testing/TestLogger.scala @@ -9,7 +9,7 @@ package sbt package internal.testing import testing.{ Logger => TLogger } -import sbt.internal.util.{ BufferedAppender, ConsoleAppender, ManagedLogger } +import sbt.internal.util.{ BufferedAppender, ManagedLogger, Terminal } import sbt.util.{ Level, ShowLines } import sbt.protocol.testing._ import java.util.concurrent.atomic.AtomicInteger @@ -96,7 +96,7 @@ object TestLogger { def debug(s: String) = log(Level.Debug, TestStringEvent(s)) def trace(t: Throwable) = logger.trace(t) private def log(level: Level.Value, event: TestStringEvent) = logger.logEvent(level, event) - def ansiCodesSupported() = ConsoleAppender.formatEnabledInEnv + def ansiCodesSupported() = Terminal.isAnsiSupported } private[sbt] def toTestItemEvent(event: TestEvent): TestItemEvent =