Merge pull request #1885 from sbt/wip/fix-ansi-escape

Fix ANSI escape sequences.

notes/0.13.8/ansi-escapes.markdown

@@ -0,0 +1,10 @@
+  [@jsuereth]: https://github.com/jsuereth
+  [1885]: https://github.com/sbt/sbt/issues/1885
+
+### Fixes with compatibility implications
+
+### Improvements
+
+### Bug fixes
+
+- Fixes handling of ANSI CSI codes. [#1885][1885] by [@jsuereth][@jsuereth]

util/log/src/main/scala/sbt/ConsoleLogger.scala

@@ -31,11 +31,40 @@ object ConsoleLogger {
   /**
    * An escape terminator is a character in the range `@` (decimal value 64) to `~` (decimal value 126).
    * It is the final character in an escape sequence.
+   *
+   * cf. http://en.wikipedia.org/wiki/ANSI_escape_code#CSI_codes
    */
+  @deprecated("No longer public.", "0.13.8")
   def isEscapeTerminator(c: Char): Boolean =
     c >= '@' && c <= '~'
 
-  /** Returns true if the string contains the ESC character. */
+  /**
+   * Test if the character AFTER an ESC is the ANSI CSI.
+   *
+   * see: http://en.wikipedia.org/wiki/ANSI_escape_code
+   *
+   * The CSI (control sequence instruction) codes start with ESC + '['.   This is for testing the second character.
+   *
+   * There is an additional CSI (one character) that we could test for, but is not frequnetly used, and we don't
+   * check for it.
+   *
+   * cf. http://en.wikipedia.org/wiki/ANSI_escape_code#CSI_codes
+   */
+  private def isCSI(c: Char): Boolean = c == '['
+
+  /**
+   * Tests whether or not a character needs to immediately terminate the ANSI sequence.
+   *
+   * c.f. http://en.wikipedia.org/wiki/ANSI_escape_code#Sequence_elements
+   */
+  private def isAnsiTwoCharacterTerminator(c: Char): Boolean =
+    (c >= '@') && (c <= '_')
+
+  /**
+   * Returns true if the string contains the ESC character.
+   *
+   * TODO - this should handle raw CSI (not used much)
+   */
   def hasEscapeSequence(s: String): Boolean =
     s.indexOf(ESC) >= 0
 
@@ -58,19 +87,28 @@ object ConsoleLogger {
       sb.append(s, start, s.length)
     else {
       sb.append(s, start, escIndex)
-      val next = skipESC(s, escIndex + 1)
+      val next: Int =
+        // If it's a CSI we skip past it and then look for a terminator.
+        if (isCSI(s.charAt(escIndex + 1))) skipESC(s, escIndex + 2)
+        else if (isAnsiTwoCharacterTerminator(s.charAt(escIndex + 1))) escIndex + 2
+        else {
+          // There could be non-ANSI character sequences we should make sure we handle here.
+          skipESC(s, escIndex + 1)
+        }
       nextESC(s, next, sb)
     }
   }
 
   /** Skips the escape sequence starting at `i-1`.  `i` should be positioned at the character after the ESC that starts the sequence. */
-  private[this] def skipESC(s: String, i: Int): Int =
+  private[this] def skipESC(s: String, i: Int): Int = {
-    if (i >= s.length)
+    if (i >= s.length) {
       i
-    else if (isEscapeTerminator(s.charAt(i)))
+    } else if (isEscapeTerminator(s.charAt(i))) {
       i + 1
-    else
+    } else {
       skipESC(s, i + 1)
+    }
+  }
 
   val formatEnabled =
     {

util/log/src/test/scala/Escapes.scala

@@ -37,28 +37,50 @@ object Escapes extends Properties("Escapes") {
 
   property("removeEscapeSequences returns string without escape sequences") =
     forAllNoShrink(genWithoutEscape, genEscapePairs) { (start: String, escapes: List[EscapeAndNot]) =>
-      val withEscapes: String = start + escapes.map { ean => ean.escape.makeString + ean.notEscape }
+      val withEscapes: String = start + (escapes.map { ean => ean.escape.makeString + ean.notEscape }).mkString("")
       val removed: String = removeEscapeSequences(withEscapes)
-      val original = start + escapes.map(_.notEscape)
+      val original = start + escapes.map(_.notEscape).mkString("")
-      ("Input string with escapes: '" + withEscapes + "'") |:
+      val diffCharString = diffIndex(original, removed)
-        ("Escapes removed '" + removed + "'") |:
+      ("Input string    : '" + withEscapes + "'") |:
+        ("Expected        : '" + original + "'") |:
+        ("Escapes removed : '" + removed + "'") |:
+        (diffCharString) |:
         (original == removed)
     }
 
-  final case class EscapeAndNot(escape: EscapeSequence, notEscape: String)
+  def diffIndex(expect: String, original: String): String = {
+    var i = 0;
+    while (i < expect.length && i < original.length) {
+      if (expect.charAt(i) != original.charAt(i)) return ("Differing character, idx: " + i + ", char: " + original.charAt(i) + ", expected: " + expect.charAt(i))
+      i += 1
+    }
+    if (expect.length != original.length) return s"Strings are different lengths!"
+    "No differences found"
+  }
+
+  final case class EscapeAndNot(escape: EscapeSequence, notEscape: String) {
+    override def toString = s"EscapeAntNot(escape = [$escape], notEscape = [${notEscape.map(_.toInt)}])"
+  }
   final case class EscapeSequence(content: String, terminator: Char) {
-    assert(content.forall(c => !isEscapeTerminator(c)), "Escape sequence content contains an escape terminator: '" + content + "'")
+    if (!content.isEmpty) {
+      assert(content.tail.forall(c => !isEscapeTerminator(c)), "Escape sequence content contains an escape terminator: '" + content + "'")
+      assert((content.head == '[') || !isEscapeTerminator(content.head), "Escape sequence content contains an escape terminator: '" + content.headOption + "'")
+    }
     assert(isEscapeTerminator(terminator))
     def makeString: String = ESC + content + terminator
+
+    override def toString =
+      if (content.isEmpty) s"ESC (${terminator.toInt})"
+      else s"ESC ($content) (${terminator.toInt})"
   }
   private[this] def noEscape(s: String): String = s.replace(ESC, ' ')
 
-  lazy val genEscapeSequence: Gen[EscapeSequence] = oneOf(genKnownSequence, genArbitraryEscapeSequence)
+  lazy val genEscapeSequence: Gen[EscapeSequence] = oneOf(genKnownSequence, genTwoCharacterSequence, genArbitraryEscapeSequence)
   lazy val genEscapePair: Gen[EscapeAndNot] = for (esc <- genEscapeSequence; not <- genWithoutEscape) yield EscapeAndNot(esc, not)
   lazy val genEscapePairs: Gen[List[EscapeAndNot]] = listOf(genEscapePair)
 
   lazy val genArbitraryEscapeSequence: Gen[EscapeSequence] =
-    for (content <- genWithoutTerminator; term <- genTerminator) yield new EscapeSequence(content, term)
+    for (content <- genWithoutTerminator if !content.isEmpty; term <- genTerminator) yield new EscapeSequence("[" + content, term)
 
   lazy val genKnownSequence: Gen[EscapeSequence] =
     oneOf((misc ++ setGraphicsMode ++ setMode ++ resetMode).map(toEscapeSequence))
@@ -74,7 +96,12 @@ object Escapes extends Properties("Escapes") {
   lazy val setMode = setModeLike('h')
   def setModeLike(term: Char): Seq[String] = (0 to 19).map(i => "=" + i.toString + term)
 
-  lazy val genWithoutTerminator = genRawString.map(_.filter { c => !isEscapeTerminator(c) })
+  lazy val genWithoutTerminator =
+    genRawString.map(_.filter { c => !isEscapeTerminator(c) && (c != '[') })
+
+  lazy val genTwoCharacterSequence =
+    // 91 == [ which is the CSI escape sequence.
+    oneOf((64 to 95)) filter (_ != 91) map (c => new EscapeSequence("", c.toChar))
 
   lazy val genTerminator: Gen[Char] = Gen.choose('@', '~')
   lazy val genWithoutEscape: Gen[String] = genRawString.map(noEscape)