From 59dbe28b0b814c0817a7f333c3c6ac92d9591b57 Mon Sep 17 00:00:00 2001 From: Nxssie Date: Mon, 28 Sep 2026 16:58:59 +0100 Subject: [PATCH 1/4] refactor(tui): extract color palette into theme.go Pure move of the palette var block (8 AdaptiveColor tokens plus modelColors) out of tui.go, with a comment mapping each token to its role. No value changes; tui.go keeps its lipgloss import. --- internal/tui/theme.go | 41 +++++++++++++++++++++++++++++++++++++++++ internal/tui/tui.go | 21 --------------------- 2 files changed, 41 insertions(+), 21 deletions(-) create mode 100644 internal/tui/theme.go diff --git a/internal/tui/theme.go b/internal/tui/theme.go new file mode 100644 index 0000000..ddcf686 --- /dev/null +++ b/internal/tui/theme.go @@ -0,0 +1,41 @@ +package tui + +import "github.com/charmbracelet/lipgloss" + +// ── palette ─────────────────────────────────────────────────────────────────── + +// The palette is a set of roles, not a paint box. Every token is an AdaptiveColor +// pair: the Light side is the value a light terminal wants, the Dark side the one +// a dark terminal wants, and lipgloss picks between them from the detected +// background. +// +// The roles, in declaration order: +// - cCyan is the primary accent, used for the current step and highlighted numbers. +// - cBlue is the deeper accent that grounds the wordmark and selected frames. +// - cBlueDim is the soft accent fill behind a selected block. +// - cGray is the readable middle of the text hierarchy, for secondary copy. +// - cDimGray is the muted tier for hints and inactive rows. +// - cWhite is the high-contrast tier for the loudest labels and prompts. +// - cText is the default reading colour for long runs of prose. +// - cRed is the error tone, kept apart from the accent so failures never read as +// ordinary emphasis. +var ( + cCyan = lipgloss.AdaptiveColor{Light: "#6d28d9", Dark: "#a78bfa"} + cBlue = lipgloss.AdaptiveColor{Light: "#5b21b6", Dark: "#8b5cf6"} + cBlueDim = lipgloss.AdaptiveColor{Light: "#ddd6fe", Dark: "#2e1065"} + cGray = lipgloss.AdaptiveColor{Light: "#52525b", Dark: "#71717a"} + cDimGray = lipgloss.AdaptiveColor{Light: "#a1a1aa", Dark: "#52525b"} + cWhite = lipgloss.AdaptiveColor{Light: "#18181b", Dark: "#ffffff"} + cText = lipgloss.AdaptiveColor{Light: "#374151", Dark: "#cbd5e1"} + cRed = lipgloss.AdaptiveColor{Light: "#dc2626", Dark: "#ef4444"} + + // modelColors are the categorical colours for per-model usage rows: the set is + // fixed so each model keeps a stable colour of its own. + modelColors = []lipgloss.Color{ + lipgloss.Color("#8b5cf6"), + lipgloss.Color("#a78bfa"), + lipgloss.Color("#10B981"), + lipgloss.Color("#F59E0B"), + lipgloss.Color("#EF4444"), + } +) diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 0f9beeb..350e1e8 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -23,27 +23,6 @@ import ( "github.com/nxssie/nan-cli/internal/session" ) -// ── palette ─────────────────────────────────────────────────────────────────── - -var ( - cCyan = lipgloss.AdaptiveColor{Light: "#6d28d9", Dark: "#a78bfa"} - cBlue = lipgloss.AdaptiveColor{Light: "#5b21b6", Dark: "#8b5cf6"} - cBlueDim = lipgloss.AdaptiveColor{Light: "#ddd6fe", Dark: "#2e1065"} - cGray = lipgloss.AdaptiveColor{Light: "#52525b", Dark: "#71717a"} - cDimGray = lipgloss.AdaptiveColor{Light: "#a1a1aa", Dark: "#52525b"} - cWhite = lipgloss.AdaptiveColor{Light: "#18181b", Dark: "#ffffff"} - cText = lipgloss.AdaptiveColor{Light: "#374151", Dark: "#cbd5e1"} - cRed = lipgloss.AdaptiveColor{Light: "#dc2626", Dark: "#ef4444"} - - modelColors = []lipgloss.Color{ - lipgloss.Color("#8b5cf6"), - lipgloss.Color("#a78bfa"), - lipgloss.Color("#10B981"), - lipgloss.Color("#F59E0B"), - lipgloss.Color("#EF4444"), - } -) - // ── layout ──────────────────────────────────────────────────────────────────── // layout holds all computed dimensions for a given terminal size. From 03b488163433cc3fb6180dca4299eee66a658aeb Mon Sep 17 00:00:00 2001 From: Nxssie Date: Mon, 28 Sep 2026 17:07:42 +0100 Subject: [PATCH 2/4] feat(tui): persisted theme setting with t to cycle auto/dark/light The palette was already AdaptiveColor pairs, but which side renders was decided solely by termenv background detection, which silently falls back to dark when the terminal cannot be asked. Members on light terminals over SSH or multiplexers had no override. - ThemeMode (auto/dark/light) persisted in ~/.config/nan/settings.json, read-modify-write so foreign keys survive and an unparseable file is never flattened; missing/corrupt/unknown loads as auto. - applyTheme pins lipgloss's renderer background flag; auto stays the documented no-op so detection keeps its current behavior. - t cycles the mode from any tab, applies it on the next frame and persists it; Run() applies the saved mode before the first frame. - About gained an Appearance row naming what auto resolved to; help and the wide footer hint mention the key. - modelColors became AdaptiveColor pairs with light-tuned values so the five categorical colors stay distinguishable on white. --- internal/tui/config_test.go | 18 +-- internal/tui/theme.go | 136 +++++++++++++++++++-- internal/tui/theme_test.go | 231 ++++++++++++++++++++++++++++++++++++ internal/tui/tui.go | 58 +++++++-- 4 files changed, 419 insertions(+), 24 deletions(-) create mode 100644 internal/tui/theme_test.go diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index 4f081a5..2b5ebca 100644 --- a/internal/tui/config_test.go +++ b/internal/tui/config_test.go @@ -1059,10 +1059,10 @@ func TestBannerFitsAndCarriesTheNames(t *testing.T) { func TestAboutFallsBackToOneLineWhenNarrow(t *testing.T) { // A 40-column terminal has no room for the art plus the wordmark, and a // wrapped banner is worse than no banner. - if strings.Contains(renderAbout(newLayout(40, 24)), "█") { + if strings.Contains(model{}.renderAbout(newLayout(40, 24)), "█") { t.Error("the banner is drawn at a width where it wraps") } - if !strings.Contains(renderAbout(newLayout(BannerWidthPlain+4, 24)), "█") { + if !strings.Contains(model{}.renderAbout(newLayout(BannerWidthPlain+4, 24)), "█") { t.Error("the banner is missing at a width that fits it") } } @@ -1074,7 +1074,7 @@ func TestHomeIsTheFirstTabAndNeedsNoNetwork(t *testing.T) { if tabDefs[0].id != tabHome { t.Fatalf("the first tab is %v, want Home", tabDefs[0].name) } - m := newModel(nil, &session.Session{}) + m := newModel(nil, &session.Session{}, ThemeAuto) if cmd := m.maybeLoad(); cmd != nil { t.Error("Home asks the API for something, so it cannot be the landing tab") } @@ -1458,7 +1458,7 @@ func setupModel(t *testing.T, sess *session.Session) model { t.Setenv("HOME", home) t.Setenv("USERPROFILE", home) t.Setenv("HERMES_HOME", filepath.Join(home, "hermes")) - return newModel(nil, sess) + return newModel(nil, sess, ThemeAuto) } // The idea this replaced was to have the Setup tab fetch the key with the @@ -1668,7 +1668,7 @@ func TestAboutShowsTheSessionPathThatExists(t *testing.T) { t.Setenv("HOME", home) t.Setenv("USERPROFILE", home) - out := renderAbout(newLayout(100, 30)) + out := model{}.renderAbout(newLayout(100, 30)) if strings.Contains(out, "~/") { t.Error("the About tab still prints a tilde path") } @@ -1875,7 +1875,7 @@ func TestSigningInGivesTheClientTheNewToken(t *testing.T) { t.Setenv("USERPROFILE", home) t.Setenv("HERMES_HOME", filepath.Join(home, "h")) - m := newModel(api.New(""), &session.Session{}) + m := newModel(api.New(""), &session.Session{}, ThemeAuto) if m.client.Token() != "" { t.Fatal("this test starts from a client with no token") } @@ -2146,7 +2146,7 @@ func TestSigningInLeadsStraightToTheKey(t *testing.T) { t.Setenv("USERPROFILE", home) t.Setenv("HERMES_HOME", filepath.Join(home, "h")) - m := newModel(api.New(""), &session.Session{}) + m := newModel(api.New(""), &session.Session{}, ThemeAuto) m.lay = newLayout(90, 30) if err := session.Save(&session.Session{Token: "a-token"}); err != nil { t.Fatal(err) @@ -2267,7 +2267,7 @@ func TestSigningOutTakesTwoPressesAndClearsEverything(t *testing.T) { if err := session.Save(sess); err != nil { t.Fatal(err) } - m := newModel(api.New("t"), sess) + m := newModel(api.New("t"), sess, ThemeAuto) m.lay = newLayout(90, 30) m.cache[tabUsage] = "somebody else's numbers" @@ -2317,7 +2317,7 @@ func TestSigningOutStartsTheSetupAgain(t *testing.T) { if err := session.Save(sess); err != nil { t.Fatal(err) } - m := newModel(api.New("t"), sess) + m := newModel(api.New("t"), sess, ThemeAuto) m.lay = newLayout(90, 30) press := func(m model, r rune) model { diff --git a/internal/tui/theme.go b/internal/tui/theme.go index ddcf686..f4db179 100644 --- a/internal/tui/theme.go +++ b/internal/tui/theme.go @@ -1,6 +1,12 @@ package tui -import "github.com/charmbracelet/lipgloss" +import ( + "encoding/json" + "os" + "path/filepath" + + "github.com/charmbracelet/lipgloss" +) // ── palette ─────────────────────────────────────────────────────────────────── @@ -30,12 +36,126 @@ var ( cRed = lipgloss.AdaptiveColor{Light: "#dc2626", Dark: "#ef4444"} // modelColors are the categorical colours for per-model usage rows: the set is - // fixed so each model keeps a stable colour of its own. - modelColors = []lipgloss.Color{ - lipgloss.Color("#8b5cf6"), - lipgloss.Color("#a78bfa"), - lipgloss.Color("#10B981"), - lipgloss.Color("#F59E0B"), - lipgloss.Color("#EF4444"), + // fixed so each model keeps a stable colour of its own. Each is a pair now, + // because the dark one is unreadable on a light background: the Light side is + // a deeper tone of the same hue, so the five stay distinguishable on white + // and a model keeps its identity whichever side the terminal resolves to. + modelColors = []lipgloss.AdaptiveColor{ + {Light: "#7c3aed", Dark: "#8b5cf6"}, + {Light: "#c026d3", Dark: "#a78bfa"}, + {Light: "#059669", Dark: "#10B981"}, + {Light: "#b45309", Dark: "#F59E0B"}, + {Light: "#dc2626", Dark: "#EF4444"}, } ) + +// ── theme mode ──────────────────────────────────────────────────────────────── + +// ThemeMode is which side of every AdaptiveColor the panel draws. Auto is the +// default and the only one that does not override the terminal: it lets the +// renderer's own background detection decide. +type ThemeMode string + +const ( + ThemeAuto ThemeMode = "auto" + ThemeDark ThemeMode = "dark" + ThemeLight ThemeMode = "light" +) + +// nextTheme is the cycle the `t` key walks: auto, then dark, then light, then +// back to auto. +func nextTheme(m ThemeMode) ThemeMode { + switch m { + case ThemeDark: + return ThemeLight + case ThemeLight: + return ThemeAuto + default: + return ThemeDark + } +} + +// applyTheme pins every AdaptiveColor to one side by setting the renderer's +// background flag, which is the only thing a theme setting can do: each pair is +// resolved per render from that flag, so the next frame is drawn in the new +// colours with no style rebuilt. +// +// Auto is deliberately a no-op. It leaves lipgloss's own termenv detection - +// including the dark fallback it uses when the terminal cannot be asked, which +// is what an SSH session or a multiplexer gets - exactly as it works today. +// That is what "auto" means, and overriding it here would take the choice away +// from the terminal. +func applyTheme(m ThemeMode) { + switch m { + case ThemeDark: + lipgloss.SetHasDarkBackground(true) + case ThemeLight: + lipgloss.SetHasDarkBackground(false) + } +} + +// settingsPath is where the theme lives, mirroring session.Path(): the same +// config directory, and the same relative fallback for a home directory that +// cannot be resolved. +func settingsPath() string { + home, err := os.UserHomeDir() + if err != nil { + return filepath.Join(".config", "nan", "settings.json") + } + return filepath.Join(home, ".config", "nan", "settings.json") +} + +func loadTheme() ThemeMode { return loadThemeFrom(settingsPath()) } + +func saveTheme(m ThemeMode) error { return saveThemeTo(settingsPath(), m) } + +// loadThemeFrom reads the saved theme and answers auto for every way of not +// having one: a missing file, an empty one, JSON that does not parse, a key +// that is not a string, or a value this build does not know. The setting is +// cosmetic - the panel works in the terminal's own colours without it - so a +// damaged file is never a reason to fail the TUI, and only a known string is +// read back. +func loadThemeFrom(path string) ThemeMode { + data, err := os.ReadFile(path) + if err != nil { + return ThemeAuto + } + var stored struct { + Theme string `json:"theme"` + } + if err := json.Unmarshal(data, &stored); err != nil { + return ThemeAuto + } + switch ThemeMode(stored.Theme) { + case ThemeDark: + return ThemeDark + case ThemeLight: + return ThemeLight + } + return ThemeAuto +} + +// saveThemeTo writes the theme key back into the settings file, leaving every +// other key exactly where it was. +// +// It is a read-modify-write for the same reason the tool configs are: this file +// is shared and one day holds a setting this CLI does not know about, and +// writing our single key from scratch would take that setting with it. The +// reader is readJSONConfig, whose whole point is refusing to hand back a file it +// could not parse - so a settings.json somebody is midway through editing is +// left alone rather than flattened, and the error travels out to the caller. +func saveThemeTo(path string, m ThemeMode) error { + if m != ThemeDark && m != ThemeLight { + m = ThemeAuto // only ever one of the three constants, whatever came in + } + cfg, err := readJSONConfig(path) + if err != nil { + return err + } + cfg["theme"] = string(m) + data, err := json.MarshalIndent(cfg, "", " ") + if err != nil { + return err + } + return writeConfigFile(path, data) +} diff --git a/internal/tui/theme_test.go b/internal/tui/theme_test.go new file mode 100644 index 0000000..187fcd6 --- /dev/null +++ b/internal/tui/theme_test.go @@ -0,0 +1,231 @@ +package tui + +import ( + "os" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" + + "github.com/nxssie/nan-cli/internal/session" +) + +// ── loading ────────────────────────────────────────────────────────────────── + +// Every way of not having a usable setting lands on auto, because the theme is +// cosmetic: a file somebody is midway through editing, or a value from a +// newer build, must never be a reason the panel refuses to open. +func TestMissingCorruptOrUnknownThemeLoadsAuto(t *testing.T) { + if got := loadThemeFrom(tempConfig(t, "settings.json")); got != ThemeAuto { + t.Errorf("a missing file = %q, want auto", got) + } + + for _, c := range []struct{ name, body string }{ + {"empty", ""}, + {"whitespace", " \n"}, + {"corrupt", `{"theme":"dark",`}, + {"unknown value", `{"theme":"solarized"}`}, + {"not a string", `{"theme":5}`}, + } { + path := tempConfig(t, c.name+".json") + if err := os.WriteFile(path, []byte(c.body), 0o600); err != nil { + t.Fatal(err) + } + if got := loadThemeFrom(path); got != ThemeAuto { + t.Errorf("%s: = %q, want auto", c.name, got) + } + } +} + +// A valid value is read back, whichever of the two explicit modes it names. +func TestKnownThemeLoads(t *testing.T) { + for _, want := range []ThemeMode{ThemeDark, ThemeLight} { + path := tempConfig(t, string(want)+".json") + if err := os.WriteFile(path, []byte(`{"theme":"`+string(want)+`"}`), 0o600); err != nil { + t.Fatal(err) + } + if got := loadThemeFrom(path); got != want { + t.Errorf("loadThemeFrom = %q, want %q", got, want) + } + } +} + +// ── saving ─────────────────────────────────────────────────────────────────── + +// settings.json is shared, the same way the tool configs are: saving our key +// must not take somebody else's with it. +func TestSavingTheThemeKeepsWhatItDoesNotOwn(t *testing.T) { + path := tempConfig(t, "settings.json") + existing := `{"theme":"dark","editor":"vim","nested":{"keep":true}}` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + if got := loadThemeFrom(path); got != ThemeDark { + t.Fatalf("loadThemeFrom = %q, want dark", got) + } + + if err := saveThemeTo(path, ThemeLight); err != nil { + t.Fatal(err) + } + if got := loadThemeFrom(path); got != ThemeLight { + t.Errorf("after saving light, loadThemeFrom = %q", got) + } + + cfg := readJSON(t, path) + if cfg["editor"] != "vim" { + t.Errorf("editor = %v, a foreign key was lost", cfg["editor"]) + } + nested, ok := cfg["nested"].(map[string]any) + if !ok || nested["keep"] != true { + t.Errorf("nested = %v, a foreign key was lost", cfg["nested"]) + } +} + +// A file that cannot be parsed is the one case where writing would do real +// damage - it would replace the whole file with the one key we understand - so +// the save gives up instead, leaving the member's bytes exactly as they are. +func TestSavingOverAFileThatDoesNotParseChangesNothing(t *testing.T) { + path := tempConfig(t, "settings.json") + broken := `{"theme":"dark","editor":"vim"` + if err := os.WriteFile(path, []byte(broken), 0o600); err != nil { + t.Fatal(err) + } + if err := saveThemeTo(path, ThemeLight); err == nil { + t.Fatal("saving over a file that does not parse was reported as success") + } + if got := readFile(t, path); got != broken { + t.Errorf("the file was rewritten:\n%s", got) + } +} + +// A save with no file to read writes one from nothing, which is the first run. +func TestSavingTheThemeCreatesTheFile(t *testing.T) { + path := tempConfig(t, "settings.json") + if err := saveThemeTo(path, ThemeDark); err != nil { + t.Fatal(err) + } + if got := loadThemeFrom(path); got != ThemeDark { + t.Errorf("a freshly written file reads back as %q, want dark", got) + } +} + +// ── cycling and applying ───────────────────────────────────────────────────── + +func TestNextThemeCyclesAutoDarkLight(t *testing.T) { + got := []ThemeMode{ThemeAuto} + m := ThemeAuto + for i := 0; i < 3; i++ { + m = nextTheme(m) + got = append(got, m) + } + want := []ThemeMode{ThemeAuto, ThemeDark, ThemeLight, ThemeAuto} + for i := range want { + if got[i] != want[i] { + t.Errorf("step %d = %q, want %q", i, got[i], want[i]) + } + } +} + +// applyTheme only ever moves the renderer's background flag, and auto moves +// nothing. lipgloss resolves every AdaptiveColor from that flag on each render, +// which is what lets a mode change re-colour the whole panel without a style +// being rebuilt - and why auto can leave detection alone and still be correct. +func TestApplyThemeForcesTheRendererAndAutoLeavesIt(t *testing.T) { + applyTheme(ThemeDark) + if !lipgloss.HasDarkBackground() { + t.Error("dark did not pin the renderer to a dark background") + } + + applyTheme(ThemeLight) + if lipgloss.HasDarkBackground() { + t.Error("light did not pin the renderer to a light background") + } + + // Auto is a no-op, so whichever explicit flag is in place survives it. + lipgloss.SetHasDarkBackground(true) + applyTheme(ThemeAuto) + if !lipgloss.HasDarkBackground() { + t.Error("auto cleared an explicit dark flag, so it is not the no-op it claims") + } + lipgloss.SetHasDarkBackground(false) + applyTheme(ThemeAuto) + if lipgloss.HasDarkBackground() { + t.Error("auto set an explicit dark flag, so it is not the no-op it claims") + } +} + +// ── the key ────────────────────────────────────────────────────────────────── + +// `t` advances the mode and writes it, so the next run opens in the mode this +// one was left in. +func TestThemeKeyCyclesAndPersists(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + m := newModel(nil, &session.Session{}, ThemeAuto) + press := func(m model) model { + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'t'}}) + return updated.(model) + } + + if m.theme != ThemeAuto { + t.Fatalf("a fresh model starts at %q, want auto", m.theme) + } + m = press(m) + if m.theme != ThemeDark { + t.Fatalf("first press = %q, want dark", m.theme) + } + if got := loadTheme(); got != ThemeDark { + t.Errorf("the press did not persist: the file holds %q", got) + } + if m = press(m); m.theme != ThemeLight { + t.Errorf("second press = %q, want light", m.theme) + } + if m = press(m); m.theme != ThemeAuto { + t.Errorf("third press = %q, want auto", m.theme) + } +} + +// The theme is the panel's key: not one the help overlay swallows, and not one +// pressed while the guided setup owns the keyboard. +func TestThemeKeyIsGuardedLikeTheOtherGlobalKeys(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + m := newModel(nil, &session.Session{}, ThemeAuto) + m.showHelp = true + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'t'}}) + if got := updated.(model).theme; got != ThemeAuto { + t.Errorf("t changed the theme to %q with the help overlay open", got) + } + + m = newModel(nil, &session.Session{}, ThemeAuto) + m.wizard = wizardEmail + updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'t'}}) + if got := updated.(model).theme; got != ThemeAuto { + t.Errorf("t changed the theme to %q during the guided setup", got) + } +} + +// ── the About row ──────────────────────────────────────────────────────────── + +// Auto's outcome is not written anywhere else, so the row says which side the +// terminal resolved to; the explicit modes just name themselves. +func TestAboutShowsTheThemeAndWhatAutoResolvedTo(t *testing.T) { + l := newLayout(100, 30) + + if out := (model{theme: ThemeLight}).renderAbout(l); !strings.Contains(out, "Theme:") || !strings.Contains(out, "light") { + t.Errorf("About does not show a light theme:\n%s", out) + } + + lipgloss.SetHasDarkBackground(true) + if out := (model{theme: ThemeAuto}).renderAbout(l); !strings.Contains(out, "auto (terminal looks dark)") { + t.Errorf("About does not say what auto resolved to:\n%s", out) + } + if out := (model{theme: ThemeDark}).renderAbout(l); !strings.Contains(out, "dark") { + t.Errorf("About does not show a dark theme:\n%s", out) + } +} diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 350e1e8..8c5c8c3 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -147,6 +147,7 @@ type model struct { lay layout scrollY int showHelp bool + theme ThemeMode keyInput textinput.Model editingKey bool setupMsg string @@ -235,7 +236,8 @@ const ( loginAskLink ) -func newModel(client *api.Client, sess *session.Session) model { +// newModel builds the panel starting in the mode the member last saved. +func newModel(client *api.Client, sess *session.Session, theme ThemeMode) model { sp := spinner.New() sp.Spinner = spinner.Dot sp.Style = lipgloss.NewStyle().Foreground(cBlue) @@ -267,6 +269,7 @@ func newModel(client *api.Client, sess *session.Session) model { spin: sp, lay: newLayout(80, 24), keyInput: ti, + theme: theme, } } @@ -600,6 +603,20 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.scrollY = 0 return m, m.maybeLoad() } + // The theme, cycled from the panel the way it is set anywhere else: + // auto follows the terminal, dark and light pin it. Guarded like the + // other global keys, so it is the panel's key and not one the help + // overlay or the guided setup answers. + case "t": + if m.showHelp || m.wizard != wizardOff { + break + } + m.theme = nextTheme(m.theme) + applyTheme(m.theme) + // A failed save is ignored on purpose: the mode is already applied + // in memory, this file holds nothing the panel reads back to work, + // and the writes that do matter report their own errors. + _ = saveTheme(m.theme) // Signing out, in two presses. `nan auth logout` already existed as a // command, which is no use to someone who is looking at the panel and // wants to switch accounts. @@ -1010,7 +1027,7 @@ func (m model) View() string { case tabHome: content = renderHome(l, m.sess.Token != "", m.sess.APIKey != "", m.mood()) case tabAbout: - content = renderAbout(l) + content = m.renderAbout(l) case tabSetup: content = m.renderSetup(l) default: @@ -1049,7 +1066,7 @@ func (m model) View() string { b.WriteString(strings.Join(visible, "\n")) b.WriteString("\n") - hint := "←/→ tabs ↑/↓ scroll r refresh ? help q quit" + hint := "←/→ tabs ↑/↓ scroll r refresh t theme ? help q quit" if l.w < 55 { hint = "←/→ ↑/↓ r ? q" } else if l.w < 72 { @@ -1194,7 +1211,7 @@ func parsePeriod(label string, raw any) period { return p } -func renderBar(pct float64, width int, color lipgloss.Color) string { +func renderBar(pct float64, width int, color lipgloss.TerminalColor) string { filled := int(pct / 100.0 * float64(width)) filled = clamp(filled, 0, width) fill := lipgloss.NewStyle().Background(color).Foreground(color).Render(strings.Repeat("█", filled)) @@ -3530,7 +3547,9 @@ func (m model) renderSetup(l layout) string { const Version = "0.1.23" -func renderAbout(l layout) string { +// renderAbout draws the account tab. It is a method because the Appearance row +// shows the theme, which lives on the model. +func (m model) renderAbout(l layout) string { var b strings.Builder logoStyle := lipgloss.NewStyle().Bold(true).Foreground(cCyan) @@ -3567,11 +3586,31 @@ func renderAbout(l layout) string { b.WriteString(l.indent + sectionStyle.Render("Session") + "\n\n") b.WriteString(l.indent + labelStyle.Render("Config:") + - dimStyle.Render(session.Path()) + "\n") + dimStyle.Render(session.Path()) + "\n\n") + + b.WriteString(l.indent + sectionStyle.Render("Appearance") + "\n\n") + b.WriteString(l.indent + labelStyle.Render("Theme:") + + dimStyle.Render(m.themeLabel()) + "\n") return b.String() } +// themeLabel is what the About tab prints for the current mode. In auto the +// useful half is which side the terminal actually resolved to, because auto is +// the one mode whose result is nowhere else on screen. +func (m model) themeLabel() string { + switch m.theme { + case ThemeDark: + return "dark" + case ThemeLight: + return "light" + } + if lipgloss.HasDarkBackground() { + return "auto (terminal looks dark)" + } + return "auto (terminal looks light)" +} + // ── help overlay ───────────────────────────────────────────────────────────── func renderHelp() string { @@ -3579,6 +3618,7 @@ func renderHelp() string { {"←/→ h/l Tab", "switch tabs"}, {"↑/↓ k/j", "scroll"}, {"r", "refresh current tab"}, + {"t", "cycle theme auto/dark/light"}, {"s", "sign in, when there is no session"}, {"e", "edit API key (Setup tab)"}, {"space", "tick or untick the tool under the cursor (Setup tab)"}, @@ -3628,7 +3668,11 @@ func Run() error { sess = &session.Session{} } client := api.New(sess.Token) - m := newModel(client, sess) + // Before the first frame: a saved theme has to be in force when the panel + // draws, not one keystroke later. + theme := loadTheme() + applyTheme(theme) + m := newModel(client, sess, theme) // Home asks the API for nothing, so there is nothing to wait for: starting // on `loading` would spin forever over a tab that is already drawn. The // data tabs set it themselves in maybeLoad when you walk into them. From 66433521aef87be27d9d129d5b74ece1ccdf8c0b Mon Sep 17 00:00:00 2001 From: Nxssie Date: Tue, 29 Sep 2026 09:25:49 +0100 Subject: [PATCH 3/4] fix(tui): address 4-lens review findings on theme persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings from the pre-PR review (risk/resilience/readability/reliability over a8057e0..d134a9b); none blocked, these are the ones worth fixing now: - reliability CRITICAL: the core contract was never asserted end to end. New test renders the tab bar with the profile pinned to truecolor and the background pinned dark vs light, asserting the panel actually repaints — coverage no longer stops at the renderer flag. - test hygiene: every test that mutates lipgloss's global background flag (and its color profile) now restores the previous value via t.Cleanup, removing hidden order coupling for -shuffle and future tests in the same binary. - coverage: the third press of the cycle must persist "auto" to the file (raw bytes, not loadThemeFrom, which maps unknowns back to auto); saveThemeTo normalizes unknown modes to auto; a refused save still applies the mode in memory. - risk: settings.json is shared, so its read side now decodes with json.Number instead of float64 — a foreign integer past 2^53 survives the round-trip byte-faithfully, with readJSONConfig's refusal semantics (missing/empty tolerated, unparseable and trailing data never rewritten). - readability: the config-dir rule is exported once as session.Dir and reused by settingsPath, so the theme file cannot drift away from session.json. Deferred as follow-ups: user-visible notice when the theme save fails (deliberate swallow, documented), cwd-relative fallback semantics, folding the remaining fixed colors into theme.go, hue-name tokens. --- internal/session/session.go | 15 +++-- internal/tui/theme.go | 72 ++++++++++++++++++++--- internal/tui/theme_test.go | 110 +++++++++++++++++++++++++++++++++++- 3 files changed, 184 insertions(+), 13 deletions(-) diff --git a/internal/session/session.go b/internal/session/session.go index a94c20c..d3f0134 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -16,7 +16,12 @@ type Session struct { var ErrNotLoggedIn = errors.New("not logged in — run: nan auth login") -func dir() (string, error) { +// Dir is the ~/.config/nan directory, and it is exported on purpose: +// settings.json lives beside session.json, the theme reader is in internal/tui, +// and it cannot call an unexported one. Two copies of this rule would drift, +// and the drift would only show up as a theme saved in one place and looked +// for in another. +func Dir() (string, error) { home, err := os.UserHomeDir() if err != nil { return "", err @@ -29,7 +34,7 @@ func dir() (string, error) { // on Windows and is not where anything is: a member told to look there finds // nothing, and the tilde is not something Explorer resolves. func Path() string { - d, err := dir() + d, err := Dir() if err != nil { return filepath.Join(".config", "nan", "session.json") } @@ -37,7 +42,7 @@ func Path() string { } func Load() (*Session, error) { - d, err := dir() + d, err := Dir() if err != nil { return nil, err } @@ -63,7 +68,7 @@ func Load() (*Session, error) { // it never leaves a half-written session.json behind either, which used to // read on the next run as not being logged in. func Save(s *Session) error { - d, err := dir() + d, err := Dir() if err != nil { return err } @@ -100,7 +105,7 @@ func Save(s *Session) error { } func Delete() error { - d, err := dir() + d, err := Dir() if err != nil { return err } diff --git a/internal/tui/theme.go b/internal/tui/theme.go index f4db179..c044601 100644 --- a/internal/tui/theme.go +++ b/internal/tui/theme.go @@ -2,10 +2,15 @@ package tui import ( "encoding/json" + "errors" + "fmt" + "io" "os" "path/filepath" "github.com/charmbracelet/lipgloss" + + "github.com/nxssie/nan-cli/internal/session" ) // ── palette ─────────────────────────────────────────────────────────────────── @@ -95,14 +100,15 @@ func applyTheme(m ThemeMode) { } // settingsPath is where the theme lives, mirroring session.Path(): the same -// config directory, and the same relative fallback for a home directory that +// config directory, resolved by session.Dir() so settings.json always lands +// beside session.json, and the same relative fallback for a home directory that // cannot be resolved. func settingsPath() string { - home, err := os.UserHomeDir() + d, err := session.Dir() if err != nil { return filepath.Join(".config", "nan", "settings.json") } - return filepath.Join(home, ".config", "nan", "settings.json") + return filepath.Join(d, "settings.json") } func loadTheme() ThemeMode { return loadThemeFrom(settingsPath()) } @@ -135,20 +141,72 @@ func loadThemeFrom(path string) ThemeMode { return ThemeAuto } +// readSettingsConfig reads settings.json with the same refusal semantics as +// readJSONConfig - a missing or empty file is an empty map, and a file that +// will not parse is refused rather than guessed at - but it decodes every +// number as a json.Number instead of a float64. +// +// settings.json is shared, and the CLI is not its only writer: a foreign key +// holds whatever its owner put there. json.Unmarshal turns every JSON number +// into a float64, so an integer past 2^53 comes back rounded, and the +// read-modify-write that only meant to set "theme" rewrites somebody else's +// number on the way through. A json.Number keeps the literal digits, so the +// round-trip writes every foreign value back exactly as it was read. +func readSettingsConfig(path string) (map[string]any, error) { + f, err := os.Open(path) + if os.IsNotExist(err) { + return map[string]any{}, nil + } + if err != nil { + return nil, err + } + defer f.Close() + + // A decoder reads a stream, so on its own it would stop at the first value + // and silently leave a second one unread; json.Unmarshal rejects that as + // trailing data. Decoding a second time is how the stream form gets the + // same strictness back: one value in the file, and the next read is EOF. + dec := json.NewDecoder(f) + dec.UseNumber() + var cfg map[string]any + if err := dec.Decode(&cfg); err != nil { + if err == io.EOF { + // An empty or whitespace-only file is one with no setting in it, + // not one that cannot be parsed. + return map[string]any{}, nil + } + return nil, fmt.Errorf("%s does not parse as JSON, so it was left exactly as it is: %w", + filepath.Base(path), err) + } + var trailing any + if err := dec.Decode(&trailing); err != io.EOF { + if err == nil { + err = errors.New("trailing data after the first JSON value") + } + return nil, fmt.Errorf("%s does not parse as JSON, so it was left exactly as it is: %w", + filepath.Base(path), err) + } + if cfg == nil { + cfg = map[string]any{} + } + return cfg, nil +} + // saveThemeTo writes the theme key back into the settings file, leaving every // other key exactly where it was. // // It is a read-modify-write for the same reason the tool configs are: this file // is shared and one day holds a setting this CLI does not know about, and // writing our single key from scratch would take that setting with it. The -// reader is readJSONConfig, whose whole point is refusing to hand back a file it -// could not parse - so a settings.json somebody is midway through editing is -// left alone rather than flattened, and the error travels out to the caller. +// reader is readSettingsConfig, whose whole point is refusing to hand back a +// file it could not parse - so a settings.json somebody is midway through +// editing is left alone rather than flattened, and the error travels out to the +// caller. func saveThemeTo(path string, m ThemeMode) error { if m != ThemeDark && m != ThemeLight { m = ThemeAuto // only ever one of the three constants, whatever came in } - cfg, err := readJSONConfig(path) + cfg, err := readSettingsConfig(path) if err != nil { return err } diff --git a/internal/tui/theme_test.go b/internal/tui/theme_test.go index 187fcd6..9d7882d 100644 --- a/internal/tui/theme_test.go +++ b/internal/tui/theme_test.go @@ -2,15 +2,33 @@ package tui import ( "os" + "path/filepath" "strings" "testing" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/muesli/termenv" "github.com/nxssie/nan-cli/internal/session" ) +// applyTheme moves two process-global pieces of lipgloss state: the background +// flag and, for correctness of the test only, the colour profile. Every test +// that touches them records what it found and hands it back when it ends, so +// one test forcing a side never colours the next one depending on order. +func restoreBackground(t *testing.T) { + t.Helper() + prev := lipgloss.HasDarkBackground() + t.Cleanup(func() { lipgloss.SetHasDarkBackground(prev) }) +} + +func restoreColorProfile(t *testing.T) { + t.Helper() + prev := lipgloss.ColorProfile() + t.Cleanup(func() { lipgloss.SetColorProfile(prev) }) +} + // ── loading ────────────────────────────────────────────────────────────────── // Every way of not having a usable setting lands on auto, because the theme is @@ -57,7 +75,7 @@ func TestKnownThemeLoads(t *testing.T) { // must not take somebody else's with it. func TestSavingTheThemeKeepsWhatItDoesNotOwn(t *testing.T) { path := tempConfig(t, "settings.json") - existing := `{"theme":"dark","editor":"vim","nested":{"keep":true}}` + existing := `{"theme":"dark","editor":"vim","nested":{"keep":true},"big":1234567890123456789}` if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { t.Fatal(err) } @@ -80,6 +98,29 @@ func TestSavingTheThemeKeepsWhatItDoesNotOwn(t *testing.T) { if !ok || nested["keep"] != true { t.Errorf("nested = %v, a foreign key was lost", cfg["nested"]) } + + // The number is checked on the raw bytes: readJSON unmarshals into float64 + // and would round it away, which is exactly the loss the save has to avoid. + // A value past 2^53 only survives if the reader kept it as a json.Number. + if got := readFile(t, path); !strings.Contains(got, "1234567890123456789") { + t.Errorf("the foreign integer lost precision on the round-trip:\n%s", got) + } +} + +// saveThemeTo only ever writes one of the three known modes: anything else - a +// value from a newer build, a caller passing a literal - normalizes to auto, so +// the file never holds a mode this build cannot apply on the next run. +func TestSavingAnUnknownThemeWritesAuto(t *testing.T) { + path := tempConfig(t, "settings.json") + if err := saveThemeTo(path, ThemeMode("junk")); err != nil { + t.Fatal(err) + } + if got := readFile(t, path); !strings.Contains(got, `"theme": "auto"`) { + t.Errorf("an unknown mode wrote something other than auto:\n%s", got) + } + if got := loadThemeFrom(path); got != ThemeAuto { + t.Errorf("the file reads back as %q, want auto", got) + } } // A file that cannot be parsed is the one case where writing would do real @@ -132,6 +173,8 @@ func TestNextThemeCyclesAutoDarkLight(t *testing.T) { // which is what lets a mode change re-colour the whole panel without a style // being rebuilt - and why auto can leave detection alone and still be correct. func TestApplyThemeForcesTheRendererAndAutoLeavesIt(t *testing.T) { + restoreBackground(t) + applyTheme(ThemeDark) if !lipgloss.HasDarkBackground() { t.Error("dark did not pin the renderer to a dark background") @@ -155,11 +198,39 @@ func TestApplyThemeForcesTheRendererAndAutoLeavesIt(t *testing.T) { } } +// The whole point of the feature is that a mode change repaints the panel, and +// applyTheme is only trusted to do it. This asserts it end-to-end: the same +// styled render, with the renderer pinned dark and then light, has to come out +// different. A test that only checked the background flag would pass even if no +// style ever read it. +func TestThemeChangeRepaintsStyledOutput(t *testing.T) { + restoreBackground(t) + restoreColorProfile(t) + + // `go test` has no TTY, so the active profile is whatever the environment + // resolves to - often Ascii, which strips every colour and would make both + // renders identical no matter what the flag says. Pin a colour-capable + // profile for the assertion; the cleanup above hands the previous one back. + lipgloss.SetColorProfile(termenv.TrueColor) + + l := newLayout(80, 24) + lipgloss.SetHasDarkBackground(true) + dark := renderTabBar(0, l) + lipgloss.SetHasDarkBackground(false) + light := renderTabBar(0, l) + + if dark == light { + t.Errorf("the same styled render came out identical under dark and light backgrounds:\n%q", dark) + } +} + // ── the key ────────────────────────────────────────────────────────────────── // `t` advances the mode and writes it, so the next run opens in the mode this // one was left in. func TestThemeKeyCyclesAndPersists(t *testing.T) { + restoreBackground(t) + home := t.TempDir() t.Setenv("HOME", home) t.Setenv("USERPROFILE", home) @@ -186,6 +257,41 @@ func TestThemeKeyCyclesAndPersists(t *testing.T) { if m = press(m); m.theme != ThemeAuto { t.Errorf("third press = %q, want auto", m.theme) } + // auto is asserted on the raw bytes: loadThemeFrom maps any unknown string + // back to auto, so it cannot tell a stored "auto" from a stored "junk". + if got := readFile(t, settingsPath()); !strings.Contains(got, `"theme": "auto"`) { + t.Errorf("the third press did not write auto to the file:\n%s", got) + } +} + +// The documented contract for `t`: the mode is applied in memory even when the +// save refuses. A settings.json that cannot be parsed is the refusal case - the +// save leaves the member's bytes alone and reports the error - and this pins the +// deliberate swallow in Update: the model still advances to dark. +func TestThemeKeyAppliesEvenWhenTheSaveRefuses(t *testing.T) { + restoreBackground(t) + + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + path := settingsPath() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + broken := `{"theme":"dark","editor":"vim"` + if err := os.WriteFile(path, []byte(broken), 0o600); err != nil { + t.Fatal(err) + } + + m := newModel(nil, &session.Session{}, ThemeAuto) + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'t'}}) + if got := updated.(model).theme; got != ThemeDark { + t.Errorf("the press left the model at %q, want dark even though the save refused", got) + } + if got := readFile(t, path); got != broken { + t.Errorf("the refused save rewrote the member's file:\n%s", got) + } } // The theme is the panel's key: not one the help overlay swallows, and not one @@ -215,6 +321,8 @@ func TestThemeKeyIsGuardedLikeTheOtherGlobalKeys(t *testing.T) { // Auto's outcome is not written anywhere else, so the row says which side the // terminal resolved to; the explicit modes just name themselves. func TestAboutShowsTheThemeAndWhatAutoResolvedTo(t *testing.T) { + restoreBackground(t) + l := newLayout(100, 30) if out := (model{theme: ThemeLight}).renderAbout(l); !strings.Contains(out, "Theme:") || !strings.Contains(out, "light") { From 369653a5a0b52d7f683dbee67e159e01c6d35c9c Mon Sep 17 00:00:00 2001 From: Nxssie Date: Wed, 7 Oct 2026 13:26:43 +0100 Subject: [PATCH 4/4] chore(gitignore): ignore local Pi runtime state The `.atl/` directory is local tooling state (skill registry output), not shipped content. Anchored to the repo root so it does not silently swallow a same-named path at any depth. --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index f5448ec..612c94a 100644 --- a/.gitignore +++ b/.gitignore @@ -46,3 +46,5 @@ tmp/ temp/ *.log nan.exe +# Local Pi runtime state +/.atl/