From f09d14506e944de1df7733b7edb8e0f65c743114 Mon Sep 17 00:00:00 2001 From: Sergey Trofimovsky Date: Tue, 24 Mar 2026 03:14:30 +0000 Subject: [PATCH 1/5] Fix NO_COLOR on TUI detail screens and add ROBOREV_COLOR_MODE env var glamour's NewTermRenderer defaults to TrueColor profile, ignoring the NO_COLOR convention. This caused review/prompt detail screens to emit ANSI color sequences even with NO_COLOR=1, while the main queue screen (using lipgloss only) correctly suppressed colors. Pass glamour.WithColorProfile() when creating term renderers in both the TUI markdown cache and the streamfmt package. When NO_COLOR is set or ROBOREV_COLOR_MODE=none, the Ascii profile is used, which strips all color output from glamour. Also fix stripTrailingPadding/StripTrailingPadding to skip the unconditional \x1b[0m reset suffix in no-color mode. Add ROBOREV_COLOR_MODE env var (auto|dark|light|none) for explicit control over the TUI color theme without relying on terminal detection. GlamourStyle() and resolveColorMode() both respect this env var. --- cmd/roborev/tui/helpers.go | 54 ++++++++++++++---- cmd/roborev/tui/helpers_test.go | 82 ++++++++++++++++++++++++++-- internal/streamfmt/render.go | 21 +++++-- internal/streamfmt/streamfmt.go | 38 +++++++++++-- internal/streamfmt/streamfmt_test.go | 55 +++++++++++++++++++ 5 files changed, 224 insertions(+), 26 deletions(-) diff --git a/cmd/roborev/tui/helpers.go b/cmd/roborev/tui/helpers.go index 0ba8d2bd2..c4900809c 100644 --- a/cmd/roborev/tui/helpers.go +++ b/cmd/roborev/tui/helpers.go @@ -1,6 +1,7 @@ package tui import ( + "os" "regexp" "strings" "time" @@ -299,6 +300,7 @@ func wrapLine(line string, width int) []string { // which blocks for seconds inside bubbletea's raw-mode input loop. type markdownCache struct { glamourStyle gansi.StyleConfig // custom style derived from dark/light, detected once at init + colorProfile termenv.Profile // color profile for glamour rendering (Ascii when NO_COLOR) tabWidth int // tab expansion width (default 2) reviewLines []string @@ -318,14 +320,36 @@ type markdownCache struct { lastPromptMaxScroll int } +// resolveColorMode returns the glamour style config and termenv color profile +// based on the ROBOREV_COLOR_MODE env var and the NO_COLOR convention. +// Valid ROBOREV_COLOR_MODE values: auto (default), dark, light, none. +func resolveColorMode() (gansi.StyleConfig, termenv.Profile) { + mode := strings.ToLower(os.Getenv("ROBOREV_COLOR_MODE")) + + if mode == "none" || termenv.EnvNoColor() { + return styles.DarkStyleConfig, termenv.Ascii + } + + profile := termenv.EnvColorProfile() + switch mode { + case "dark": + return styles.DarkStyleConfig, profile + case "light": + return styles.LightStyleConfig, profile + default: // "auto" or "" + if termenv.HasDarkBackground() { + return styles.DarkStyleConfig, profile + } + return styles.LightStyleConfig, profile + } +} + // newMarkdownCache creates a markdownCache, detecting terminal background // color now (before bubbletea enters raw mode and takes over stdin). // Builds a custom style with zero margins to avoid extra padding. +// Respects ROBOREV_COLOR_MODE env var and NO_COLOR convention. func newMarkdownCache(tabWidth int) *markdownCache { - style := styles.LightStyleConfig - if termenv.HasDarkBackground() { - style = styles.DarkStyleConfig - } + style, profile := resolveColorMode() // Remove document and code block margins that add extra indentation. zeroMargin := uint(0) style.Document.Margin = &zeroMargin @@ -339,7 +363,7 @@ func newMarkdownCache(tabWidth int) *markdownCache { } else if tabWidth > 16 { tabWidth = 16 } - return &markdownCache{glamourStyle: style, tabWidth: tabWidth} + return &markdownCache{glamourStyle: style, colorProfile: profile, tabWidth: tabWidth} } // truncateLongLines normalizes tabs and truncates lines inside fenced code @@ -481,15 +505,21 @@ var trailingPadRe = regexp.MustCompile(`(\s|\x1b\[[0-9;]*m)+$`) // stripTrailingPadding removes trailing whitespace and ANSI SGR codes from a // glamour output line, then appends a reset to ensure clean color state. -func stripTrailingPadding(line string) string { - return trailingPadRe.ReplaceAllString(line, "") + "\x1b[0m" +// When noColor is true, the reset sequence is omitted. +func stripTrailingPadding(line string, noColor bool) string { + line = trailingPadRe.ReplaceAllString(line, "") + if noColor { + return line + } + return line + "\x1b[0m" } // renderMarkdownLines renders markdown text using glamour and splits into lines. // wrapWidth controls glamour's word-wrap column (capped for readability). // maxWidth controls line truncation (actual terminal width). +// colorProfile controls glamour's color output (use termenv.Ascii to suppress colors). // Falls back to wrapText if glamour rendering fails. -func renderMarkdownLines(text string, wrapWidth, maxWidth int, glamourStyle gansi.StyleConfig, tabWidth int) []string { +func renderMarkdownLines(text string, wrapWidth, maxWidth int, glamourStyle gansi.StyleConfig, tabWidth int, colorProfile termenv.Profile) []string { // Truncate long lines before glamour so they don't get word-wrapped. // Use maxWidth (terminal width) so content fills the available space. text = truncateLongLines(text, maxWidth, tabWidth) @@ -497,6 +527,7 @@ func renderMarkdownLines(text string, wrapWidth, maxWidth int, glamourStyle gans glamour.WithStyles(glamourStyle), glamour.WithWordWrap(wrapWidth), glamour.WithPreservedNewLines(), + glamour.WithColorProfile(colorProfile), ) if err != nil { return sanitizeLines(wrapText(text, wrapWidth)) @@ -505,9 +536,10 @@ func renderMarkdownLines(text string, wrapWidth, maxWidth int, glamourStyle gans if err != nil { return sanitizeLines(wrapText(text, wrapWidth)) } + noColor := colorProfile == termenv.Ascii lines := strings.Split(strings.TrimRight(out, "\n"), "\n") for i, line := range lines { - line = stripTrailingPadding(line) + line = stripTrailingPadding(line, noColor) line = sanitizeEscapes(line) // Truncate output lines that still exceed maxWidth (glamour can add // indentation for block quotes, lists, etc. beyond the wrap width). @@ -526,7 +558,7 @@ func (c *markdownCache) getReviewLines(text string, wrapWidth, maxWidth int, rev if c.reviewID == reviewID && c.reviewWidth == maxWidth && c.reviewText == text { return c.reviewLines } - c.reviewLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth) + c.reviewLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth, c.colorProfile) c.reviewID = reviewID c.reviewWidth = maxWidth c.reviewText = text @@ -540,7 +572,7 @@ func (c *markdownCache) getPromptLines(text string, wrapWidth, maxWidth int, rev if c.promptID == reviewID && c.promptWidth == maxWidth && c.promptText == text { return c.promptLines } - c.promptLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth) + c.promptLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth, c.colorProfile) c.promptID = reviewID c.promptWidth = maxWidth c.promptText = text diff --git a/cmd/roborev/tui/helpers_test.go b/cmd/roborev/tui/helpers_test.go index e0ad3b23e..3943c9060 100644 --- a/cmd/roborev/tui/helpers_test.go +++ b/cmd/roborev/tui/helpers_test.go @@ -10,6 +10,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/glamour/styles" "github.com/mattn/go-runewidth" + "github.com/muesli/termenv" "github.com/roborev-dev/roborev/internal/storage" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -23,7 +24,7 @@ func stripTestANSI(s string) string { func TestRenderMarkdownLinesPreservesNewlines(t *testing.T) { // Verify that single newlines in plain text are preserved (not collapsed into one paragraph) - lines := renderMarkdownLines("Line 1\nLine 2\nLine 3", 80, 80, styles.DarkStyleConfig, 2) + lines := renderMarkdownLines("Line 1\nLine 2\nLine 3", 80, 80, styles.DarkStyleConfig, 2, termenv.TrueColor) found := 0 for _, line := range lines { @@ -36,7 +37,7 @@ func TestRenderMarkdownLinesPreservesNewlines(t *testing.T) { } func TestRenderMarkdownLinesFallsBackOnEmpty(t *testing.T) { - lines := renderMarkdownLines("", 80, 80, styles.DarkStyleConfig, 2) + lines := renderMarkdownLines("", 80, 80, styles.DarkStyleConfig, 2, termenv.TrueColor) // Should not panic and should produce some output (even if empty) assert.NotNil(t, lines) } @@ -304,7 +305,7 @@ func TestRenderMarkdownLinesPreservesLongProse(t *testing.T) { // Long prose lines should be word-wrapped by glamour, not truncated. // All words must appear in the rendered output. longProse := "This is a very long prose line with important content that should be word-wrapped by glamour rather than truncated so that no information is lost from the rendered output" - lines := renderMarkdownLines(longProse, 60, 80, styles.DarkStyleConfig, 2) + lines := renderMarkdownLines(longProse, 60, 80, styles.DarkStyleConfig, 2, termenv.TrueColor) var combined strings.Builder for _, line := range lines { @@ -423,7 +424,7 @@ func TestRenderMarkdownLinesNoOverflow(t *testing.T) { longLine := strings.Repeat("x", 200) text := "Review:\n\n```\n" + longLine + "\n```\n" width := 76 - lines := renderMarkdownLines(text, width, width, styles.DarkStyleConfig, 2) + lines := renderMarkdownLines(text, width, width, styles.DarkStyleConfig, 2, termenv.TrueColor) for i, line := range lines { stripped := stripTestANSI(line) @@ -433,6 +434,79 @@ func TestRenderMarkdownLinesNoOverflow(t *testing.T) { } } +func TestResolveColorMode(t *testing.T) { + tests := []struct { + name string + colorMode string + noColor string + wantProfile termenv.Profile + wantDarkBase bool // true if DarkStyleConfig should be selected + }{ + { + name: "none mode returns Ascii", + colorMode: "none", + wantProfile: termenv.Ascii, + }, + { + name: "NO_COLOR returns Ascii", + noColor: "1", + wantProfile: termenv.Ascii, + }, + { + name: "dark mode forces dark style", + colorMode: "dark", + wantDarkBase: true, + }, + { + name: "light mode forces light style", + colorMode: "light", + wantDarkBase: false, + }, + { + name: "empty defaults to auto", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.colorMode != "" { + t.Setenv("ROBOREV_COLOR_MODE", tt.colorMode) + } else { + t.Setenv("ROBOREV_COLOR_MODE", "") + } + if tt.noColor != "" { + t.Setenv("NO_COLOR", tt.noColor) + } else { + t.Setenv("NO_COLOR", "") + } + style, profile := resolveColorMode() + if tt.wantProfile != 0 { + assert.Equal(t, tt.wantProfile, profile) + } + if tt.colorMode == "dark" || tt.colorMode == "none" { + // DarkStyleConfig has non-nil Heading.Color + assert.NotNil(t, style.H1.Color, "expected dark-based style") + } + _ = style // ensure no panic + }) + } +} + +func TestRenderMarkdownLinesNoColor(t *testing.T) { + // When colorProfile is Ascii, output should contain no ANSI color sequences. + // Bold/reset sequences (\x1b[;1m, \x1b[0m) are still emitted by glamour + // for text formatting — these are not colors and are acceptable under NO_COLOR. + text := "# Heading\n\nSome **bold** text and `code`." + lines := renderMarkdownLines(text, 80, 80, styles.DarkStyleConfig, 2, termenv.Ascii) + + combined := strings.Join(lines, "\n") + // Match ANSI SGR sequences that set foreground/background colors: + // \x1b[3Xm (fg), \x1b[4Xm (bg), \x1b[9Xm (bright fg), \x1b[10Xm (bright bg), + // \x1b[38;...m (extended fg), \x1b[48;...m (extended bg). + colorSGR := regexp.MustCompile(`\x1b\[(3[0-7]|4[0-7]|9[0-7]|10[0-7]|38;|48;)[0-9;]*m`) + matches := colorSGR.FindAllString(combined, -1) + assert.Empty(t, matches, "expected no ANSI color sequences with Ascii profile, got: %v", matches) +} + func TestReflowHelpRows(t *testing.T) { tests := []struct { diff --git a/internal/streamfmt/render.go b/internal/streamfmt/render.go index bc6bafb53..e26285638 100644 --- a/internal/streamfmt/render.go +++ b/internal/streamfmt/render.go @@ -8,6 +8,7 @@ import ( gansi "github.com/charmbracelet/glamour/ansi" xansi "github.com/charmbracelet/x/ansi" "github.com/mattn/go-runewidth" + "github.com/muesli/termenv" ) // ansiEscapePattern matches ANSI escape sequences (colors, cursor @@ -88,9 +89,13 @@ var trailingPadRe = regexp.MustCompile(`(\s|\x1b\[[0-9;]*m)+$`) // StripTrailingPadding removes trailing whitespace and ANSI SGR codes // from a glamour output line, then appends a reset to ensure clean -// color state. -func StripTrailingPadding(line string) string { - return trailingPadRe.ReplaceAllString(line, "") + "\x1b[0m" +// color state. When noColor is true, the reset sequence is omitted. +func StripTrailingPadding(line string, noColor bool) string { + line = trailingPadRe.ReplaceAllString(line, "") + if noColor { + return line + } + return line + "\x1b[0m" } // WrapText wraps text to the specified width, preserving existing @@ -233,17 +238,20 @@ func ParseFence(line string) (byte, int, bool) { // RenderMarkdownLines renders markdown text using glamour and splits // into lines. wrapWidth controls glamour's word-wrap column. -// maxWidth controls line truncation (actual terminal width). Falls -// back to WrapText if glamour rendering fails. +// maxWidth controls line truncation (actual terminal width). +// colorProfile controls glamour's color output (use termenv.Ascii to suppress colors). +// Falls back to WrapText if glamour rendering fails. func RenderMarkdownLines( text string, wrapWidth, maxWidth int, glamourStyle gansi.StyleConfig, tabWidth int, + colorProfile termenv.Profile, ) []string { text = TruncateLongLines(text, maxWidth, tabWidth) r, err := glamour.NewTermRenderer( glamour.WithStyles(glamourStyle), glamour.WithWordWrap(wrapWidth), glamour.WithPreservedNewLines(), + glamour.WithColorProfile(colorProfile), ) if err != nil { return SanitizeLines(WrapText(text, wrapWidth)) @@ -252,9 +260,10 @@ func RenderMarkdownLines( if err != nil { return SanitizeLines(WrapText(text, wrapWidth)) } + noColor := colorProfile == termenv.Ascii lines := strings.Split(strings.TrimRight(out, "\n"), "\n") for i, line := range lines { - line = StripTrailingPadding(line) + line = StripTrailingPadding(line, noColor) line = SanitizeEscapes(line) if xansi.StringWidth(line) > maxWidth { line = xansi.Truncate(line, maxWidth, "") diff --git a/internal/streamfmt/streamfmt.go b/internal/streamfmt/streamfmt.go index 1677f19ab..357e47234 100644 --- a/internal/streamfmt/streamfmt.go +++ b/internal/streamfmt/streamfmt.go @@ -6,6 +6,7 @@ import ( "encoding/json" "fmt" "io" + "os" "strings" "unicode" @@ -53,6 +54,7 @@ type Formatter struct { width int // terminal width; 0 = no wrapping glamourStyle gansi.StyleConfig // detected once at init + colorProfile termenv.Profile // color profile for glamour (Ascii when NO_COLOR) writeErr error // first write error encountered during formatting lastWasTool bool // tracks tool vs text transitions for spacing @@ -70,6 +72,7 @@ func New(w io.Writer, isTTY bool) *Formatter { f := &Formatter{w: w, isTTY: isTTY} if isTTY { f.glamourStyle = GlamourStyle() + f.colorProfile = ResolveColorProfile() f.width = TerminalWidth(w) } return f @@ -82,7 +85,8 @@ func NewWithWidth( w io.Writer, width int, style gansi.StyleConfig, ) *Formatter { return &Formatter{ - w: w, isTTY: true, width: width, glamourStyle: style, + w: w, isTTY: true, width: width, + glamourStyle: style, colorProfile: ResolveColorProfile(), } } @@ -99,10 +103,23 @@ func TerminalWidth(w io.Writer) int { // GlamourStyle returns a glamour style config with zero margins, // matching the TUI's rendering. Detects dark/light background once. +// Respects ROBOREV_COLOR_MODE env var for explicit dark/light/none selection. func GlamourStyle() gansi.StyleConfig { - style := styles.LightStyleConfig - if termenv.HasDarkBackground() { + mode := strings.ToLower(os.Getenv("ROBOREV_COLOR_MODE")) + var style gansi.StyleConfig + switch mode { + case "dark": style = styles.DarkStyleConfig + case "light": + style = styles.LightStyleConfig + case "none": + // Use dark style as base; colors will be stripped by Ascii profile. + style = styles.DarkStyleConfig + default: // "auto" or "" + style = styles.LightStyleConfig + if termenv.HasDarkBackground() { + style = styles.DarkStyleConfig + } } zeroMargin := uint(0) style.Document.Margin = &zeroMargin @@ -112,6 +129,17 @@ func GlamourStyle() gansi.StyleConfig { return style } +// ResolveColorProfile returns the termenv color profile based on +// the ROBOREV_COLOR_MODE env var and the NO_COLOR convention. +// Returns termenv.Ascii when colors should be suppressed. +func ResolveColorProfile() termenv.Profile { + mode := strings.ToLower(os.Getenv("ROBOREV_COLOR_MODE")) + if mode == "none" || termenv.EnvNoColor() { + return termenv.Ascii + } + return termenv.EnvColorProfile() +} + // Width returns the configured terminal width. func (f *Formatter) Width() int { return f.width @@ -459,7 +487,7 @@ func (f *Formatter) writeText(text string) { return } lines := RenderMarkdownLines( - text, f.width, f.width, f.glamourStyle, 2, + text, f.width, f.width, f.glamourStyle, 2, f.colorProfile, ) for _, line := range lines { f.writef("%s\n", line) @@ -522,7 +550,7 @@ func PrintMarkdownOrPlain(w io.Writer, text string) { } width := TerminalWidth(w) style := GlamourStyle() - lines := RenderMarkdownLines(text, width, width, style, 2) + lines := RenderMarkdownLines(text, width, width, style, 2, ResolveColorProfile()) for _, line := range lines { fmt.Fprintln(w, line) } diff --git a/internal/streamfmt/streamfmt_test.go b/internal/streamfmt/streamfmt_test.go index e0a88bd6a..79101c6a6 100644 --- a/internal/streamfmt/streamfmt_test.go +++ b/internal/streamfmt/streamfmt_test.go @@ -5,9 +5,11 @@ import ( "encoding/json" "fmt" "io" + "regexp" "strings" "testing" + "github.com/muesli/termenv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -880,3 +882,56 @@ func TestFormatterWidth(t *testing.T) { fmtr := NewWithWidth(io.Discard, 42, GlamourStyle()) require.Equal(t, 42, fmtr.Width(), "Width() = %d, want 42", fmtr.Width()) } + +func TestResolveColorProfile(t *testing.T) { + t.Run("NO_COLOR returns Ascii", func(t *testing.T) { + t.Setenv("NO_COLOR", "1") + t.Setenv("ROBOREV_COLOR_MODE", "") + assert.Equal(t, termenv.Ascii, ResolveColorProfile()) + }) + t.Run("ROBOREV_COLOR_MODE=none returns Ascii", func(t *testing.T) { + t.Setenv("NO_COLOR", "") + t.Setenv("ROBOREV_COLOR_MODE", "none") + assert.Equal(t, termenv.Ascii, ResolveColorProfile()) + }) + t.Run("default does not return Ascii", func(t *testing.T) { + t.Setenv("NO_COLOR", "") + t.Setenv("ROBOREV_COLOR_MODE", "") + // In a test environment the profile may vary, but it should not be Ascii + // unless the test runner itself sets NO_COLOR. + profile := ResolveColorProfile() + _ = profile // just ensure no panic + }) +} + +func TestRenderMarkdownLinesNoColor(t *testing.T) { + text := "# Heading\n\nSome **bold** text." + style := GlamourStyle() + lines := RenderMarkdownLines(text, 80, 80, style, 2, termenv.Ascii) + + combined := strings.Join(lines, "\n") + // Match ANSI SGR sequences that set foreground/background colors. + // Bold/reset are acceptable under NO_COLOR convention. + colorSGR := regexp.MustCompile(`\x1b\[(3[0-7]|4[0-7]|9[0-7]|10[0-7]|38;|48;)[0-9;]*m`) + matches := colorSGR.FindAllString(combined, -1) + assert.Empty(t, matches, "expected no ANSI color sequences with Ascii profile, got: %v", matches) +} + +func TestGlamourStyleRespectsColorMode(t *testing.T) { + t.Run("dark mode", func(t *testing.T) { + t.Setenv("ROBOREV_COLOR_MODE", "dark") + style := GlamourStyle() + assert.NotNil(t, style.H1.Color, "dark mode should have heading colors") + }) + t.Run("light mode", func(t *testing.T) { + t.Setenv("ROBOREV_COLOR_MODE", "light") + style := GlamourStyle() + assert.NotNil(t, style.H1.Color, "light mode should have heading colors") + }) + t.Run("none mode", func(t *testing.T) { + t.Setenv("ROBOREV_COLOR_MODE", "none") + // Should not panic; style is still returned (colors stripped by Ascii profile). + style := GlamourStyle() + _ = style + }) +} From 6251457fe5041216a43d94b4d387e3461c64066e Mon Sep 17 00:00:00 2001 From: Sergey Trofimovsky Date: Tue, 24 Mar 2026 04:03:51 +0000 Subject: [PATCH 2/5] Fix NO_COLOR on TUI detail screens and add ROBOREV_COLOR_MODE env var glamour's NewTermRenderer defaults to TrueColor profile, ignoring the NO_COLOR convention. Pass glamour.WithColorProfile() to all term renderers so NO_COLOR=1 strips colors from review/prompt detail screens. Add ROBOREV_COLOR_MODE env var (auto|dark|light|none) for explicit control over the TUI color theme. Apply the setting to both the glamour markdown layer and the lipgloss AdaptiveColor layer via SetHasDarkBackground/SetColorProfile on the default renderer. NO_COLOR takes precedence over ROBOREV_COLOR_MODE at all layers. Consolidate color resolution: remove duplicate resolveColorMode() from TUI helpers, delegate to streamfmt.GlamourStyle() and streamfmt.ResolveColorProfile(). GlamourStyle() now checks NO_COLOR to avoid unnecessary terminal background queries. --- AGENTS.md | 1 + CLAUDE.md | 1 + README.md | 8 ++++ cmd/roborev/tui/helpers.go | 42 +++----------------- cmd/roborev/tui/helpers_test.go | 57 ---------------------------- cmd/roborev/tui/tui.go | 18 +++++++++ internal/streamfmt/streamfmt.go | 14 +++---- internal/streamfmt/streamfmt_test.go | 55 +++++++++++++++++++++------ 8 files changed, 83 insertions(+), 113 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0e87ff9d6..62cf65cc7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,6 +128,7 @@ CLI (roborev) -> HTTP API -> Daemon -> Worker Pool -> Agent adapters - Runtime info: `~/.roborev/daemon.json` - SQLite DB: `~/.roborev/reviews.db` using WAL mode - Data dir override: `ROBOREV_DATA_DIR` +- Color mode: `ROBOREV_COLOR_MODE` env var (`auto`, `dark`, `light`, `none`); `NO_COLOR=1` also supported - Global config: `~/.roborev/config.toml` - Repo config: `.roborev.toml` at repo root - Config precedence is generally: CLI flags -> repo config -> global config -> defaults diff --git a/CLAUDE.md b/CLAUDE.md index 9d99333a7..c72d74544 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,6 +35,7 @@ CLI (roborev) → HTTP API → Daemon (roborev daemon run) → Worker Pool → A - **Storage**: SQLite at `~/.roborev/reviews.db` with WAL mode - **Config**: Global at `~/.roborev/config.toml`, per-repo at `.roborev.toml` - **Data dir**: Set `ROBOREV_DATA_DIR` env var to override `~/.roborev` +- **Color mode**: `ROBOREV_COLOR_MODE=auto|dark|light|none` controls TUI color theme; `NO_COLOR=1` strips all colors - **Runtime info**: Daemon writes PID/addr/port to `~/.roborev/daemon.json` ## Package Map diff --git a/README.md b/README.md index 738975d4b..68cfdb6d2 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,14 @@ Project-specific review instructions here. See [configuration guide](https://roborev.io/configuration/) for all options. +### Environment Variables + +| Variable | Description | +|----------|-------------| +| `ROBOREV_DATA_DIR` | Override default data directory (`~/.roborev`) | +| `ROBOREV_COLOR_MODE` | TUI color theme: `auto` (default), `dark`, `light`, `none` | +| `NO_COLOR` | Set to any value to disable all color output ([no-color.org](https://no-color.org)) | + ## Supported Agents | Agent | Install | diff --git a/cmd/roborev/tui/helpers.go b/cmd/roborev/tui/helpers.go index c4900809c..0910bb4bc 100644 --- a/cmd/roborev/tui/helpers.go +++ b/cmd/roborev/tui/helpers.go @@ -1,7 +1,6 @@ package tui import ( - "os" "regexp" "strings" "time" @@ -9,11 +8,11 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/glamour" gansi "github.com/charmbracelet/glamour/ansi" - "github.com/charmbracelet/glamour/styles" xansi "github.com/charmbracelet/x/ansi" "github.com/mattn/go-runewidth" "github.com/muesli/termenv" "github.com/roborev-dev/roborev/internal/storage" + "github.com/roborev-dev/roborev/internal/streamfmt" ) // Filter type constants used in filterStack and popFilter/pushFilter. @@ -320,44 +319,13 @@ type markdownCache struct { lastPromptMaxScroll int } -// resolveColorMode returns the glamour style config and termenv color profile -// based on the ROBOREV_COLOR_MODE env var and the NO_COLOR convention. -// Valid ROBOREV_COLOR_MODE values: auto (default), dark, light, none. -func resolveColorMode() (gansi.StyleConfig, termenv.Profile) { - mode := strings.ToLower(os.Getenv("ROBOREV_COLOR_MODE")) - - if mode == "none" || termenv.EnvNoColor() { - return styles.DarkStyleConfig, termenv.Ascii - } - - profile := termenv.EnvColorProfile() - switch mode { - case "dark": - return styles.DarkStyleConfig, profile - case "light": - return styles.LightStyleConfig, profile - default: // "auto" or "" - if termenv.HasDarkBackground() { - return styles.DarkStyleConfig, profile - } - return styles.LightStyleConfig, profile - } -} - // newMarkdownCache creates a markdownCache, detecting terminal background // color now (before bubbletea enters raw mode and takes over stdin). -// Builds a custom style with zero margins to avoid extra padding. -// Respects ROBOREV_COLOR_MODE env var and NO_COLOR convention. +// Delegates style and color profile resolution to the streamfmt package, +// which respects ROBOREV_COLOR_MODE env var and NO_COLOR convention. func newMarkdownCache(tabWidth int) *markdownCache { - style, profile := resolveColorMode() - // Remove document and code block margins that add extra indentation. - zeroMargin := uint(0) - style.Document.Margin = &zeroMargin - style.CodeBlock.Margin = &zeroMargin - // Remove inline code prefix/suffix spaces (rendered as visible - // colored blocks around `backtick` content). - style.Code.Prefix = "" - style.Code.Suffix = "" + style := streamfmt.GlamourStyle() + profile := streamfmt.ResolveColorProfile() if tabWidth <= 0 { tabWidth = 2 } else if tabWidth > 16 { diff --git a/cmd/roborev/tui/helpers_test.go b/cmd/roborev/tui/helpers_test.go index 3943c9060..4c613fd95 100644 --- a/cmd/roborev/tui/helpers_test.go +++ b/cmd/roborev/tui/helpers_test.go @@ -434,63 +434,6 @@ func TestRenderMarkdownLinesNoOverflow(t *testing.T) { } } -func TestResolveColorMode(t *testing.T) { - tests := []struct { - name string - colorMode string - noColor string - wantProfile termenv.Profile - wantDarkBase bool // true if DarkStyleConfig should be selected - }{ - { - name: "none mode returns Ascii", - colorMode: "none", - wantProfile: termenv.Ascii, - }, - { - name: "NO_COLOR returns Ascii", - noColor: "1", - wantProfile: termenv.Ascii, - }, - { - name: "dark mode forces dark style", - colorMode: "dark", - wantDarkBase: true, - }, - { - name: "light mode forces light style", - colorMode: "light", - wantDarkBase: false, - }, - { - name: "empty defaults to auto", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.colorMode != "" { - t.Setenv("ROBOREV_COLOR_MODE", tt.colorMode) - } else { - t.Setenv("ROBOREV_COLOR_MODE", "") - } - if tt.noColor != "" { - t.Setenv("NO_COLOR", tt.noColor) - } else { - t.Setenv("NO_COLOR", "") - } - style, profile := resolveColorMode() - if tt.wantProfile != 0 { - assert.Equal(t, tt.wantProfile, profile) - } - if tt.colorMode == "dark" || tt.colorMode == "none" { - // DarkStyleConfig has non-nil Heading.Color - assert.NotNil(t, style.H1.Color, "expected dark-based style") - } - _ = style // ensure no panic - }) - } -} - func TestRenderMarkdownLinesNoColor(t *testing.T) { // When colorProfile is Ascii, output should contain no ANSI color sequences. // Bold/reset sequences (\x1b[;1m, \x1b[0m) are still emitted by glamour diff --git a/cmd/roborev/tui/tui.go b/cmd/roborev/tui/tui.go index 4bff02450..44e9256ac 100644 --- a/cmd/roborev/tui/tui.go +++ b/cmd/roborev/tui/tui.go @@ -18,6 +18,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/charmbracelet/lipgloss/table" "github.com/mattn/go-runewidth" + "github.com/muesli/termenv" "github.com/roborev-dev/roborev/internal/config" "github.com/roborev-dev/roborev/internal/daemon" "github.com/roborev-dev/roborev/internal/git" @@ -527,6 +528,23 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model { filterStack = append(filterStack, filterTypeBranch) } + // Apply ROBOREV_COLOR_MODE to the lipgloss default renderer so that + // AdaptiveColor styles on the queue screen respect the env var. + // NO_COLOR takes precedence per the convention. + // The glamour/markdown layer is handled separately via streamfmt. + if termenv.EnvNoColor() { + lipgloss.SetColorProfile(termenv.Ascii) + } else { + switch strings.ToLower(os.Getenv("ROBOREV_COLOR_MODE")) { + case "dark": + lipgloss.SetHasDarkBackground(true) + case "light": + lipgloss.SetHasDarkBackground(false) + case "none": + lipgloss.SetColorProfile(termenv.Ascii) + } + } + return model{ endpoint: ep, daemonVersion: daemonVersion, diff --git a/internal/streamfmt/streamfmt.go b/internal/streamfmt/streamfmt.go index 357e47234..087a15855 100644 --- a/internal/streamfmt/streamfmt.go +++ b/internal/streamfmt/streamfmt.go @@ -103,18 +103,18 @@ func TerminalWidth(w io.Writer) int { // GlamourStyle returns a glamour style config with zero margins, // matching the TUI's rendering. Detects dark/light background once. -// Respects ROBOREV_COLOR_MODE env var for explicit dark/light/none selection. +// Respects ROBOREV_COLOR_MODE env var and NO_COLOR convention. func GlamourStyle() gansi.StyleConfig { mode := strings.ToLower(os.Getenv("ROBOREV_COLOR_MODE")) var style gansi.StyleConfig - switch mode { - case "dark": - style = styles.DarkStyleConfig - case "light": - style = styles.LightStyleConfig - case "none": + switch { + case mode == "none" || termenv.EnvNoColor(): // Use dark style as base; colors will be stripped by Ascii profile. style = styles.DarkStyleConfig + case mode == "dark": + style = styles.DarkStyleConfig + case mode == "light": + style = styles.LightStyleConfig default: // "auto" or "" style = styles.LightStyleConfig if termenv.HasDarkBackground() { diff --git a/internal/streamfmt/streamfmt_test.go b/internal/streamfmt/streamfmt_test.go index 79101c6a6..5467f08c0 100644 --- a/internal/streamfmt/streamfmt_test.go +++ b/internal/streamfmt/streamfmt_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/charmbracelet/glamour/styles" "github.com/muesli/termenv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -894,13 +895,16 @@ func TestResolveColorProfile(t *testing.T) { t.Setenv("ROBOREV_COLOR_MODE", "none") assert.Equal(t, termenv.Ascii, ResolveColorProfile()) }) - t.Run("default does not return Ascii", func(t *testing.T) { + t.Run("NO_COLOR takes precedence over ROBOREV_COLOR_MODE=dark", func(t *testing.T) { + t.Setenv("NO_COLOR", "1") + t.Setenv("ROBOREV_COLOR_MODE", "dark") + assert.Equal(t, termenv.Ascii, ResolveColorProfile()) + }) + t.Run("default returns env color profile", func(t *testing.T) { t.Setenv("NO_COLOR", "") t.Setenv("ROBOREV_COLOR_MODE", "") - // In a test environment the profile may vary, but it should not be Ascii - // unless the test runner itself sets NO_COLOR. - profile := ResolveColorProfile() - _ = profile // just ensure no panic + // Should match what termenv detects for the current environment. + assert.Equal(t, termenv.EnvColorProfile(), ResolveColorProfile()) }) } @@ -918,20 +922,47 @@ func TestRenderMarkdownLinesNoColor(t *testing.T) { } func TestGlamourStyleRespectsColorMode(t *testing.T) { - t.Run("dark mode", func(t *testing.T) { + // Use the upstream style configs as reference values so tests don't + // break if glamour changes its default color palette. + darkDocColor := *styles.DarkStyleConfig.Document.Color + lightDocColor := *styles.LightStyleConfig.Document.Color + require.NotEqual(t, darkDocColor, lightDocColor, "dark and light Document.Color must differ for this test to be meaningful") + + t.Run("dark mode selects dark style", func(t *testing.T) { t.Setenv("ROBOREV_COLOR_MODE", "dark") + t.Setenv("NO_COLOR", "") style := GlamourStyle() - assert.NotNil(t, style.H1.Color, "dark mode should have heading colors") + require.NotNil(t, style.Document.Color) + assert.Equal(t, darkDocColor, *style.Document.Color) }) - t.Run("light mode", func(t *testing.T) { + t.Run("light mode selects light style", func(t *testing.T) { t.Setenv("ROBOREV_COLOR_MODE", "light") + t.Setenv("NO_COLOR", "") style := GlamourStyle() - assert.NotNil(t, style.H1.Color, "light mode should have heading colors") + require.NotNil(t, style.Document.Color) + assert.Equal(t, lightDocColor, *style.Document.Color) }) - t.Run("none mode", func(t *testing.T) { + t.Run("none mode selects dark style as base", func(t *testing.T) { t.Setenv("ROBOREV_COLOR_MODE", "none") - // Should not panic; style is still returned (colors stripped by Ascii profile). + t.Setenv("NO_COLOR", "") + style := GlamourStyle() + require.NotNil(t, style.Document.Color) + assert.Equal(t, darkDocColor, *style.Document.Color) + }) + t.Run("NO_COLOR selects dark style as base", func(t *testing.T) { + t.Setenv("NO_COLOR", "1") + t.Setenv("ROBOREV_COLOR_MODE", "") + style := GlamourStyle() + require.NotNil(t, style.Document.Color) + assert.Equal(t, darkDocColor, *style.Document.Color) + }) + t.Run("NO_COLOR takes precedence over dark mode", func(t *testing.T) { + t.Setenv("NO_COLOR", "1") + t.Setenv("ROBOREV_COLOR_MODE", "dark") + // NO_COLOR wins: profile should be Ascii, style should be dark base. + assert.Equal(t, termenv.Ascii, ResolveColorProfile()) style := GlamourStyle() - _ = style + require.NotNil(t, style.Document.Color) + assert.Equal(t, darkDocColor, *style.Document.Color) }) } From ed832260999d77fd8dc53afb051d543bedff77a0 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 24 Mar 2026 08:20:27 -0500 Subject: [PATCH 3/5] Strip all SGR sequences in no-color mode to prevent formatting bleed In no-color mode, Glamour still emits non-color SGR sequences (bold, underline) via termenv.Ascii. Previously, stripTrailingPadding only removed trailing SGR sequences and omitted the reset, leaving mid-line attributes open to bleed into subsequent lines. Now strip all SGR sequences from the entire line when noColor is enabled. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/roborev/tui/helpers.go | 8 ++++++-- internal/streamfmt/render.go | 9 +++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/cmd/roborev/tui/helpers.go b/cmd/roborev/tui/helpers.go index 0910bb4bc..1052ad385 100644 --- a/cmd/roborev/tui/helpers.go +++ b/cmd/roborev/tui/helpers.go @@ -471,13 +471,17 @@ func sanitizeLines(lines []string) []string { // the wrap width. Stripping this padding prevents overflow on narrow terminals. var trailingPadRe = regexp.MustCompile(`(\s|\x1b\[[0-9;]*m)+$`) +// allSGRRe matches any ANSI SGR escape sequence. +var allSGRRe = regexp.MustCompile(`\x1b\[[0-9;]*m`) + // stripTrailingPadding removes trailing whitespace and ANSI SGR codes from a // glamour output line, then appends a reset to ensure clean color state. -// When noColor is true, the reset sequence is omitted. +// When noColor is true, all SGR sequences are stripped to prevent open +// attributes (bold, underline) from bleeding across lines. func stripTrailingPadding(line string, noColor bool) string { line = trailingPadRe.ReplaceAllString(line, "") if noColor { - return line + return allSGRRe.ReplaceAllString(line, "") } return line + "\x1b[0m" } diff --git a/internal/streamfmt/render.go b/internal/streamfmt/render.go index e26285638..b15142e50 100644 --- a/internal/streamfmt/render.go +++ b/internal/streamfmt/render.go @@ -87,13 +87,18 @@ func SanitizeLines(lines []string) []string { // terminals. var trailingPadRe = regexp.MustCompile(`(\s|\x1b\[[0-9;]*m)+$`) +// allSGRRe matches any ANSI SGR escape sequence. +var allSGRRe = regexp.MustCompile(`\x1b\[[0-9;]*m`) + // StripTrailingPadding removes trailing whitespace and ANSI SGR codes // from a glamour output line, then appends a reset to ensure clean -// color state. When noColor is true, the reset sequence is omitted. +// color state. When noColor is true, all SGR sequences are stripped +// to prevent open attributes (bold, underline) from bleeding across +// lines. func StripTrailingPadding(line string, noColor bool) string { line = trailingPadRe.ReplaceAllString(line, "") if noColor { - return line + return allSGRRe.ReplaceAllString(line, "") } return line + "\x1b[0m" } From faf0143edb9051a7225130996e7f3358ab283411 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 24 Mar 2026 08:22:49 -0500 Subject: [PATCH 4/5] Add regression tests for stripTrailingPadding in no-color mode Cover mid-line bold/underline SGR stripping, trailing padding removal, mixed sequences, and plain text preservation for both the TUI and streamfmt helpers. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/roborev/tui/helpers_test.go | 63 +++++++++++++++++++++++++++++++ internal/streamfmt/render_test.go | 63 +++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/cmd/roborev/tui/helpers_test.go b/cmd/roborev/tui/helpers_test.go index 4c613fd95..e0bc49a8b 100644 --- a/cmd/roborev/tui/helpers_test.go +++ b/cmd/roborev/tui/helpers_test.go @@ -929,3 +929,66 @@ func TestWrapLine(t *testing.T) { }) } } + +func TestStripTrailingPadding(t *testing.T) { + bold := "\x1b[1m" + underline := "\x1b[4m" + reset := "\x1b[0m" + red := "\x1b[31m" + + tests := []struct { + name string + line string + noColor bool + want string + }{ + { + name: "color mode appends reset", + line: red + "hello" + reset, + noColor: false, + want: red + "hello" + reset, + }, + { + name: "color mode strips trailing padding", + line: red + "hello" + reset + " " + reset, + noColor: false, + want: red + "hello" + reset, + }, + { + name: "noColor strips mid-line bold", + line: bold + "hello" + reset, + noColor: true, + want: "hello", + }, + { + name: "noColor strips mid-line underline", + line: underline + "text" + reset, + noColor: true, + want: "text", + }, + { + name: "noColor strips mixed SGR sequences", + line: bold + underline + "mixed" + reset + " plain", + noColor: true, + want: "mixed plain", + }, + { + name: "noColor preserves plain text", + line: "no formatting here", + noColor: true, + want: "no formatting here", + }, + { + name: "noColor strips trailing padding and all SGR", + line: bold + "word" + reset + " ", + noColor: true, + want: "word", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := stripTrailingPadding(tt.line, tt.noColor) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/internal/streamfmt/render_test.go b/internal/streamfmt/render_test.go index 615cceb12..4a8d5305e 100644 --- a/internal/streamfmt/render_test.go +++ b/internal/streamfmt/render_test.go @@ -191,3 +191,66 @@ func TestRenderJobLog_OpenCodeEvents(t *testing.T) { assertLogNotContains(t, out, "step_finish") assertLogNotContains(t, out, `"type"`) } + +func TestStripTrailingPadding(t *testing.T) { + bold := "\x1b[1m" + underline := "\x1b[4m" + reset := "\x1b[0m" + red := "\x1b[31m" + + tests := []struct { + name string + line string + noColor bool + want string + }{ + { + name: "color mode appends reset", + line: red + "hello" + reset, + noColor: false, + want: red + "hello" + reset, + }, + { + name: "color mode strips trailing padding", + line: red + "hello" + reset + " " + reset, + noColor: false, + want: red + "hello" + reset, + }, + { + name: "noColor strips mid-line bold", + line: bold + "hello" + reset, + noColor: true, + want: "hello", + }, + { + name: "noColor strips mid-line underline", + line: underline + "text" + reset, + noColor: true, + want: "text", + }, + { + name: "noColor strips mixed SGR sequences", + line: bold + underline + "mixed" + reset + " plain", + noColor: true, + want: "mixed plain", + }, + { + name: "noColor preserves plain text", + line: "no formatting here", + noColor: true, + want: "no formatting here", + }, + { + name: "noColor strips trailing padding and all SGR", + line: bold + "word" + reset + " ", + noColor: true, + want: "word", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := StripTrailingPadding(tt.line, tt.noColor) + assert.Equal(t, tt.want, got) + }) + } +} From 27321fe756674f6802dd43fe158332194666ca37 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 24 Mar 2026 08:31:29 -0500 Subject: [PATCH 5/5] Fix stale test comments and tighten no-color SGR assertions The TestRenderMarkdownLinesNoColor tests claimed bold/reset sequences were preserved in Ascii mode, but stripTrailingPadding now strips all SGR. Update comments to reflect the current behavior and widen the regex to assert no SGR sequences of any kind remain. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/roborev/tui/helpers_test.go | 15 ++++++--------- internal/streamfmt/streamfmt_test.go | 11 ++++++----- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/cmd/roborev/tui/helpers_test.go b/cmd/roborev/tui/helpers_test.go index e0bc49a8b..59dd97034 100644 --- a/cmd/roborev/tui/helpers_test.go +++ b/cmd/roborev/tui/helpers_test.go @@ -435,19 +435,16 @@ func TestRenderMarkdownLinesNoOverflow(t *testing.T) { } func TestRenderMarkdownLinesNoColor(t *testing.T) { - // When colorProfile is Ascii, output should contain no ANSI color sequences. - // Bold/reset sequences (\x1b[;1m, \x1b[0m) are still emitted by glamour - // for text formatting — these are not colors and are acceptable under NO_COLOR. + // When colorProfile is Ascii, stripTrailingPadding removes all SGR + // sequences (colors, bold, underline, reset) so no formatting can + // bleed across lines. text := "# Heading\n\nSome **bold** text and `code`." lines := renderMarkdownLines(text, 80, 80, styles.DarkStyleConfig, 2, termenv.Ascii) combined := strings.Join(lines, "\n") - // Match ANSI SGR sequences that set foreground/background colors: - // \x1b[3Xm (fg), \x1b[4Xm (bg), \x1b[9Xm (bright fg), \x1b[10Xm (bright bg), - // \x1b[38;...m (extended fg), \x1b[48;...m (extended bg). - colorSGR := regexp.MustCompile(`\x1b\[(3[0-7]|4[0-7]|9[0-7]|10[0-7]|38;|48;)[0-9;]*m`) - matches := colorSGR.FindAllString(combined, -1) - assert.Empty(t, matches, "expected no ANSI color sequences with Ascii profile, got: %v", matches) + allSGR := regexp.MustCompile(`\x1b\[[0-9;]*m`) + matches := allSGR.FindAllString(combined, -1) + assert.Empty(t, matches, "expected no SGR sequences with Ascii profile, got: %v", matches) } func TestReflowHelpRows(t *testing.T) { diff --git a/internal/streamfmt/streamfmt_test.go b/internal/streamfmt/streamfmt_test.go index 5467f08c0..b8e04b02a 100644 --- a/internal/streamfmt/streamfmt_test.go +++ b/internal/streamfmt/streamfmt_test.go @@ -909,16 +909,17 @@ func TestResolveColorProfile(t *testing.T) { } func TestRenderMarkdownLinesNoColor(t *testing.T) { + // When colorProfile is Ascii, StripTrailingPadding removes all SGR + // sequences (colors, bold, underline, reset) so no formatting can + // bleed across lines. text := "# Heading\n\nSome **bold** text." style := GlamourStyle() lines := RenderMarkdownLines(text, 80, 80, style, 2, termenv.Ascii) combined := strings.Join(lines, "\n") - // Match ANSI SGR sequences that set foreground/background colors. - // Bold/reset are acceptable under NO_COLOR convention. - colorSGR := regexp.MustCompile(`\x1b\[(3[0-7]|4[0-7]|9[0-7]|10[0-7]|38;|48;)[0-9;]*m`) - matches := colorSGR.FindAllString(combined, -1) - assert.Empty(t, matches, "expected no ANSI color sequences with Ascii profile, got: %v", matches) + allSGR := regexp.MustCompile(`\x1b\[[0-9;]*m`) + matches := allSGR.FindAllString(combined, -1) + assert.Empty(t, matches, "expected no SGR sequences with Ascii profile, got: %v", matches) } func TestGlamourStyleRespectsColorMode(t *testing.T) {