From cd92c19e42970c24feecadace64b63052c342c98 Mon Sep 17 00:00:00 2001 From: HANCORE-linux Date: Mon, 8 Jun 2026 10:04:35 +0200 Subject: [PATCH 1/3] fix(theme): validate and merge user theme files instead of blind override User theme files in ~/.config/cliamp/themes/ could silently replace built-in themes even when partial or corrupt, leaving empty hex fields that produced broken colours via lipgloss.Color("") in styles.go. - validHex() validates #rgb/#rrggbb/#rrggbbaa format - Theme.Validate() checks all six colour fields - merge() applies only valid-hex fields onto the destination theme - loadUserDir now uses two modes: - MERGE: when the file name matches a built-in theme, partial fields are merged onto the built-in (rest survives) - VALIDATE: themes without built-in match require all six hex fields or are rejected - Invalid hex values in either mode are silently ignored --- theme/load_test.go | 89 ++++++++++++++++++++++++++++++++---- theme/theme.go | 86 ++++++++++++++++++++++++++++++++++- theme/theme_test.go | 107 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 273 insertions(+), 9 deletions(-) diff --git a/theme/load_test.go b/theme/load_test.go index c6fbe22f7..99a704b76 100644 --- a/theme/load_test.go +++ b/theme/load_test.go @@ -45,19 +45,20 @@ func TestLoadAllSortedCaseInsensitive(t *testing.T) { } } -func TestLoadAllUserThemeOverridesBuiltin(t *testing.T) { +func TestLoadAllPartialUserOverrideMergesOntoBuiltin(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) - // Put a user override file named "dracula.toml" with a distinctive accent color. userDir := filepath.Join(home, ".config", "cliamp", "themes") if err := os.MkdirAll(userDir, 0o755); err != nil { t.Fatalf("MkdirAll: %v", err) } - overridden := `accent = "#ff00ff" + + // Only two fields — should MERGE onto the built-in dracula. + partial := `accent = "#ff00ff" fg = "#123456" ` - if err := os.WriteFile(filepath.Join(userDir, "dracula.toml"), []byte(overridden), 0o644); err != nil { + if err := os.WriteFile(filepath.Join(userDir, "dracula.toml"), []byte(partial), 0o644); err != nil { t.Fatalf("WriteFile: %v", err) } @@ -70,13 +71,18 @@ fg = "#123456" } } if got.Name == "" { - t.Fatal("dracula theme not present after override") + t.Fatal("dracula theme not present after merge") } + // User fields take effect. if got.Accent != "#ff00ff" { - t.Errorf("Accent = %q, want #ff00ff (user override)", got.Accent) + t.Errorf("Accent = %q, want #ff00ff", got.Accent) } if got.FG != "#123456" { - t.Errorf("FG = %q, want #123456 (user override)", got.FG) + t.Errorf("FG = %q, want #123456", got.FG) + } + // Built-in dracula has bright_fg "#f8f8f2" — must survive merge. + if got.BrightFG != "#f8f8f2" { + t.Errorf("built-in BrightFG should survive merge: got %q, want #f8f8f2", got.BrightFG) } } @@ -88,7 +94,13 @@ func TestLoadAllAddsUserOnlyTheme(t *testing.T) { if err := os.MkdirAll(userDir, 0o755); err != nil { t.Fatalf("MkdirAll: %v", err) } - custom := `accent = "#abcdef"` + // New theme (no built-in match): all six hex fields required. + custom := `accent = "#abcdef" +bright_fg = "#ffffff" +fg = "#cccccc" +green = "#00ff00" +yellow = "#ffff00" +red = "#ff0000"` if err := os.WriteFile(filepath.Join(userDir, "mytheme.toml"), []byte(custom), 0o644); err != nil { t.Fatalf("WriteFile: %v", err) } @@ -108,6 +120,67 @@ func TestLoadAllAddsUserOnlyTheme(t *testing.T) { } } +func TestLoadAllSkipsPartialThemeWithoutBuiltinMatch(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + userDir := filepath.Join(home, ".config", "cliamp", "themes") + if err := os.MkdirAll(userDir, 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // Only accent set, no built-in "broken" exists — should be rejected. + partial := `accent = "#ff0000"` + if err := os.WriteFile(filepath.Join(userDir, "broken.toml"), []byte(partial), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + themes := LoadAll() + for _, th := range themes { + if th.Name == "broken" { + t.Fatal("partial theme without built-in match should not be loaded") + } + } +} + +func TestLoadAllSkipsInvalidHexInMerge(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + userDir := filepath.Join(home, ".config", "cliamp", "themes") + if err := os.MkdirAll(userDir, 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + // All six fields present but red is invalid hex — valid fields merge, + // invalid field is ignored (built-in value survives). + bad := `accent = "#ff0000" +bright_fg = "#f8f8f2" +fg = "#6272a4" +green = "#50fa7b" +yellow = "#f1fa8c" +red = "not-a-color"` + if err := os.WriteFile(filepath.Join(userDir, "dracula.toml"), []byte(bad), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + themes := LoadAll() + var got Theme + for _, th := range themes { + if strings.EqualFold(th.Name, "dracula") { + got = th + break + } + } + if got.Name == "" { + t.Fatal("dracula theme not found after merge") + } + if got.Accent != "#ff0000" { + t.Errorf("valid Accent should merge: got %q, want #ff0000", got.Accent) + } + if got.Red != "#ff5555" { + t.Errorf("invalid Red should be ignored (built-in): got %q, want #ff5555", got.Red) + } +} + func TestLoadAllIgnoresNonTomlFiles(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) diff --git a/theme/theme.go b/theme/theme.go index 20fd9c19b..3c5dd3c3c 100644 --- a/theme/theme.go +++ b/theme/theme.go @@ -5,6 +5,7 @@ import ( "bufio" "cmp" "embed" + "fmt" "io" "os" "path/filepath" @@ -36,6 +37,71 @@ func (t Theme) IsDefault() bool { return t.Accent == "" && t.Green == "" && t.BrightFG == "" } +// validHex reports whether s is a valid hex color like "#fff" or "#aabbcc". +func validHex(s string) bool { + if len(s) < 4 || s[0] != '#' { + return false + } + for _, r := range s[1:] { + if !(r >= '0' && r <= '9') && !(r >= 'a' && r <= 'f') && !(r >= 'A' && r <= 'F') { + return false + } + } + n := len(s) - 1 + return n == 3 || n == 6 || n == 8 +} + +// Validate checks that all six color fields are non-empty hex values. +func (t Theme) Validate() error { + missing := make([]string, 0, 6) + if !validHex(t.Accent) { + missing = append(missing, "accent") + } + if !validHex(t.BrightFG) { + missing = append(missing, "bright_fg") + } + if !validHex(t.FG) { + missing = append(missing, "fg") + } + if !validHex(t.Green) { + missing = append(missing, "green") + } + if !validHex(t.Yellow) { + missing = append(missing, "yellow") + } + if !validHex(t.Red) { + missing = append(missing, "red") + } + if len(missing) > 0 { + return fmt.Errorf("missing or invalid hex fields: %s", strings.Join(missing, ", ")) + } + return nil +} + +// merge applies every non-empty valid-hex field from src onto dst. +// Fields with invalid hex values are silently ignored so that a corrupt +// or partial user file only changes the colours it explicitly sets. +func merge(dst *Theme, src Theme) { + if validHex(src.Accent) { + dst.Accent = src.Accent + } + if validHex(src.BrightFG) { + dst.BrightFG = src.BrightFG + } + if validHex(src.FG) { + dst.FG = src.FG + } + if validHex(src.Green) { + dst.Green = src.Green + } + if validHex(src.Yellow) { + dst.Yellow = src.Yellow + } + if validHex(src.Red) { + dst.Red = src.Red + } +} + // Default returns a sentinel "Default" theme with empty hex values, // signaling that ANSI fallback colors should be used. func Default() Theme { @@ -129,6 +195,12 @@ func loadBuiltin(themes map[string]Theme) { } // loadUserDir loads themes from ~/.config/cliamp/themes/*.toml. +// +// If a user file matches a built-in theme name its fields are merged onto +// the built-in, so that a partial override (e.g. only "accent = ...") +// changes only that colour. Themes without a built-in match require all +// six hex fields to pass validation. Invalid hex values in either mode are +// silently ignored — the corresponding built-in (or zero) value survives. func loadUserDir(dir string, themes map[string]Theme) { entries, err := os.ReadDir(dir) if err != nil { @@ -149,6 +221,18 @@ func loadUserDir(dir string, themes map[string]Theme) { if err != nil { continue } - themes[strings.ToLower(name)] = t + key := strings.ToLower(name) + if _, exists := themes[key]; exists { + // Merge onto the existing (built-in) theme. + existing := themes[key] + merge(&existing, t) + themes[key] = existing + } else { + // New theme (no built-in match): all six fields required. + if err := t.Validate(); err != nil { + continue + } + themes[key] = t + } } } diff --git a/theme/theme_test.go b/theme/theme_test.go index 18927b725..bf6a62288 100644 --- a/theme/theme_test.go +++ b/theme/theme_test.go @@ -123,3 +123,110 @@ func TestParsedThemeNotDefault(t *testing.T) { t.Error("theme with accent should not be IsDefault()") } } + +func TestValidHex(t *testing.T) { + tests := []struct { + s string + want bool + }{ + {"#fff", true}, + {"#FFf", true}, + {"#aabbcc", true}, + {"#AABBCC", true}, + {"#aabbccdd", true}, + {"", false}, + {"aabbcc", false}, + {"#xyz", false}, + {"#12", false}, + {"#1234567", false}, + {"#gggggg", false}, + } + for _, tt := range tests { + got := validHex(tt.s) + if got != tt.want { + t.Errorf("validHex(%q) = %v, want %v", tt.s, got, tt.want) + } + } +} + +func TestValidate(t *testing.T) { + valid := Theme{"test", "#bd93f9", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "#ff5555"} + if err := valid.Validate(); err != nil { + t.Errorf("valid theme should pass: %v", err) + } + + partial := Theme{"partial", "#ff0000", "", "", "", "", ""} + if err := partial.Validate(); err == nil { + t.Error("partial theme should fail validation") + } + + empty := Theme{"empty", "", "", "", "", "", ""} + if err := empty.Validate(); err == nil { + t.Error("empty theme should fail validation") + } + + badHex := Theme{"bad", "#ff0000", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "not-a-color"} + if err := badHex.Validate(); err == nil { + t.Error("theme with invalid hex should fail validation") + } +} + +func TestValidateErrorContainsFieldNames(t *testing.T) { + th := Theme{"broken", "#ff0000", "", "", "", "", ""} + err := th.Validate() + if err == nil { + t.Fatal("expected error") + } + msg := err.Error() + for _, field := range []string{"bright_fg", "fg", "green", "yellow", "red"} { + if !strings.Contains(msg, field) { + t.Errorf("error %q should mention %q", msg, field) + } + } +} + +func TestMerge(t *testing.T) { + builtin := Theme{"dracula", "#bd93f9", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "#ff5555"} + + // Partial override: only accent and a new fg. + partial := Theme{"dracula", "#ff00ff", "", "#123456", "", "", ""} + + dst := builtin + merge(&dst, partial) + + if dst.Accent != "#ff00ff" { + t.Errorf("Accent = %q, want #ff00ff", dst.Accent) + } + if dst.FG != "#123456" { + t.Errorf("FG = %q, want #123456", dst.FG) + } + if dst.BrightFG != "#f8f8f2" { + t.Errorf("BrightFG should survive merge: got %q", dst.BrightFG) + } + if dst.Green != "#50fa7b" { + t.Errorf("Green should survive merge: got %q", dst.Green) + } + if dst.Yellow != "#f1fa8c" { + t.Errorf("Yellow should survive merge: got %q", dst.Yellow) + } + if dst.Red != "#ff5555" { + t.Errorf("Red should survive merge: got %q", dst.Red) + } +} + +func TestMergeIgnoresInvalidHex(t *testing.T) { + builtin := Theme{"dracula", "#bd93f9", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "#ff5555"} + + // User file has valid accent + invalid red. + user := Theme{"dracula", "#aa0000", "", "", "", "", "not-a-color"} + + dst := builtin + merge(&dst, user) + + if dst.Accent != "#aa0000" { + t.Errorf("valid Accent should merge: got %q", dst.Accent) + } + if dst.Red != "#ff5555" { + t.Errorf("invalid Red should be ignored (built-in): got %q, want #ff5555", dst.Red) + } +} From 31bf2dd974ddb5711ad09cad5f8bf37c050cb621 Mon Sep 17 00:00:00 2001 From: HANCORE-linux Date: Mon, 8 Jun 2026 10:37:15 +0200 Subject: [PATCH 2/3] refactor(theme): convert tests to table-driven, fix XDG_CONFIG_HOME isolation - TestValidate and TestMerge now use table-driven subtests - TestLoadAllUserThemeScenarios combines 4 LoadAll scenarios with shared setup - All env-based tests use CLIAMP_CONFIG_DIR instead of HOME to avoid XDG_CONFIG_HOME override in appdir.Dir() --- theme/load_test.go | 247 +++++++++++++++++++------------------------- theme/theme_test.go | 162 +++++++++++++++++------------ 2 files changed, 201 insertions(+), 208 deletions(-) diff --git a/theme/load_test.go b/theme/load_test.go index 99a704b76..0264674e7 100644 --- a/theme/load_test.go +++ b/theme/load_test.go @@ -8,8 +8,7 @@ import ( ) func TestLoadAllIncludesBuiltinThemes(t *testing.T) { - // Point HOME somewhere empty so only embedded themes load. - t.Setenv("HOME", t.TempDir()) + t.Setenv("CLIAMP_CONFIG_DIR", filepath.Join(t.TempDir(), "empty")) themes := LoadAll() if len(themes) == 0 { @@ -33,7 +32,7 @@ func TestLoadAllIncludesBuiltinThemes(t *testing.T) { } func TestLoadAllSortedCaseInsensitive(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + t.Setenv("CLIAMP_CONFIG_DIR", filepath.Join(t.TempDir(), "empty")) themes := LoadAll() for i := 1; i < len(themes); i++ { @@ -45,147 +44,116 @@ func TestLoadAllSortedCaseInsensitive(t *testing.T) { } } -func TestLoadAllPartialUserOverrideMergesOntoBuiltin(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - - userDir := filepath.Join(home, ".config", "cliamp", "themes") - if err := os.MkdirAll(userDir, 0o755); err != nil { - t.Fatalf("MkdirAll: %v", err) - } - - // Only two fields — should MERGE onto the built-in dracula. - partial := `accent = "#ff00ff" -fg = "#123456" -` - if err := os.WriteFile(filepath.Join(userDir, "dracula.toml"), []byte(partial), 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - - themes := LoadAll() - var got Theme - for _, th := range themes { - if strings.EqualFold(th.Name, "dracula") { - got = th - break - } - } - if got.Name == "" { - t.Fatal("dracula theme not present after merge") - } - // User fields take effect. - if got.Accent != "#ff00ff" { - t.Errorf("Accent = %q, want #ff00ff", got.Accent) - } - if got.FG != "#123456" { - t.Errorf("FG = %q, want #123456", got.FG) - } - // Built-in dracula has bright_fg "#f8f8f2" — must survive merge. - if got.BrightFG != "#f8f8f2" { - t.Errorf("built-in BrightFG should survive merge: got %q, want #f8f8f2", got.BrightFG) - } -} - -func TestLoadAllAddsUserOnlyTheme(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - - userDir := filepath.Join(home, ".config", "cliamp", "themes") - if err := os.MkdirAll(userDir, 0o755); err != nil { - t.Fatalf("MkdirAll: %v", err) - } - // New theme (no built-in match): all six hex fields required. - custom := `accent = "#abcdef" -bright_fg = "#ffffff" -fg = "#cccccc" -green = "#00ff00" -yellow = "#ffff00" -red = "#ff0000"` - if err := os.WriteFile(filepath.Join(userDir, "mytheme.toml"), []byte(custom), 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - - themes := LoadAll() - var found bool - for _, th := range themes { - if th.Name == "mytheme" { - found = true - if th.Accent != "#abcdef" { - t.Errorf("Accent = %q, want #abcdef", th.Accent) +func TestLoadAllUserThemeScenarios(t *testing.T) { + tests := []struct { + name string + fileName string + fileBody string + check func(t *testing.T, themes []Theme) + }{ + { + name: "partial merge onto builtin", + fileName: "dracula.toml", + fileBody: "accent = \"#ff00ff\"\nfg = \"#123456\"\n", + check: func(t *testing.T, themes []Theme) { + var got Theme + for _, th := range themes { + if strings.EqualFold(th.Name, "dracula") { + got = th + break + } + } + if got.Name == "" { + t.Fatal("dracula theme not present after merge") + } + if got.Accent != "#ff00ff" { + t.Errorf("Accent = %q, want #ff00ff", got.Accent) + } + if got.FG != "#123456" { + t.Errorf("FG = %q, want #123456", got.FG) + } + if got.BrightFG != "#f8f8f2" { + t.Errorf("built-in BrightFG should survive: got %q, want #f8f8f2", got.BrightFG) + } + }, + }, + { + name: "full standalone theme accepted", + fileName: "mytheme.toml", + fileBody: "accent = \"#abcdef\"\nbright_fg = \"#ffffff\"\nfg = \"#cccccc\"\ngreen = \"#00ff00\"\nyellow = \"#ffff00\"\nred = \"#ff0000\"", + check: func(t *testing.T, themes []Theme) { + var found bool + for _, th := range themes { + if th.Name == "mytheme" { + found = true + if th.Accent != "#abcdef" { + t.Errorf("Accent = %q, want #abcdef", th.Accent) + } + } + } + if !found { + t.Error("user theme mytheme not loaded") + } + }, + }, + { + name: "partial without builtin match skipped", + fileName: "broken.toml", + fileBody: "accent = \"#ff0000\"", + check: func(t *testing.T, themes []Theme) { + for _, th := range themes { + if th.Name == "broken" { + t.Fatal("partial theme without built-in match should not be loaded") + } + } + }, + }, + { + name: "invalid hex in merge ignored", + fileName: "dracula.toml", + fileBody: "accent = \"#ff0000\"\nbright_fg = \"#f8f8f2\"\nfg = \"#6272a4\"\ngreen = \"#50fa7b\"\nyellow = \"#f1fa8c\"\nred = \"not-a-color\"", + check: func(t *testing.T, themes []Theme) { + var got Theme + for _, th := range themes { + if strings.EqualFold(th.Name, "dracula") { + got = th + break + } + } + if got.Name == "" { + t.Fatal("dracula theme not found after merge") + } + if got.Accent != "#ff0000" { + t.Errorf("valid Accent should merge: got %q, want #ff0000", got.Accent) + } + if got.Red != "#ff5555" { + t.Errorf("invalid Red should be ignored (built-in): got %q, want #ff5555", got.Red) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + configDir := filepath.Join(t.TempDir(), ".config", "cliamp") + t.Setenv("CLIAMP_CONFIG_DIR", configDir) + userDir := filepath.Join(configDir, "themes") + if err := os.MkdirAll(userDir, 0o755); err != nil { + t.Fatalf("MkdirAll: %v", err) + } + if err := os.WriteFile(filepath.Join(userDir, tt.fileName), []byte(tt.fileBody), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) } - } - } - if !found { - t.Error("user theme mytheme not loaded") - } -} - -func TestLoadAllSkipsPartialThemeWithoutBuiltinMatch(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - - userDir := filepath.Join(home, ".config", "cliamp", "themes") - if err := os.MkdirAll(userDir, 0o755); err != nil { - t.Fatalf("MkdirAll: %v", err) - } - // Only accent set, no built-in "broken" exists — should be rejected. - partial := `accent = "#ff0000"` - if err := os.WriteFile(filepath.Join(userDir, "broken.toml"), []byte(partial), 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - - themes := LoadAll() - for _, th := range themes { - if th.Name == "broken" { - t.Fatal("partial theme without built-in match should not be loaded") - } - } -} - -func TestLoadAllSkipsInvalidHexInMerge(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - userDir := filepath.Join(home, ".config", "cliamp", "themes") - if err := os.MkdirAll(userDir, 0o755); err != nil { - t.Fatalf("MkdirAll: %v", err) - } - // All six fields present but red is invalid hex — valid fields merge, - // invalid field is ignored (built-in value survives). - bad := `accent = "#ff0000" -bright_fg = "#f8f8f2" -fg = "#6272a4" -green = "#50fa7b" -yellow = "#f1fa8c" -red = "not-a-color"` - if err := os.WriteFile(filepath.Join(userDir, "dracula.toml"), []byte(bad), 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - - themes := LoadAll() - var got Theme - for _, th := range themes { - if strings.EqualFold(th.Name, "dracula") { - got = th - break - } - } - if got.Name == "" { - t.Fatal("dracula theme not found after merge") - } - if got.Accent != "#ff0000" { - t.Errorf("valid Accent should merge: got %q, want #ff0000", got.Accent) - } - if got.Red != "#ff5555" { - t.Errorf("invalid Red should be ignored (built-in): got %q, want #ff5555", got.Red) + tt.check(t, LoadAll()) + }) } } func TestLoadAllIgnoresNonTomlFiles(t *testing.T) { - home := t.TempDir() - t.Setenv("HOME", home) - - userDir := filepath.Join(home, ".config", "cliamp", "themes") + configDir := filepath.Join(t.TempDir(), ".config", "cliamp") + t.Setenv("CLIAMP_CONFIG_DIR", configDir) + userDir := filepath.Join(configDir, "themes") if err := os.MkdirAll(userDir, 0o755); err != nil { t.Fatalf("MkdirAll: %v", err) } @@ -206,8 +174,7 @@ func TestLoadAllIgnoresNonTomlFiles(t *testing.T) { } func TestLoadAllMissingUserDir(t *testing.T) { - // HOME points at a dir where ~/.config/cliamp/themes doesn't exist. - t.Setenv("HOME", t.TempDir()) + t.Setenv("CLIAMP_CONFIG_DIR", filepath.Join(t.TempDir(), "empty")) themes := LoadAll() if len(themes) == 0 { t.Error("LoadAll() with missing user dir should still return built-in themes") diff --git a/theme/theme_test.go b/theme/theme_test.go index bf6a62288..64a1ed287 100644 --- a/theme/theme_test.go +++ b/theme/theme_test.go @@ -150,83 +150,109 @@ func TestValidHex(t *testing.T) { } func TestValidate(t *testing.T) { - valid := Theme{"test", "#bd93f9", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "#ff5555"} - if err := valid.Validate(); err != nil { - t.Errorf("valid theme should pass: %v", err) - } - - partial := Theme{"partial", "#ff0000", "", "", "", "", ""} - if err := partial.Validate(); err == nil { - t.Error("partial theme should fail validation") - } - - empty := Theme{"empty", "", "", "", "", "", ""} - if err := empty.Validate(); err == nil { - t.Error("empty theme should fail validation") - } - - badHex := Theme{"bad", "#ff0000", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "not-a-color"} - if err := badHex.Validate(); err == nil { - t.Error("theme with invalid hex should fail validation") + tests := []struct { + name string + theme Theme + wantErr bool + errMsg string + }{ + { + name: "all valid fields", + theme: Theme{"test", "#bd93f9", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "#ff5555"}, + wantErr: false, + }, + { + name: "partial fields", + theme: Theme{"partial", "#ff0000", "", "", "", "", ""}, + wantErr: true, + }, + { + name: "all empty", + theme: Theme{"empty", "", "", "", "", "", ""}, + wantErr: true, + }, + { + name: "invalid hex one field", + theme: Theme{"bad", "#ff0000", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "not-a-color"}, + wantErr: true, + }, + { + name: "error contains field names", + theme: Theme{"broken", "#ff0000", "", "", "", "", ""}, + wantErr: true, + errMsg: "bright_fg", + }, } -} -func TestValidateErrorContainsFieldNames(t *testing.T) { - th := Theme{"broken", "#ff0000", "", "", "", "", ""} - err := th.Validate() - if err == nil { - t.Fatal("expected error") - } - msg := err.Error() - for _, field := range []string{"bright_fg", "fg", "green", "yellow", "red"} { - if !strings.Contains(msg, field) { - t.Errorf("error %q should mention %q", msg, field) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.theme.Validate() + if tt.wantErr && err == nil { + t.Fatal("expected error, got nil") + } + if !tt.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.errMsg != "" && err != nil { + if !strings.Contains(err.Error(), tt.errMsg) { + t.Errorf("error %q should contain %q", err.Error(), tt.errMsg) + } + } + }) } } func TestMerge(t *testing.T) { builtin := Theme{"dracula", "#bd93f9", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "#ff5555"} - // Partial override: only accent and a new fg. - partial := Theme{"dracula", "#ff00ff", "", "#123456", "", "", ""} - - dst := builtin - merge(&dst, partial) - - if dst.Accent != "#ff00ff" { - t.Errorf("Accent = %q, want #ff00ff", dst.Accent) - } - if dst.FG != "#123456" { - t.Errorf("FG = %q, want #123456", dst.FG) - } - if dst.BrightFG != "#f8f8f2" { - t.Errorf("BrightFG should survive merge: got %q", dst.BrightFG) - } - if dst.Green != "#50fa7b" { - t.Errorf("Green should survive merge: got %q", dst.Green) - } - if dst.Yellow != "#f1fa8c" { - t.Errorf("Yellow should survive merge: got %q", dst.Yellow) - } - if dst.Red != "#ff5555" { - t.Errorf("Red should survive merge: got %q", dst.Red) + tests := []struct { + name string + src Theme + check func(t *testing.T, dst Theme) + }{ + { + name: "partial override", + src: Theme{"dracula", "#ff00ff", "", "#123456", "", "", ""}, + check: func(t *testing.T, dst Theme) { + if dst.Accent != "#ff00ff" { + t.Errorf("Accent = %q, want #ff00ff", dst.Accent) + } + if dst.FG != "#123456" { + t.Errorf("FG = %q, want #123456", dst.FG) + } + if dst.BrightFG != "#f8f8f2" { + t.Errorf("BrightFG should survive: got %q", dst.BrightFG) + } + if dst.Green != "#50fa7b" { + t.Errorf("Green should survive: got %q", dst.Green) + } + if dst.Yellow != "#f1fa8c" { + t.Errorf("Yellow should survive: got %q", dst.Yellow) + } + if dst.Red != "#ff5555" { + t.Errorf("Red should survive: got %q", dst.Red) + } + }, + }, + { + name: "ignores invalid hex", + src: Theme{"dracula", "#aa0000", "", "", "", "", "not-a-color"}, + check: func(t *testing.T, dst Theme) { + if dst.Accent != "#aa0000" { + t.Errorf("valid Accent should merge: got %q", dst.Accent) + } + if dst.Red != "#ff5555" { + t.Errorf("invalid Red should be ignored: got %q, want #ff5555", dst.Red) + } + }, + }, } -} -func TestMergeIgnoresInvalidHex(t *testing.T) { - builtin := Theme{"dracula", "#bd93f9", "#f8f8f2", "#6272a4", "#50fa7b", "#f1fa8c", "#ff5555"} - - // User file has valid accent + invalid red. - user := Theme{"dracula", "#aa0000", "", "", "", "", "not-a-color"} - - dst := builtin - merge(&dst, user) - - if dst.Accent != "#aa0000" { - t.Errorf("valid Accent should merge: got %q", dst.Accent) - } - if dst.Red != "#ff5555" { - t.Errorf("invalid Red should be ignored (built-in): got %q, want #ff5555", dst.Red) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dst := builtin + merge(&dst, tt.src) + tt.check(t, dst) + }) } } From 05ee02150539cd54d876ad663240c51a32189463 Mon Sep 17 00:00:00 2001 From: HANCORE-linux Date: Mon, 8 Jun 2026 10:47:00 +0200 Subject: [PATCH 3/3] style(theme): use raw string literals for multi-line TOML in tests --- theme/load_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/theme/load_test.go b/theme/load_test.go index 0264674e7..25ce030af 100644 --- a/theme/load_test.go +++ b/theme/load_test.go @@ -80,7 +80,12 @@ func TestLoadAllUserThemeScenarios(t *testing.T) { { name: "full standalone theme accepted", fileName: "mytheme.toml", - fileBody: "accent = \"#abcdef\"\nbright_fg = \"#ffffff\"\nfg = \"#cccccc\"\ngreen = \"#00ff00\"\nyellow = \"#ffff00\"\nred = \"#ff0000\"", + fileBody: `accent = "#abcdef" +bright_fg = "#ffffff" +fg = "#cccccc" +green = "#00ff00" +yellow = "#ffff00" +red = "#ff0000"`, check: func(t *testing.T, themes []Theme) { var found bool for _, th := range themes { @@ -111,7 +116,12 @@ func TestLoadAllUserThemeScenarios(t *testing.T) { { name: "invalid hex in merge ignored", fileName: "dracula.toml", - fileBody: "accent = \"#ff0000\"\nbright_fg = \"#f8f8f2\"\nfg = \"#6272a4\"\ngreen = \"#50fa7b\"\nyellow = \"#f1fa8c\"\nred = \"not-a-color\"", + fileBody: `accent = "#ff0000" +bright_fg = "#f8f8f2" +fg = "#6272a4" +green = "#50fa7b" +yellow = "#f1fa8c" +red = "not-a-color"`, check: func(t *testing.T, themes []Theme) { var got Theme for _, th := range themes {