From 29d9c14edf47170213e0432e90aa67b062375cf1 Mon Sep 17 00:00:00 2001
From: Ethan Atkins <contact@ethanatkins.com>
Date: Tue, 20 Oct 2020 11:21:41 -0700
Subject: [PATCH] Improve log formatting logic for ci use cases

It is currently the case that all tagged log messages going through a
ConsoleAppender have color codes added and then subsequently stripped
out. This didn't work well in combination with -Dsbt.color=true and
-Dsbt.ci=true because the -Dsbt.color=true was causing
ConsoleAppender.formatEnabledInEnv to be set to true which caused ansi
codes to be enabled. Travis CI supports color codes but not other ansi
escape sequences (this is also often true of embedded shells like in
intellij or jedit). This commit reworks the ConsoleAppender so that we
only add colors if useFormat is enable dand only add
ClearScreenAfterCursor if ansi codes are supported. It also reworks the
ConsoleAppender.write method so that if useFormat is true but
ansiCodesSupported is false that we only remove the non color related
ansi escape sequences so that colors remain.
---
 .../sbt/internal/util/ConsoleAppender.scala   | 32 +++++++++++--------
 1 file changed, 18 insertions(+), 14 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 56a48a612..fcf092f81 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
@@ -368,14 +368,7 @@ trait Appender extends AutoCloseable {
   private[util] def ansiCodesSupported: Boolean = properties.isAnsiSupported
   private[util] def useFormat: Boolean = properties.isColorEnabled
 
-  private def reset: String = {
-    if (ansiCodesSupported && useFormat) scala.Console.RESET
-    else ""
-  }
-  private def clearScreenAfterCursor: String = {
-    if (ansiCodesSupported && useFormat) ClearScreenAfterCursor
-    else ""
-  }
+  private def reset: String = scala.Console.RESET
 
   private val SUCCESS_LABEL_COLOR = GREEN
   private val SUCCESS_MESSAGE_COLOR = reset
@@ -465,19 +458,23 @@ trait Appender extends AutoCloseable {
       if (message == null) ()
       else {
         val len =
-          labelColor.length + label.length + messageColor.length + reset.length * 3 + clearScreenAfterCursor.length
+          labelColor.length + label.length + messageColor.length + reset.length * 3 + ClearScreenAfterCursor.length
         val builder: StringBuilder = new StringBuilder(len)
         message.linesIterator.foreach { line =>
           builder.ensureCapacity(len + line.length + 4)
           builder.setLength(0)
 
-          def fmted(a: String, b: String) = builder.append(reset).append(a).append(b).append(reset)
+          def fmted(a: String, b: String) = {
+            if (useFormat) builder.append(reset).append(a).append(b).append(reset)
+            else builder.append(b)
+          }
 
-          builder.append(reset).append('[')
+          if (useFormat) builder.append(reset)
+          builder.append('[')
           fmted(labelColor, label)
           builder.append("] ")
           fmted(messageColor, line)
-          builder.append(clearScreenAfterCursor)
+          if (ansiCodesSupported) builder.append(ClearScreenAfterCursor)
           write(builder.toString)
         }
       }
@@ -489,8 +486,15 @@ trait Appender extends AutoCloseable {
   }
 
   private def write(msg: String): Unit = {
-    val toWrite =
-      if (!useFormat || !ansiCodesSupported) EscHelpers.removeEscapeSequences(msg) else msg
+    // There is no api for removing only colors but not other ansi escape sequences
+    // so we do nothing if useFormat is false but ansiCodesSupported is true which is
+    // a rare use case but if ansiCodesSupported is true, color codes should work so
+    // 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) {
+      if (useFormat) EscHelpers.stripMoves(msg) else EscHelpers.removeEscapeSequences(msg)
+    } else msg
     out.println(toWrite)
   }