Skip to content

Commit c6ad86f

Browse files
strofimovskystrofimovsky-trulioowesmclaude
authored
Fix NO_COLOR on TUI detail screens and add ROBOREV_COLOR_MODE env var (#566)
## Summary - Fix `NO_COLOR=1` not working on TUI review/prompt detail screens (glamour was ignoring it) - Add `ROBOREV_COLOR_MODE` env var (`auto`|`dark`|`light`|`none`) for explicit TUI color theme control - Apply color mode to both glamour (markdown) and lipgloss (AdaptiveColor) layers - `NO_COLOR` takes precedence over `ROBOREV_COLOR_MODE` at all layers ## Problem glamour's `NewTermRenderer` defaults to `TrueColor` profile regardless of `NO_COLOR`. The main queue screen (lipgloss only) correctly respected `NO_COLOR`, but review/prompt detail screens (glamour-rendered markdown) always emitted ANSI color sequences. Additionally, there was no way to force a dark/light theme when terminal background auto-detection failed. ## Changes ### Core fix - Pass `glamour.WithColorProfile()` to all `glamour.NewTermRenderer` calls in both `cmd/roborev/tui/helpers.go` and `internal/streamfmt/render.go` - Fix `stripTrailingPadding`/`StripTrailingPadding` to skip the `\x1b[0m` reset suffix in no-color mode ### ROBOREV_COLOR_MODE env var - `auto` (default): auto-detect terminal background - `dark`: force dark color palette - `light`: force light color palette - `none`: strip all colors (same as `NO_COLOR=1`) ### Lipgloss integration - Call `lipgloss.SetHasDarkBackground()` / `lipgloss.SetColorProfile()` in TUI startup so `AdaptiveColor` styles on the queue screen also respect the env var - Check `NO_COLOR` first to ensure it always takes precedence ### Consolidation - Remove duplicate `resolveColorMode()` from TUI helpers; delegate to `streamfmt.GlamourStyle()` + `streamfmt.ResolveColorProfile()` - Update `GlamourStyle()` to check `NO_COLOR` (previously skipped, causing unnecessary terminal background queries) ### Documentation - Add `ROBOREV_COLOR_MODE` and `NO_COLOR` to README.md, AGENTS.md, and CLAUDE.md ## Testing - All existing tests pass (`go test ./...`) - New tests: `TestResolveColorProfile`, `TestRenderMarkdownLinesNoColor`, `TestGlamourStyleRespectsColorMode` with proper dark/light style differentiation - Manual testing confirmed: `NO_COLOR=1`, `ROBOREV_COLOR_MODE=none`, `=dark`, `=light` all work on both queue and detail screens --------- Co-authored-by: Sergey Trofimovsky <sergey.trofimovsky@trulioo.com> Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent cbcbb38 commit c6ad86f

File tree

10 files changed

+337
-36
lines changed

10 files changed

+337
-36
lines changed

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ CLI (roborev) -> HTTP API -> Daemon -> Worker Pool -> Agent adapters
128128
- Runtime info: `~/.roborev/daemon.json`
129129
- SQLite DB: `~/.roborev/reviews.db` using WAL mode
130130
- Data dir override: `ROBOREV_DATA_DIR`
131+
- Color mode: `ROBOREV_COLOR_MODE` env var (`auto`, `dark`, `light`, `none`); `NO_COLOR=1` also supported
131132
- Global config: `~/.roborev/config.toml`
132133
- Repo config: `.roborev.toml` at repo root
133134
- Config precedence is generally: CLI flags -> repo config -> global config -> defaults

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ CLI (roborev) → HTTP API → Daemon (roborev daemon run) → Worker Pool → A
3535
- **Storage**: SQLite at `~/.roborev/reviews.db` with WAL mode
3636
- **Config**: Global at `~/.roborev/config.toml`, per-repo at `.roborev.toml`
3737
- **Data dir**: Set `ROBOREV_DATA_DIR` env var to override `~/.roborev`
38+
- **Color mode**: `ROBOREV_COLOR_MODE=auto|dark|light|none` controls TUI color theme; `NO_COLOR=1` strips all colors
3839
- **Runtime info**: Daemon writes PID/addr/port to `~/.roborev/daemon.json`
3940

4041
## Package Map

README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,14 @@ Project-specific review instructions here.
162162

163163
See [configuration guide](https://roborev.io/configuration/) for all options.
164164

165+
### Environment Variables
166+
167+
| Variable | Description |
168+
|----------|-------------|
169+
| `ROBOREV_DATA_DIR` | Override default data directory (`~/.roborev`) |
170+
| `ROBOREV_COLOR_MODE` | TUI color theme: `auto` (default), `dark`, `light`, `none` |
171+
| `NO_COLOR` | Set to any value to disable all color output ([no-color.org](https://no-color.org)) |
172+
165173
## Supported Agents
166174

167175
| Agent | Install |

cmd/roborev/tui/helpers.go

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import (
88
tea "github.com/charmbracelet/bubbletea"
99
"github.com/charmbracelet/glamour"
1010
gansi "github.com/charmbracelet/glamour/ansi"
11-
"github.com/charmbracelet/glamour/styles"
1211
xansi "github.com/charmbracelet/x/ansi"
1312
"github.com/mattn/go-runewidth"
1413
"github.com/muesli/termenv"
1514
"github.com/roborev-dev/roborev/internal/storage"
15+
"github.com/roborev-dev/roborev/internal/streamfmt"
1616
)
1717

1818
// Filter type constants used in filterStack and popFilter/pushFilter.
@@ -299,6 +299,7 @@ func wrapLine(line string, width int) []string {
299299
// which blocks for seconds inside bubbletea's raw-mode input loop.
300300
type markdownCache struct {
301301
glamourStyle gansi.StyleConfig // custom style derived from dark/light, detected once at init
302+
colorProfile termenv.Profile // color profile for glamour rendering (Ascii when NO_COLOR)
302303
tabWidth int // tab expansion width (default 2)
303304

304305
reviewLines []string
@@ -320,26 +321,17 @@ type markdownCache struct {
320321

321322
// newMarkdownCache creates a markdownCache, detecting terminal background
322323
// color now (before bubbletea enters raw mode and takes over stdin).
323-
// Builds a custom style with zero margins to avoid extra padding.
324+
// Delegates style and color profile resolution to the streamfmt package,
325+
// which respects ROBOREV_COLOR_MODE env var and NO_COLOR convention.
324326
func newMarkdownCache(tabWidth int) *markdownCache {
325-
style := styles.LightStyleConfig
326-
if termenv.HasDarkBackground() {
327-
style = styles.DarkStyleConfig
328-
}
329-
// Remove document and code block margins that add extra indentation.
330-
zeroMargin := uint(0)
331-
style.Document.Margin = &zeroMargin
332-
style.CodeBlock.Margin = &zeroMargin
333-
// Remove inline code prefix/suffix spaces (rendered as visible
334-
// colored blocks around `backtick` content).
335-
style.Code.Prefix = ""
336-
style.Code.Suffix = ""
327+
style := streamfmt.GlamourStyle()
328+
profile := streamfmt.ResolveColorProfile()
337329
if tabWidth <= 0 {
338330
tabWidth = 2
339331
} else if tabWidth > 16 {
340332
tabWidth = 16
341333
}
342-
return &markdownCache{glamourStyle: style, tabWidth: tabWidth}
334+
return &markdownCache{glamourStyle: style, colorProfile: profile, tabWidth: tabWidth}
343335
}
344336

345337
// truncateLongLines normalizes tabs and truncates lines inside fenced code
@@ -479,24 +471,35 @@ func sanitizeLines(lines []string) []string {
479471
// the wrap width. Stripping this padding prevents overflow on narrow terminals.
480472
var trailingPadRe = regexp.MustCompile(`(\s|\x1b\[[0-9;]*m)+$`)
481473

474+
// allSGRRe matches any ANSI SGR escape sequence.
475+
var allSGRRe = regexp.MustCompile(`\x1b\[[0-9;]*m`)
476+
482477
// stripTrailingPadding removes trailing whitespace and ANSI SGR codes from a
483478
// glamour output line, then appends a reset to ensure clean color state.
484-
func stripTrailingPadding(line string) string {
485-
return trailingPadRe.ReplaceAllString(line, "") + "\x1b[0m"
479+
// When noColor is true, all SGR sequences are stripped to prevent open
480+
// attributes (bold, underline) from bleeding across lines.
481+
func stripTrailingPadding(line string, noColor bool) string {
482+
line = trailingPadRe.ReplaceAllString(line, "")
483+
if noColor {
484+
return allSGRRe.ReplaceAllString(line, "")
485+
}
486+
return line + "\x1b[0m"
486487
}
487488

488489
// renderMarkdownLines renders markdown text using glamour and splits into lines.
489490
// wrapWidth controls glamour's word-wrap column (capped for readability).
490491
// maxWidth controls line truncation (actual terminal width).
492+
// colorProfile controls glamour's color output (use termenv.Ascii to suppress colors).
491493
// Falls back to wrapText if glamour rendering fails.
492-
func renderMarkdownLines(text string, wrapWidth, maxWidth int, glamourStyle gansi.StyleConfig, tabWidth int) []string {
494+
func renderMarkdownLines(text string, wrapWidth, maxWidth int, glamourStyle gansi.StyleConfig, tabWidth int, colorProfile termenv.Profile) []string {
493495
// Truncate long lines before glamour so they don't get word-wrapped.
494496
// Use maxWidth (terminal width) so content fills the available space.
495497
text = truncateLongLines(text, maxWidth, tabWidth)
496498
r, err := glamour.NewTermRenderer(
497499
glamour.WithStyles(glamourStyle),
498500
glamour.WithWordWrap(wrapWidth),
499501
glamour.WithPreservedNewLines(),
502+
glamour.WithColorProfile(colorProfile),
500503
)
501504
if err != nil {
502505
return sanitizeLines(wrapText(text, wrapWidth))
@@ -505,9 +508,10 @@ func renderMarkdownLines(text string, wrapWidth, maxWidth int, glamourStyle gans
505508
if err != nil {
506509
return sanitizeLines(wrapText(text, wrapWidth))
507510
}
511+
noColor := colorProfile == termenv.Ascii
508512
lines := strings.Split(strings.TrimRight(out, "\n"), "\n")
509513
for i, line := range lines {
510-
line = stripTrailingPadding(line)
514+
line = stripTrailingPadding(line, noColor)
511515
line = sanitizeEscapes(line)
512516
// Truncate output lines that still exceed maxWidth (glamour can add
513517
// indentation for block quotes, lists, etc. beyond the wrap width).
@@ -526,7 +530,7 @@ func (c *markdownCache) getReviewLines(text string, wrapWidth, maxWidth int, rev
526530
if c.reviewID == reviewID && c.reviewWidth == maxWidth && c.reviewText == text {
527531
return c.reviewLines
528532
}
529-
c.reviewLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth)
533+
c.reviewLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth, c.colorProfile)
530534
c.reviewID = reviewID
531535
c.reviewWidth = maxWidth
532536
c.reviewText = text
@@ -540,7 +544,7 @@ func (c *markdownCache) getPromptLines(text string, wrapWidth, maxWidth int, rev
540544
if c.promptID == reviewID && c.promptWidth == maxWidth && c.promptText == text {
541545
return c.promptLines
542546
}
543-
c.promptLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth)
547+
c.promptLines = renderMarkdownLines(text, wrapWidth, maxWidth, c.glamourStyle, c.tabWidth, c.colorProfile)
544548
c.promptID = reviewID
545549
c.promptWidth = maxWidth
546550
c.promptText = text

cmd/roborev/tui/helpers_test.go

Lines changed: 81 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
tea "github.com/charmbracelet/bubbletea"
1111
"github.com/charmbracelet/glamour/styles"
1212
"github.com/mattn/go-runewidth"
13+
"github.com/muesli/termenv"
1314
"github.com/roborev-dev/roborev/internal/storage"
1415
"github.com/stretchr/testify/assert"
1516
"github.com/stretchr/testify/require"
@@ -23,7 +24,7 @@ func stripTestANSI(s string) string {
2324

2425
func TestRenderMarkdownLinesPreservesNewlines(t *testing.T) {
2526
// Verify that single newlines in plain text are preserved (not collapsed into one paragraph)
26-
lines := renderMarkdownLines("Line 1\nLine 2\nLine 3", 80, 80, styles.DarkStyleConfig, 2)
27+
lines := renderMarkdownLines("Line 1\nLine 2\nLine 3", 80, 80, styles.DarkStyleConfig, 2, termenv.TrueColor)
2728

2829
found := 0
2930
for _, line := range lines {
@@ -36,7 +37,7 @@ func TestRenderMarkdownLinesPreservesNewlines(t *testing.T) {
3637
}
3738

3839
func TestRenderMarkdownLinesFallsBackOnEmpty(t *testing.T) {
39-
lines := renderMarkdownLines("", 80, 80, styles.DarkStyleConfig, 2)
40+
lines := renderMarkdownLines("", 80, 80, styles.DarkStyleConfig, 2, termenv.TrueColor)
4041
// Should not panic and should produce some output (even if empty)
4142
assert.NotNil(t, lines)
4243
}
@@ -304,7 +305,7 @@ func TestRenderMarkdownLinesPreservesLongProse(t *testing.T) {
304305
// Long prose lines should be word-wrapped by glamour, not truncated.
305306
// All words must appear in the rendered output.
306307
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"
307-
lines := renderMarkdownLines(longProse, 60, 80, styles.DarkStyleConfig, 2)
308+
lines := renderMarkdownLines(longProse, 60, 80, styles.DarkStyleConfig, 2, termenv.TrueColor)
308309

309310
var combined strings.Builder
310311
for _, line := range lines {
@@ -423,7 +424,7 @@ func TestRenderMarkdownLinesNoOverflow(t *testing.T) {
423424
longLine := strings.Repeat("x", 200)
424425
text := "Review:\n\n```\n" + longLine + "\n```\n"
425426
width := 76
426-
lines := renderMarkdownLines(text, width, width, styles.DarkStyleConfig, 2)
427+
lines := renderMarkdownLines(text, width, width, styles.DarkStyleConfig, 2, termenv.TrueColor)
427428

428429
for i, line := range lines {
429430
stripped := stripTestANSI(line)
@@ -433,6 +434,19 @@ func TestRenderMarkdownLinesNoOverflow(t *testing.T) {
433434
}
434435
}
435436

437+
func TestRenderMarkdownLinesNoColor(t *testing.T) {
438+
// When colorProfile is Ascii, stripTrailingPadding removes all SGR
439+
// sequences (colors, bold, underline, reset) so no formatting can
440+
// bleed across lines.
441+
text := "# Heading\n\nSome **bold** text and `code`."
442+
lines := renderMarkdownLines(text, 80, 80, styles.DarkStyleConfig, 2, termenv.Ascii)
443+
444+
combined := strings.Join(lines, "\n")
445+
allSGR := regexp.MustCompile(`\x1b\[[0-9;]*m`)
446+
matches := allSGR.FindAllString(combined, -1)
447+
assert.Empty(t, matches, "expected no SGR sequences with Ascii profile, got: %v", matches)
448+
}
449+
436450
func TestReflowHelpRows(t *testing.T) {
437451

438452
tests := []struct {
@@ -912,3 +926,66 @@ func TestWrapLine(t *testing.T) {
912926
})
913927
}
914928
}
929+
930+
func TestStripTrailingPadding(t *testing.T) {
931+
bold := "\x1b[1m"
932+
underline := "\x1b[4m"
933+
reset := "\x1b[0m"
934+
red := "\x1b[31m"
935+
936+
tests := []struct {
937+
name string
938+
line string
939+
noColor bool
940+
want string
941+
}{
942+
{
943+
name: "color mode appends reset",
944+
line: red + "hello" + reset,
945+
noColor: false,
946+
want: red + "hello" + reset,
947+
},
948+
{
949+
name: "color mode strips trailing padding",
950+
line: red + "hello" + reset + " " + reset,
951+
noColor: false,
952+
want: red + "hello" + reset,
953+
},
954+
{
955+
name: "noColor strips mid-line bold",
956+
line: bold + "hello" + reset,
957+
noColor: true,
958+
want: "hello",
959+
},
960+
{
961+
name: "noColor strips mid-line underline",
962+
line: underline + "text" + reset,
963+
noColor: true,
964+
want: "text",
965+
},
966+
{
967+
name: "noColor strips mixed SGR sequences",
968+
line: bold + underline + "mixed" + reset + " plain",
969+
noColor: true,
970+
want: "mixed plain",
971+
},
972+
{
973+
name: "noColor preserves plain text",
974+
line: "no formatting here",
975+
noColor: true,
976+
want: "no formatting here",
977+
},
978+
{
979+
name: "noColor strips trailing padding and all SGR",
980+
line: bold + "word" + reset + " ",
981+
noColor: true,
982+
want: "word",
983+
},
984+
}
985+
for _, tt := range tests {
986+
t.Run(tt.name, func(t *testing.T) {
987+
got := stripTrailingPadding(tt.line, tt.noColor)
988+
assert.Equal(t, tt.want, got)
989+
})
990+
}
991+
}

cmd/roborev/tui/tui.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/charmbracelet/lipgloss"
1919
"github.com/charmbracelet/lipgloss/table"
2020
"github.com/mattn/go-runewidth"
21+
"github.com/muesli/termenv"
2122
"github.com/roborev-dev/roborev/internal/config"
2223
"github.com/roborev-dev/roborev/internal/daemon"
2324
"github.com/roborev-dev/roborev/internal/git"
@@ -527,6 +528,23 @@ func newModel(ep daemon.DaemonEndpoint, opts ...option) model {
527528
filterStack = append(filterStack, filterTypeBranch)
528529
}
529530

531+
// Apply ROBOREV_COLOR_MODE to the lipgloss default renderer so that
532+
// AdaptiveColor styles on the queue screen respect the env var.
533+
// NO_COLOR takes precedence per the convention.
534+
// The glamour/markdown layer is handled separately via streamfmt.
535+
if termenv.EnvNoColor() {
536+
lipgloss.SetColorProfile(termenv.Ascii)
537+
} else {
538+
switch strings.ToLower(os.Getenv("ROBOREV_COLOR_MODE")) {
539+
case "dark":
540+
lipgloss.SetHasDarkBackground(true)
541+
case "light":
542+
lipgloss.SetHasDarkBackground(false)
543+
case "none":
544+
lipgloss.SetColorProfile(termenv.Ascii)
545+
}
546+
}
547+
530548
return model{
531549
endpoint: ep,
532550
daemonVersion: daemonVersion,

internal/streamfmt/render.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
gansi "github.com/charmbracelet/glamour/ansi"
99
xansi "github.com/charmbracelet/x/ansi"
1010
"github.com/mattn/go-runewidth"
11+
"github.com/muesli/termenv"
1112
)
1213

1314
// ansiEscapePattern matches ANSI escape sequences (colors, cursor
@@ -86,11 +87,20 @@ func SanitizeLines(lines []string) []string {
8687
// terminals.
8788
var trailingPadRe = regexp.MustCompile(`(\s|\x1b\[[0-9;]*m)+$`)
8889

90+
// allSGRRe matches any ANSI SGR escape sequence.
91+
var allSGRRe = regexp.MustCompile(`\x1b\[[0-9;]*m`)
92+
8993
// StripTrailingPadding removes trailing whitespace and ANSI SGR codes
9094
// from a glamour output line, then appends a reset to ensure clean
91-
// color state.
92-
func StripTrailingPadding(line string) string {
93-
return trailingPadRe.ReplaceAllString(line, "") + "\x1b[0m"
95+
// color state. When noColor is true, all SGR sequences are stripped
96+
// to prevent open attributes (bold, underline) from bleeding across
97+
// lines.
98+
func StripTrailingPadding(line string, noColor bool) string {
99+
line = trailingPadRe.ReplaceAllString(line, "")
100+
if noColor {
101+
return allSGRRe.ReplaceAllString(line, "")
102+
}
103+
return line + "\x1b[0m"
94104
}
95105

96106
// WrapText wraps text to the specified width, preserving existing
@@ -233,17 +243,20 @@ func ParseFence(line string) (byte, int, bool) {
233243

234244
// RenderMarkdownLines renders markdown text using glamour and splits
235245
// into lines. wrapWidth controls glamour's word-wrap column.
236-
// maxWidth controls line truncation (actual terminal width). Falls
237-
// back to WrapText if glamour rendering fails.
246+
// maxWidth controls line truncation (actual terminal width).
247+
// colorProfile controls glamour's color output (use termenv.Ascii to suppress colors).
248+
// Falls back to WrapText if glamour rendering fails.
238249
func RenderMarkdownLines(
239250
text string, wrapWidth, maxWidth int,
240251
glamourStyle gansi.StyleConfig, tabWidth int,
252+
colorProfile termenv.Profile,
241253
) []string {
242254
text = TruncateLongLines(text, maxWidth, tabWidth)
243255
r, err := glamour.NewTermRenderer(
244256
glamour.WithStyles(glamourStyle),
245257
glamour.WithWordWrap(wrapWidth),
246258
glamour.WithPreservedNewLines(),
259+
glamour.WithColorProfile(colorProfile),
247260
)
248261
if err != nil {
249262
return SanitizeLines(WrapText(text, wrapWidth))
@@ -252,9 +265,10 @@ func RenderMarkdownLines(
252265
if err != nil {
253266
return SanitizeLines(WrapText(text, wrapWidth))
254267
}
268+
noColor := colorProfile == termenv.Ascii
255269
lines := strings.Split(strings.TrimRight(out, "\n"), "\n")
256270
for i, line := range lines {
257-
line = StripTrailingPadding(line)
271+
line = StripTrailingPadding(line, noColor)
258272
line = SanitizeEscapes(line)
259273
if xansi.StringWidth(line) > maxWidth {
260274
line = xansi.Truncate(line, maxWidth, "")

0 commit comments

Comments
 (0)