From 923f4261d07bc1aadce12f9ef383dad91dfb1582 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Fri, 27 Feb 2015 14:43:25 -0500 Subject: [PATCH] Fix ANSI escape sequences. Now we handle the CSI (ESC + [). Fixes #1143 --- notes/0.13.8/ansi-escapes.markdown | 10 ++++ .../src/main/scala/sbt/ConsoleLogger.scala | 50 ++++++++++++++++--- util/log/src/test/scala/Escapes.scala | 45 +++++++++++++---- 3 files changed, 90 insertions(+), 15 deletions(-) create mode 100644 notes/0.13.8/ansi-escapes.markdown diff --git a/notes/0.13.8/ansi-escapes.markdown b/notes/0.13.8/ansi-escapes.markdown new file mode 100644 index 000000000..bd88b644e --- /dev/null +++ b/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] diff --git a/util/log/src/main/scala/sbt/ConsoleLogger.scala b/util/log/src/main/scala/sbt/ConsoleLogger.scala index a614e4315..f86d7f2f5 100644 --- a/util/log/src/main/scala/sbt/ConsoleLogger.scala +++ b/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 = - if (i >= s.length) + private[this] def skipESC(s: String, i: Int): Int = { + 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 = { diff --git a/util/log/src/test/scala/Escapes.scala b/util/log/src/test/scala/Escapes.scala index f780d25bf..408ec5e23 100644 --- a/util/log/src/test/scala/Escapes.scala +++ b/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) - ("Input string with escapes: '" + withEscapes + "'") |: - ("Escapes removed '" + removed + "'") |: + val original = start + escapes.map(_.notEscape).mkString("") + val diffCharString = diffIndex(original, 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)