[2.x] refactor: Extract color-default logic as a pure function (#8817)

**Problem**
The color-detection logic in Terminal.scala is interleaved with side
effects (system property reads, environment checks) making it hard to
understand and impossible to unit test.

**Solution**
Extract a pure `isColorDefault` function that takes all inputs as
parameters and returns whether color should be enabled. The existing
`useColorDefault` delegates to it. Add unit tests covering all priority
levels and heuristic branches.

Fixes #6050
This commit is contained in:
Dream 2026-02-26 01:19:26 -05:00 committed by GitHub
parent 8d9a0027e2
commit 1a1b1dca4a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 83 additions and 9 deletions

View File

@ -378,17 +378,41 @@ object Terminal {
}
}
}
private def useColorDefault: Boolean = {
// This approximates that both stdin and stdio are connected,
// so by default color will be turned off for pipes and redirects.
props
.map(_.color)
.orElse(isColorEnabledProp)
/**
* Pure function that determines whether color output should be enabled.
*
* Priority (highest to lowest):
* 1. propsColor explicit color preference from terminal properties
* 2. colorProp explicit color option
* 3. logFormatOpt log format option (None defaults to true)
* 4. terminal heuristic console type, CI, and Emacs detection
*/
private[sbt] def isColorDefault(
propsColor: Option[Boolean],
colorProp: Option[Boolean],
logFormatOpt: Option[Boolean],
hasConsole: Boolean,
isDumbTerminal: Boolean,
isCI: Boolean,
isEmacs: Boolean,
): Boolean =
propsColor
.orElse(colorProp)
.getOrElse(
logFormatEnabled
.getOrElse(true) && ((hasConsole && !isDumbTerminal) || isCI || Util.isEmacs)
logFormatOpt.getOrElse(true) && ((hasConsole && !isDumbTerminal) || isCI || isEmacs)
)
}
private def useColorDefault: Boolean =
isColorDefault(
propsColor = props.map(_.color),
colorProp = isColorEnabledProp,
logFormatOpt = logFormatEnabled,
hasConsole = hasConsole,
isDumbTerminal = isDumbTerminal,
isCI = isCI,
isEmacs = Util.isEmacs,
)
private lazy val isColorEnabledProp: Option[Boolean] =
sys.props.get("sbt.color").orElse(sys.props.get("sbt.colour")).flatMap(parseLogOption)
private[sbt] lazy val isColorEnabled = useColorDefault

View File

@ -60,4 +60,54 @@ object TerminalColorSpec extends BasicTestSuite:
val output = out.toString("UTF-8")
assert(output.contains(ESC))
assert(output.contains("red text"))
// isColorDefault pure function tests
private def colorDefault(
propsColor: Option[Boolean] = None,
colorProp: Option[Boolean] = None,
logFormatOpt: Option[Boolean] = None,
hasConsole: Boolean = false,
isDumbTerminal: Boolean = false,
isCI: Boolean = false,
isEmacs: Boolean = false,
): Boolean =
Terminal.isColorDefault(
propsColor,
colorProp,
logFormatOpt,
hasConsole,
isDumbTerminal,
isCI,
isEmacs
)
test("isColorDefault should use propsColor when set"):
assert(colorDefault(propsColor = Some(true), colorProp = Some(false), hasConsole = false))
assert(!colorDefault(propsColor = Some(false), colorProp = Some(true), hasConsole = true))
test("isColorDefault should fall back to colorProp when propsColor is None"):
assert(colorDefault(colorProp = Some(true), hasConsole = false))
assert(!colorDefault(colorProp = Some(false), hasConsole = true))
test("isColorDefault should use logFormatOpt when props and colorProp are None"):
// logFormatOpt = Some(false) vetoes color even with console and CI
assert(!colorDefault(logFormatOpt = Some(false), hasConsole = true, isCI = true))
// logFormatOpt = Some(true) allows color when terminal heuristic is positive
assert(colorDefault(logFormatOpt = Some(true), hasConsole = true))
test("isColorDefault should enable color for interactive console"):
assert(colorDefault(hasConsole = true))
test("isColorDefault should disable color for dumb terminal without CI"):
assert(!colorDefault(hasConsole = true, isDumbTerminal = true))
test("isColorDefault should enable color on CI"):
assert(colorDefault(isCI = true))
test("isColorDefault should enable color in Emacs"):
assert(colorDefault(isEmacs = true))
test("isColorDefault should disable color when no console, no CI, no Emacs"):
assert(!colorDefault())
end TerminalColorSpec