fix(theme): preserve palette foreground styles
- commit
- bd7ecc43ae76bbb1d7a73468574f5034f1ba667d
- author
- brice · · 2026-07-17 09:24 UTC
- parent
- adce876
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +94 -21
M internal/usecase/theme/styles_test.go +68 -0
| @@ -390,6 +390,74 @@ func TestPaletteTitleHueDerivation(t *testing.T) { | |||
| 390 | 390 | } | |
| 391 | 391 | } | |
| 392 | 392 | ||
| 393 | + | func TestPaletteForegroundStylesKeepTheirRenderedBackgrounds(t *testing.T) { | |
| 394 | + | base := Theme{ | |
| 395 | + | Foreground: renderer.RGB{R: 240, G: 240, B: 240}, | |
| 396 | + | Background: renderer.RGB{R: 50, G: 50, B: 50}, | |
| 397 | + | HasFG: true, HasBG: true, Known: true, TrueColor: true, UsePalette: true, | |
| 398 | + | } | |
| 399 | + | ||
| 400 | + | tests := []struct { | |
| 401 | + | name string | |
| 402 | + | theme Theme | |
| 403 | + | wantForeground renderer.RGB | |
| 404 | + | wantIndexed int | |
| 405 | + | }{ | |
| 406 | + | { | |
| 407 | + | name: "RGB palette foreground", | |
| 408 | + | theme: func() Theme { t := base; t.PaletteKnown = 1 << ansiBlue; return t }(), | |
| 409 | + | wantForeground: Blend(base.Foreground, renderer.RGB{}, titleHueBlend), | |
| 410 | + | wantIndexed: -1, | |
| 411 | + | }, | |
| 412 | + | { | |
| 413 | + | name: "indexed foreground fallback", | |
| 414 | + | theme: func() Theme { t := base; t.SchemeKnown = true; return t }(), | |
| 415 | + | wantIndexed: ansiBrightBlue, | |
| 416 | + | }, | |
| 417 | + | } | |
| 418 | + | ||
| 419 | + | for _, tt := range tests { | |
| 420 | + | t.Run(tt.name, func(t *testing.T) { | |
| 421 | + | styles := NewStyles(tt.theme) | |
| 422 | + | ||
| 423 | + | for name, style := range map[string]renderer.Style{ | |
| 424 | + | "border": styles.Border, | |
| 425 | + | "muted text": styles.PaletteDesc, | |
| 426 | + | } { | |
| 427 | + | require.False(t, style.HasBackgroundRGB, "%s must remain foreground-only", name) | |
| 428 | + | if tt.wantIndexed >= 0 { | |
| 429 | + | require.False(t, style.HasForegroundRGB, "%s must use an indexed foreground", name) | |
| 430 | + | require.Equal(t, tt.wantIndexed, style.Foreground, name) | |
| 431 | + | } else { | |
| 432 | + | require.True(t, style.HasForegroundRGB, "%s must use the RGB palette foreground", name) | |
| 433 | + | require.Equal(t, tt.wantForeground, style.ForegroundRGB, name) | |
| 434 | + | } | |
| 435 | + | } | |
| 436 | + | ||
| 437 | + | status := StatusBarStyle(tt.theme) | |
| 438 | + | accent := AccentStyle(tt.theme) | |
| 439 | + | for name, style := range map[string]renderer.Style{ | |
| 440 | + | "title": styles.TabTitle, | |
| 441 | + | "active title": styles.TabTitleActive, | |
| 442 | + | } { | |
| 443 | + | wantBackground := status.BackgroundRGB | |
| 444 | + | if name == "active title" { | |
| 445 | + | wantBackground = accent.BackgroundRGB | |
| 446 | + | } | |
| 447 | + | require.True(t, style.HasBackgroundRGB, "%s must retain its title background", name) | |
| 448 | + | require.Equal(t, wantBackground, style.BackgroundRGB, name) | |
| 449 | + | if tt.wantIndexed >= 0 { | |
| 450 | + | require.False(t, style.HasForegroundRGB, "%s must use an indexed foreground", name) | |
| 451 | + | require.Equal(t, tt.wantIndexed, style.Foreground, name) | |
| 452 | + | } else { | |
| 453 | + | require.True(t, style.HasForegroundRGB, "%s must use the RGB palette foreground", name) | |
| 454 | + | require.Equal(t, tt.wantForeground, style.ForegroundRGB, name) | |
| 455 | + | } | |
| 456 | + | } | |
| 457 | + | }) | |
| 458 | + | } | |
| 459 | + | } | |
| 460 | + | ||
| 393 | 461 | func TestPaletteIndexedForegroundFallback(t *testing.T) { | |
| 394 | 462 | base := Theme{UsePalette: true, SchemeKnown: true, Known: true} | |
| 395 | 463 | ||
| @@ -390,6 +390,74 @@ func TestPaletteTitleHueDerivation(t *testing.T) { | |||||
| 390 | } | 390 | } | ||
| 391 | } | 391 | } | ||
| 392 | 392 | ||||
| 393 | + | func TestPaletteForegroundStylesKeepTheirRenderedBackgrounds(t *testing.T) { | |||
| 394 | + | base := Theme{ | |||
| 395 | + | Foreground: renderer.RGB{R: 240, G: 240, B: 240}, | |||
| 396 | + | Background: renderer.RGB{R: 50, G: 50, B: 50}, | |||
| 397 | + | HasFG: true, HasBG: true, Known: true, TrueColor: true, UsePalette: true, | |||
| 398 | + | } | |||
| 399 | + | ||||
| 400 | + | tests := []struct { | |||
| 401 | + | name string | |||
| 402 | + | theme Theme | |||
| 403 | + | wantForeground renderer.RGB | |||
| 404 | + | wantIndexed int | |||
| 405 | + | }{ | |||
| 406 | + | { | |||
| 407 | + | name: "RGB palette foreground", | |||
| 408 | + | theme: func() Theme { t := base; t.PaletteKnown = 1 << ansiBlue; return t }(), | |||
| 409 | + | wantForeground: Blend(base.Foreground, renderer.RGB{}, titleHueBlend), | |||
| 410 | + | wantIndexed: -1, | |||
| 411 | + | }, | |||
| 412 | + | { | |||
| 413 | + | name: "indexed foreground fallback", | |||
| 414 | + | theme: func() Theme { t := base; t.SchemeKnown = true; return t }(), | |||
| 415 | + | wantIndexed: ansiBrightBlue, | |||
| 416 | + | }, | |||
| 417 | + | } | |||
| 418 | + | ||||
| 419 | + | for _, tt := range tests { | |||
| 420 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 421 | + | styles := NewStyles(tt.theme) | |||
| 422 | + | ||||
| 423 | + | for name, style := range map[string]renderer.Style{ | |||
| 424 | + | "border": styles.Border, | |||
| 425 | + | "muted text": styles.PaletteDesc, | |||
| 426 | + | } { | |||
| 427 | + | require.False(t, style.HasBackgroundRGB, "%s must remain foreground-only", name) | |||
| 428 | + | if tt.wantIndexed >= 0 { | |||
| 429 | + | require.False(t, style.HasForegroundRGB, "%s must use an indexed foreground", name) | |||
| 430 | + | require.Equal(t, tt.wantIndexed, style.Foreground, name) | |||
| 431 | + | } else { | |||
| 432 | + | require.True(t, style.HasForegroundRGB, "%s must use the RGB palette foreground", name) | |||
| 433 | + | require.Equal(t, tt.wantForeground, style.ForegroundRGB, name) | |||
| 434 | + | } | |||
| 435 | + | } | |||
| 436 | + | ||||
| 437 | + | status := StatusBarStyle(tt.theme) | |||
| 438 | + | accent := AccentStyle(tt.theme) | |||
| 439 | + | for name, style := range map[string]renderer.Style{ | |||
| 440 | + | "title": styles.TabTitle, | |||
| 441 | + | "active title": styles.TabTitleActive, | |||
| 442 | + | } { | |||
| 443 | + | wantBackground := status.BackgroundRGB | |||
| 444 | + | if name == "active title" { | |||
| 445 | + | wantBackground = accent.BackgroundRGB | |||
| 446 | + | } | |||
| 447 | + | require.True(t, style.HasBackgroundRGB, "%s must retain its title background", name) | |||
| 448 | + | require.Equal(t, wantBackground, style.BackgroundRGB, name) | |||
| 449 | + | if tt.wantIndexed >= 0 { | |||
| 450 | + | require.False(t, style.HasForegroundRGB, "%s must use an indexed foreground", name) | |||
| 451 | + | require.Equal(t, tt.wantIndexed, style.Foreground, name) | |||
| 452 | + | } else { | |||
| 453 | + | require.True(t, style.HasForegroundRGB, "%s must use the RGB palette foreground", name) | |||
| 454 | + | require.Equal(t, tt.wantForeground, style.ForegroundRGB, name) | |||
| 455 | + | } | |||
| 456 | + | } | |||
| 457 | + | }) | |||
| 458 | + | } | |||
| 459 | + | } | |||
| 460 | + | ||||
| 393 | func TestPaletteIndexedForegroundFallback(t *testing.T) { | 461 | func TestPaletteIndexedForegroundFallback(t *testing.T) { | ||
| 394 | base := Theme{UsePalette: true, SchemeKnown: true, Known: true} | 462 | base := Theme{UsePalette: true, SchemeKnown: true, Known: true} | ||
| 395 | 463 | ||||
M internal/usecase/theme/theme.go +19 -19
| @@ -86,32 +86,31 @@ func ParseXColor(s string) (renderer.RGB, bool) { | |||
| 86 | 86 | if len(parts) != 3 { | |
| 87 | 87 | return renderer.RGB{}, false | |
| 88 | 88 | } | |
| 89 | - | width := len(parts[0]) | |
| 90 | - | if width != 2 && width != 4 { | |
| 91 | - | return renderer.RGB{}, false | |
| 92 | - | } | |
| 93 | 89 | var out [3]uint8 | |
| 94 | 90 | for i, part := range parts { | |
| 95 | - | if len(part) != width { | |
| 96 | - | return renderer.RGB{}, false | |
| 97 | - | } | |
| 98 | - | if width == 2 { | |
| 99 | - | v, ok := parseHexByte(part) | |
| 100 | - | if !ok { | |
| 101 | - | return renderer.RGB{}, false | |
| 102 | - | } | |
| 103 | - | out[i] = v | |
| 104 | - | continue | |
| 105 | - | } | |
| 106 | - | v, err := strconv.ParseUint(part, 16, 16) | |
| 107 | - | if err != nil { | |
| 91 | + | component, ok := parseXColorComponent(part) | |
| 92 | + | if !ok { | |
| 108 | 93 | return renderer.RGB{}, false | |
| 109 | 94 | } | |
| 110 | - | out[i] = uint8(v >> 8) | |
| 95 | + | out[i] = component | |
| 111 | 96 | } | |
| 112 | 97 | return renderer.RGB{R: out[0], G: out[1], B: out[2]}, true | |
| 113 | 98 | } | |
| 114 | 99 | ||
| 100 | + | // parseXColorComponent scales an XParseColor component of one to four hex | |
| 101 | + | // digits to an 8-bit channel. | |
| 102 | + | func parseXColorComponent(s string) (uint8, bool) { | |
| 103 | + | if len(s) < 1 || len(s) > 4 { | |
| 104 | + | return 0, false | |
| 105 | + | } | |
| 106 | + | value, err := strconv.ParseUint(s, 16, 16) | |
| 107 | + | if err != nil { | |
| 108 | + | return 0, false | |
| 109 | + | } | |
| 110 | + | max := uint64((1 << (4 * len(s))) - 1) | |
| 111 | + | return uint8(value * 255 / max), true | |
| 112 | + | } | |
| 113 | + | ||
| 115 | 114 | func parseHexByte(s string) (uint8, bool) { | |
| 116 | 115 | v, err := strconv.ParseUint(s, 16, 8) | |
| 117 | 116 | if err != nil { | |
| @@ -500,7 +499,8 @@ func paletteForegroundStyle(base, neutral renderer.Style, t Theme) renderer.Styl | |||
| 500 | 499 | } | |
| 501 | 500 | candidate := Blend(base.ForegroundRGB, blue, titleHueBlend) | |
| 502 | 501 | if ContrastRatio(candidate, base.BackgroundRGB) >= accentContrastMin { | |
| 503 | - | out := base | |
| 502 | + | out := neutral | |
| 503 | + | out.HasForegroundRGB = true | |
| 504 | 504 | out.ForegroundRGB = candidate | |
| 505 | 505 | return out | |
| 506 | 506 | } | |
| @@ -86,32 +86,31 @@ func ParseXColor(s string) (renderer.RGB, bool) { | |||||
| 86 | if len(parts) != 3 { | 86 | if len(parts) != 3 { | ||
| 87 | return renderer.RGB{}, false | 87 | return renderer.RGB{}, false | ||
| 88 | } | 88 | } | ||
| 89 | - | width := len(parts[0]) | |||
| 90 | - | if width != 2 && width != 4 { | |||
| 91 | - | return renderer.RGB{}, false | |||
| 92 | - | } | |||
| 93 | var out [3]uint8 | 89 | var out [3]uint8 | ||
| 94 | for i, part := range parts { | 90 | for i, part := range parts { | ||
| 95 | - | if len(part) != width { | 91 | + | component, ok := parseXColorComponent(part) |
| 96 | - | return renderer.RGB{}, false | 92 | + | if !ok { |
| 97 | - | } | |||
| 98 | - | if width == 2 { | |||
| 99 | - | v, ok := parseHexByte(part) | |||
| 100 | - | if !ok { | |||
| 101 | - | return renderer.RGB{}, false | |||
| 102 | - | } | |||
| 103 | - | out[i] = v | |||
| 104 | - | continue | |||
| 105 | - | } | |||
| 106 | - | v, err := strconv.ParseUint(part, 16, 16) | |||
| 107 | - | if err != nil { | |||
| 108 | return renderer.RGB{}, false | 93 | return renderer.RGB{}, false | ||
| 109 | } | 94 | } | ||
| 110 | - | out[i] = uint8(v >> 8) | 95 | + | out[i] = component |
| 111 | } | 96 | } | ||
| 112 | return renderer.RGB{R: out[0], G: out[1], B: out[2]}, true | 97 | return renderer.RGB{R: out[0], G: out[1], B: out[2]}, true | ||
| 113 | } | 98 | } | ||
| 114 | 99 | ||||
| 100 | + | // parseXColorComponent scales an XParseColor component of one to four hex | |||
| 101 | + | // digits to an 8-bit channel. | |||
| 102 | + | func parseXColorComponent(s string) (uint8, bool) { | |||
| 103 | + | if len(s) < 1 || len(s) > 4 { | |||
| 104 | + | return 0, false | |||
| 105 | + | } | |||
| 106 | + | value, err := strconv.ParseUint(s, 16, 16) | |||
| 107 | + | if err != nil { | |||
| 108 | + | return 0, false | |||
| 109 | + | } | |||
| 110 | + | max := uint64((1 << (4 * len(s))) - 1) | |||
| 111 | + | return uint8(value * 255 / max), true | |||
| 112 | + | } | |||
| 113 | + | ||||
| 115 | func parseHexByte(s string) (uint8, bool) { | 114 | func parseHexByte(s string) (uint8, bool) { | ||
| 116 | v, err := strconv.ParseUint(s, 16, 8) | 115 | v, err := strconv.ParseUint(s, 16, 8) | ||
| 117 | if err != nil { | 116 | if err != nil { | ||
| @@ -500,7 +499,8 @@ func paletteForegroundStyle(base, neutral renderer.Style, t Theme) renderer.Styl | |||||
| 500 | } | 499 | } | ||
| 501 | candidate := Blend(base.ForegroundRGB, blue, titleHueBlend) | 500 | candidate := Blend(base.ForegroundRGB, blue, titleHueBlend) | ||
| 502 | if ContrastRatio(candidate, base.BackgroundRGB) >= accentContrastMin { | 501 | if ContrastRatio(candidate, base.BackgroundRGB) >= accentContrastMin { | ||
| 503 | - | out := base | 502 | + | out := neutral |
| 503 | + | out.HasForegroundRGB = true | |||
| 504 | out.ForegroundRGB = candidate | 504 | out.ForegroundRGB = candidate | ||
| 505 | return out | 505 | return out | ||
| 506 | } | 506 | } | ||
M internal/usecase/theme/theme_test.go +7 -2
| @@ -16,10 +16,15 @@ func TestParseXColor(t *testing.T) { | |||
| 16 | 16 | want renderer.RGB | |
| 17 | 17 | ok bool | |
| 18 | 18 | }{ | |
| 19 | - | {name: "short rgb", in: "rgb:12/ab/FF", want: renderer.RGB{R: 0x12, G: 0xab, B: 0xff}, ok: true}, | |
| 19 | + | {name: "mixed component widths scale independently", in: "rgb:1/24/86", want: renderer.RGB{R: 0x11, G: 0x24, B: 0x86}, ok: true}, | |
| 20 | + | {name: "one, three, and four digit components", in: "rgb:f/abc/1234", want: renderer.RGB{R: 0xff, G: 0xab, B: 0x12}, ok: true}, | |
| 20 | 21 | {name: "wide rgb uses high byte", in: "rgb:1234/abcd/FFFF", want: renderer.RGB{R: 0x12, G: 0xab, B: 0xff}, ok: true}, | |
| 21 | 22 | {name: "hex", in: "#01aBff", want: renderer.RGB{R: 0x01, G: 0xab, B: 0xff}, ok: true}, | |
| 22 | - | {name: "mixed width rejected", in: "rgb:12/abcd/ff", ok: false}, | |
| 23 | + | {name: "empty component rejected", in: "rgb:/24/86", ok: false}, | |
| 24 | + | {name: "too-wide component rejected", in: "rgb:12345/24/86", ok: false}, | |
| 25 | + | {name: "non-hex component rejected", in: "rgb:1/2g/86", ok: false}, | |
| 26 | + | {name: "missing component rejected", in: "rgb:1/24", ok: false}, | |
| 27 | + | {name: "extra component rejected", in: "rgb:1/24/86/ff", ok: false}, | |
| 23 | 28 | {name: "garbage rejected", in: "not-a-color", ok: false}, | |
| 24 | 29 | {name: "bad hex rejected", in: "#xyzxyz", ok: false}, | |
| 25 | 30 | } | |
| @@ -16,10 +16,15 @@ func TestParseXColor(t *testing.T) { | |||||
| 16 | want renderer.RGB | 16 | want renderer.RGB | ||
| 17 | ok bool | 17 | ok bool | ||
| 18 | }{ | 18 | }{ | ||
| 19 | - | {name: "short rgb", in: "rgb:12/ab/FF", want: renderer.RGB{R: 0x12, G: 0xab, B: 0xff}, ok: true}, | 19 | + | {name: "mixed component widths scale independently", in: "rgb:1/24/86", want: renderer.RGB{R: 0x11, G: 0x24, B: 0x86}, ok: true}, |
| 20 | + | {name: "one, three, and four digit components", in: "rgb:f/abc/1234", want: renderer.RGB{R: 0xff, G: 0xab, B: 0x12}, ok: true}, | |||
| 20 | {name: "wide rgb uses high byte", in: "rgb:1234/abcd/FFFF", want: renderer.RGB{R: 0x12, G: 0xab, B: 0xff}, ok: true}, | 21 | {name: "wide rgb uses high byte", in: "rgb:1234/abcd/FFFF", want: renderer.RGB{R: 0x12, G: 0xab, B: 0xff}, ok: true}, | ||
| 21 | {name: "hex", in: "#01aBff", want: renderer.RGB{R: 0x01, G: 0xab, B: 0xff}, ok: true}, | 22 | {name: "hex", in: "#01aBff", want: renderer.RGB{R: 0x01, G: 0xab, B: 0xff}, ok: true}, | ||
| 22 | - | {name: "mixed width rejected", in: "rgb:12/abcd/ff", ok: false}, | 23 | + | {name: "empty component rejected", in: "rgb:/24/86", ok: false}, |
| 24 | + | {name: "too-wide component rejected", in: "rgb:12345/24/86", ok: false}, | |||
| 25 | + | {name: "non-hex component rejected", in: "rgb:1/2g/86", ok: false}, | |||
| 26 | + | {name: "missing component rejected", in: "rgb:1/24", ok: false}, | |||
| 27 | + | {name: "extra component rejected", in: "rgb:1/24/86/ff", ok: false}, | |||
| 23 | {name: "garbage rejected", in: "not-a-color", ok: false}, | 28 | {name: "garbage rejected", in: "not-a-color", ok: false}, | ||
| 24 | {name: "bad hex rejected", in: "#xyzxyz", ok: false}, | 29 | {name: "bad hex rejected", in: "#xyzxyz", ok: false}, | ||
| 25 | } | 30 | } | ||