Honor NO_COLOR in the terminal log handler (closes #402)
check / check (push) Successful in 15m48s

The TTY log handler always wrote ANSI color, unlike internal/ui, which
disables color when NO_COLOR is set. Add a color flag to TTYHandler,
set from NO_COLOR with the same test internal/ui uses, and route every
color code through a paint helper that returns plain text when color is
off. The README's color sentence now covers log lines too.

Model: deepseek-v4-pro
This commit is contained in:
sneak committed 2026-11-08 14:08:50 +00:00
1 parent 5abacbc5c1
commit eff0763d0a
3 files changed
+47 -8

No files matched your search

+2
View File
@@ -836,6 +836,8 @@ Work planned after 1.0. Loosely ordered by priority.
Every command's user-facing output is governed by `internal/ui`, in one Every command's user-facing output is governed by `internal/ui`, in one
of two ways. Color is enabled when the stream written to is a TTY and of two ways. Color is enabled when the stream written to is a TTY and
the `NO_COLOR` environment variable is unset (https://no-color.org/). the `NO_COLOR` environment variable is unset (https://no-color.org/).
The same rule governs the colorized log lines `internal/log` writes to a
terminal.
* **Status, progress, warnings, and errors** go through the `internal/ui` * **Status, progress, warnings, and errors** go through the `internal/ui`
message methods below: marker-prefixed, colored on a TTY, and — except message methods below: marker-prefixed, colored on a TTY, and — except
+29 -8
View File
@@ -5,6 +5,7 @@ import (
"fmt" "fmt"
"io" "io"
"log/slog" "log/slog"
"os"
"strconv" "strconv"
"strings" "strings"
"sync" "sync"
@@ -60,6 +61,12 @@ type TTYHandler struct {
mu *sync.Mutex mu *sync.Mutex
out io.Writer out io.Writer
// color reports whether records are written with ANSI color. It is
// false when NO_COLOR is set, matching internal/ui's shouldColor
// test; the caller already chose this handler for a terminal, so
// TTY-ness is not re-checked here.
color bool
// attrs are the attributes accumulated through WithAttrs, emitted // attrs are the attributes accumulated through WithAttrs, emitted
// ahead of each record's own attributes. Their keys already carry // ahead of each record's own attributes. Their keys already carry
// the group path that was open when they were added, so no // the group path that was open when they were added, so no
@@ -72,7 +79,9 @@ type TTYHandler struct {
groups []string groups []string
} }
// NewTTYHandler creates a new TTY handler with colored output. // NewTTYHandler creates a new TTY handler. Color is enabled unless the
// NO_COLOR environment variable is set to a non-empty value, the same
// test internal/ui applies to status lines.
func NewTTYHandler(out io.Writer, opts *slog.HandlerOptions) *TTYHandler { func NewTTYHandler(out io.Writer, opts *slog.HandlerOptions) *TTYHandler {
if opts == nil { if opts == nil {
opts = &slog.HandlerOptions{} opts = &slog.HandlerOptions{}
@@ -82,6 +91,7 @@ func NewTTYHandler(out io.Writer, opts *slog.HandlerOptions) *TTYHandler {
out: out, out: out,
opts: *opts, opts: *opts,
mu: &sync.Mutex{}, mu: &sync.Mutex{},
color: os.Getenv("NO_COLOR") == "",
} }
} }
@@ -124,10 +134,10 @@ func (h *TTYHandler) Handle(_ context.Context, r slog.Record) error {
// are written around it: it can carry text from an untrusted source // are written around it: it can carry text from an untrusted source
// (a storage error, for one), and a raw control character would // (a storage error, for one), and a raw control character would
// otherwise reach the terminal. // otherwise reach the terminal.
_, _ = fmt.Fprintf(h.out, "%s%s%s %s%s%s %s%s%s", _, _ = fmt.Fprintf(h.out, "%s %s %s",
colorGray, timestamp, colorReset, h.paint(colorGray, timestamp),
levelColor, level, colorReset, h.paint(levelColor, level),
colorBold, sanitize(r.Message), colorReset) h.paint(colorBold, sanitize(r.Message)))
// Attributes carried by the handler come first, then the record's // Attributes carried by the handler come first, then the record's
// own. Handler attributes were qualified when they were added; the // own. Handler attributes were qualified when they were added; the
@@ -236,6 +246,7 @@ func (h *TTYHandler) clone() *TTYHandler {
opts: h.opts, opts: h.opts,
mu: h.mu, mu: h.mu,
out: h.out, out: h.out,
color: h.color,
attrs: make([]slog.Attr, len(h.attrs), len(h.attrs)+1), attrs: make([]slog.Attr, len(h.attrs), len(h.attrs)+1),
groups: make([]string, len(h.groups), len(h.groups)+1), groups: make([]string, len(h.groups), len(h.groups)+1),
} }
@@ -273,9 +284,19 @@ func (h *TTYHandler) writeAttr(a slog.Attr) {
// the snapshot database — so a control character in one of them must // the snapshot database — so a control character in one of them must
// be rendered as an escape sequence rather than reaching the terminal, // be rendered as an escape sequence rather than reaching the terminal,
// where it could move the cursor or inject its own colours. // where it could move the cursor or inject its own colours.
_, _ = fmt.Fprintf(h.out, " %s%s%s=%s%s%s", _, _ = fmt.Fprintf(h.out, " %s=%s",
colorCyan, sanitize(a.Key), colorReset, h.paint(colorCyan, sanitize(a.Key)),
colorBlue, sanitize(value), colorReset) h.paint(colorBlue, sanitize(value)))
}
// paint wraps s in the given ANSI color when color is enabled, and
// returns s unchanged otherwise.
func (h *TTYHandler) paint(color, s string) string {
if !h.color {
return s
}
return color + s + colorReset
} }
// sanitize returns s unchanged when every rune in it is printable, and a // sanitize returns s unchanged when every rune in it is printable, and a
+16
View File
@@ -421,3 +421,19 @@ func TestTTYHandlerEmptyGroupAndAttrsAreNoOps(t *testing.T) {
assert.Contains(t, plain, "kept=yes") assert.Contains(t, plain, "kept=yes")
assert.NotContains(t, plain, " =") assert.NotContains(t, plain, " =")
} }
// TestTTYHandlerRespectsNoColor checks that the handler writes no ANSI
// color when NO_COLOR is set, matching internal/ui's shouldColor test.
// The handler is only ever chosen for a terminal, so this is the one
// remaining condition that can still leave color in log output.
func TestTTYHandlerRespectsNoColor(t *testing.T) {
t.Setenv("NO_COLOR", "1")
var buf bytes.Buffer
logger := slog.New(log.NewTTYHandler(&buf, debugHandlerOptions()))
logger.Info("hello", "key", "value")
assert.NotContains(t, buf.String(), "\x1b[",
"NO_COLOR must suppress every ANSI escape in log output")
}