From 2c4bd67e800cb0c9ab461443488f15d54adbaac4 Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:16:09 -0600 Subject: [PATCH 1/9] feat(theme): accept a light/dark theme table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `theme = "auto"` only ever chose between github-light-default and github-dark-default, so a reader who wanted one-light and one-dark-pro had no way to follow their terminal. Accept a `[theme]` table naming both sides instead, with an optional `fallback` for terminals that never answer the background probe, following Helix's config shape. The committed preference now stays whatever config asked for until someone picks a theme in the selector, so quitting without touching themes no longer rewrites `auto` — or a pair — into the one id it happened to resolve to. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013Y3vNR6iJEH4epjQo8JARe --- .changeset/tidy-hounds-repeat.md | 5 + docs/themes.md | 19 ++ packages/hunk/src/app/startup.test.ts | 57 +++++ packages/hunk/src/app/startup.ts | 7 +- packages/hunk/src/core/bootstrap.ts | 3 +- packages/hunk/src/core/run/commandInputs.ts | 3 +- packages/hunk/src/core/run/config.test.ts | 214 ++++++++++++++++++ packages/hunk/src/core/run/config.ts | 152 +++++++++++-- .../hunk/src/core/theme/selection.test.ts | 108 +++++++++ packages/hunk/src/core/theme/selection.ts | 99 ++++++++ packages/hunk/src/ui/App.tsx | 5 +- .../hooks/useThemeSelectorController.test.tsx | 52 +++++ .../ui/hooks/useThemeSelectorController.ts | 24 +- packages/hunk/src/ui/themes.test.ts | 22 ++ packages/hunk/src/ui/themes.ts | 8 +- test/pty/chrome.test.ts | 56 ++++- .../src/content/docs/docs/configure/themes.md | 15 ++ .../src/content/docs/docs/reference/config.md | 6 +- 18 files changed, 822 insertions(+), 33 deletions(-) create mode 100644 .changeset/tidy-hounds-repeat.md create mode 100644 packages/hunk/src/core/theme/selection.test.ts create mode 100644 packages/hunk/src/core/theme/selection.ts diff --git a/.changeset/tidy-hounds-repeat.md b/.changeset/tidy-hounds-repeat.md new file mode 100644 index 000000000..c2972fa29 --- /dev/null +++ b/.changeset/tidy-hounds-repeat.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Accept a `[theme]` table that names one theme per terminal background, so `dark` and `light` terminals each get a theme you chose instead of only Hunk's GitHub defaults. `fallback` covers terminals that never report a background. diff --git a/docs/themes.md b/docs/themes.md index e953491ac..4f8ca53ee 100644 --- a/docs/themes.md +++ b/docs/themes.md @@ -18,6 +18,25 @@ Hunk chooses `github-light-default` for light backgrounds and `github-dark-default` for dark backgrounds, falling back to `github-dark-default` when the terminal does not answer. +To pick the two themes yourself, write `theme` as a table instead of an id: + +```toml +[theme] +dark = "catppuccin-mocha" # required +light = "catppuccin-latte" # required +fallback = "github-dark-default" # optional +``` + +Hunk queries the terminal background the same way `auto` does, then draws +`dark` or `light`. `fallback` covers sessions where Hunk never gets an answer: +terminals that ignore the query, and captured pager hosts such as LazyGit, +where Hunk never asks. Without it those sessions use `dark`. Both sides accept +any built-in id, a custom theme id, and the compatibility aliases. + +A `--theme ` flag overrides the table for that run, and picking a theme in +the app (`t`, or `View -> Themes…`) replaces the pair with the single id you +chose — the save-on-quit prompt shows that before writing anything. + Older theme ids such as `graphite` and `paper` remain accepted as compatibility aliases. diff --git a/packages/hunk/src/app/startup.test.ts b/packages/hunk/src/app/startup.test.ts index bc6edbcc1..3a556fe36 100644 --- a/packages/hunk/src/app/startup.test.ts +++ b/packages/hunk/src/app/startup.test.ts @@ -775,6 +775,63 @@ describe("startup planning", () => { expect(detected).toBe(0); }); + test("detects the terminal background for an adaptive theme pair", async () => { + const cliInput: CliInput = { + kind: "patch", + file: "-", + options: { + theme: { dark: "vitesse-dark", light: "one-light" }, + pager: true, + }, + }; + const controllingTerminal = { stdin: {} as never, close: () => {} }; + let probes = 0; + + const plan = await prepareStartupPlan(["bun", "hunk", "patch", "-"], { + parseCliImpl: async () => cliInput as ParsedCliInput, + resolveRuntimeCliInputImpl: (input) => input, + resolveConfiguredCliInputImpl: (input) => createTestConfigResolution(input), + loadAppBootstrapImpl: async (input) => createBootstrap(input), + openControllingTerminalImpl: () => controllingTerminal, + detectTerminalThemeModeFromBackgroundImpl: async () => { + probes += 1; + return "light"; + }, + stdinIsTTY: false, + stdoutIsTTY: true, + stdout: { write: () => true } as never, + }); + + expect(plan).toMatchObject({ kind: "app", bootstrap: { initialThemeMode: "light" } }); + expect(probes).toBe(1); + }); + + test("skips the background probe when one theme covers every terminal", async () => { + const cliInput: CliInput = { + kind: "patch", + file: "-", + options: { theme: "dracula", pager: true }, + }; + let probes = 0; + + await prepareStartupPlan(["bun", "hunk", "patch", "-", "--theme", "dracula"], { + parseCliImpl: async () => cliInput as ParsedCliInput, + resolveRuntimeCliInputImpl: (input) => input, + resolveConfiguredCliInputImpl: (input) => createTestConfigResolution(input), + loadAppBootstrapImpl: async (input) => createBootstrap(input), + openControllingTerminalImpl: () => ({ stdin: {} as never, close: () => {} }), + detectTerminalThemeModeFromBackgroundImpl: async () => { + probes += 1; + return "dark"; + }, + stdinIsTTY: false, + stdoutIsTTY: true, + stdout: { write: () => true } as never, + }); + + expect(probes).toBe(0); + }); + test("opens the controlling terminal for piped patch startup", async () => { const cliInput: CliInput = { kind: "patch", diff --git a/packages/hunk/src/app/startup.ts b/packages/hunk/src/app/startup.ts index 0459ca847..a0c5b0e8b 100644 --- a/packages/hunk/src/app/startup.ts +++ b/packages/hunk/src/app/startup.ts @@ -7,6 +7,7 @@ import type { loadAppBootstrap } from "../core/changeset/loaders"; import { looksLikePatchInput } from "../core/process/pager"; import { sanitizeTerminalText } from "../lib/terminalText"; import { detectTerminalThemeModeFromBackground } from "../core/theme/detection"; +import { themeSelectionNeedsTerminalMode } from "../core/theme/selection"; import { openControllingTerminal, resolveRuntimeCliInput, @@ -557,7 +558,11 @@ export async function prepareStartupPlan( // Embedded reviews inherit their owner's detected mode so bootstrap never queries a terminal // whose input and renderer are already exclusively owned. let initialThemeMode: AppBootstrap["initialThemeMode"] = deps.terminalThemeMode; - if (!initialThemeMode && cliInput.options.theme === "auto" && stdoutIsTTY) { + if ( + !initialThemeMode && + themeSelectionNeedsTerminalMode(cliInput.options.theme) && + stdoutIsTTY + ) { const themeInput = controllingTerminal?.stdin ?? (stdinIsTTY ? process.stdin : null); if (themeInput) { initialThemeMode = diff --git a/packages/hunk/src/core/bootstrap.ts b/packages/hunk/src/core/bootstrap.ts index 8e5288180..f541efe0f 100644 --- a/packages/hunk/src/core/bootstrap.ts +++ b/packages/hunk/src/core/bootstrap.ts @@ -17,6 +17,7 @@ import type { CliInput, CursorLine, LayoutMode, SidebarVisibility } from "./run/ import type { UserKeyBinding } from "./run/config"; import type { StartupNotice } from "./process/startupNotice"; import type { TerminalThemeMode } from "./theme/detection"; +import type { ThemeSelection } from "./theme/selection"; import type { VcsCatalog } from "./vcs/types"; /** Where a review was loaded from, retained so the session can reload and watch it. */ @@ -39,7 +40,7 @@ export interface AppBootstrap { reloadContext: ReloadContext; changeset: Changeset; initialMode: LayoutMode; - initialTheme?: string; + initialTheme?: ThemeSelection; initialThemeMode?: TerminalThemeMode; /** Selectable custom themes for this session, in menu order. */ customThemes?: readonly NamedCustomThemeConfig[]; diff --git a/packages/hunk/src/core/run/commandInputs.ts b/packages/hunk/src/core/run/commandInputs.ts index 5713d571a..25c31c1cf 100644 --- a/packages/hunk/src/core/run/commandInputs.ts +++ b/packages/hunk/src/core/run/commandInputs.ts @@ -14,6 +14,7 @@ import type { ExtensionVcsStashShowInput, } from "../../extension-api/types"; import type { InstallSource } from "../install/installSource"; +import type { ThemeSelection } from "../theme/selection"; export type LayoutMode = "auto" | "split" | "stack"; export type CursorLine = "row" | "number" | "off"; @@ -24,7 +25,7 @@ export interface CommonOptions { mode?: LayoutMode; cursorLine?: CursorLine; vcs?: VcsMode; - theme?: string; + theme?: ThemeSelection; agentContext?: string; pager?: boolean; watch?: boolean; diff --git a/packages/hunk/src/core/run/config.test.ts b/packages/hunk/src/core/run/config.test.ts index 9b5f312e7..4aa2ce3ea 100644 --- a/packages/hunk/src/core/run/config.test.ts +++ b/packages/hunk/src/core/run/config.test.ts @@ -163,6 +163,220 @@ describe("config persistence", () => { }); }); +describe("adaptive theme config", () => { + function writeUserConfig(home: string, lines: readonly string[]) { + const configPath = join(home, ".config", "hunk", "config.toml"); + mkdirSync(join(home, ".config", "hunk"), { recursive: true }); + writeFileSync(configPath, lines.join("\n")); + return configPath; + } + + test("reads a [theme] table into an adaptive selection", () => { + const home = createTempDir("hunk-adaptive-theme-home-"); + const repo = createTempDir("hunk-adaptive-theme-repo-"); + createRepo(repo); + writeUserConfig(home, [ + "[theme]", + 'dark = "catppuccin-mocha"', + 'light = "catppuccin-latte"', + 'fallback = "nord"', + ]); + + const resolved = resolveConfiguredCliInput(createPatchPagerInput(), { + cwd: repo, + env: { HOME: home }, + }); + + expect(resolved.input.options.theme).toEqual({ + dark: "catppuccin-mocha", + light: "catppuccin-latte", + fallback: "nord", + }); + }); + + test("lets an explicit --theme id outrank a configured pair", () => { + const home = createTempDir("hunk-adaptive-theme-cli-home-"); + const repo = createTempDir("hunk-adaptive-theme-cli-repo-"); + createRepo(repo); + writeUserConfig(home, ["[theme]", 'dark = "vitesse-dark"', 'light = "vitesse-light"']); + + const resolved = resolveConfiguredCliInput(createPatchPagerInput({ theme: "dracula" }), { + cwd: repo, + env: { HOME: home }, + }); + + expect(resolved.input.options.theme).toBe("dracula"); + }); + + test("lets a repo layer replace a user pair with one id", () => { + const home = createTempDir("hunk-adaptive-theme-layer-home-"); + const repo = createTempDir("hunk-adaptive-theme-layer-repo-"); + createRepo(repo); + writeUserConfig(home, ["[theme]", 'dark = "vitesse-dark"', 'light = "vitesse-light"']); + mkdirSync(join(repo, ".hunk"), { recursive: true }); + writeFileSync(join(repo, ".hunk", "config.toml"), 'theme = "nord"\n'); + + const resolved = resolveConfiguredCliInput(createPatchPagerInput(), { + cwd: repo, + env: { HOME: home }, + }); + + expect(resolved.input.options.theme).toBe("nord"); + }); + + test("rejects a [theme] table that leaves one background unanswered", () => { + const home = createTempDir("hunk-adaptive-theme-invalid-home-"); + const repo = createTempDir("hunk-adaptive-theme-invalid-repo-"); + createRepo(repo); + writeUserConfig(home, ["[theme]", 'dark = "vitesse-dark"']); + + expect(() => + resolveConfiguredCliInput(createPatchPagerInput(), { cwd: repo, env: { HOME: home } }), + ).toThrow("Expected [theme] to set both `dark` and `light` to theme ids."); + }); + + test("requires a [custom_theme] table when a pair names the custom theme", () => { + const home = createTempDir("hunk-adaptive-theme-custom-home-"); + const repo = createTempDir("hunk-adaptive-theme-custom-repo-"); + createRepo(repo); + writeUserConfig(home, ["[theme]", 'dark = "custom"', 'light = "github-light-default"']); + + expect(() => + resolveConfiguredCliInput(createPatchPagerInput(), { cwd: repo, env: { HOME: home } }), + ).toThrow('Expected a [custom_theme] table when config selects theme = "custom".'); + }); + + test("treats an unchanged pair as clean and shows the collapse when one theme is picked", () => { + const base = { + mode: "auto", + theme: { dark: "vitesse-dark", light: "vitesse-light" }, + showLineNumbers: true, + wrapLines: false, + showHunkHeaders: true, + showMenuBar: true, + showAgentNotes: false, + copyDecorations: false, + cursorLine: "row", + } as const; + + expect( + diffPersistedViewPreferences(base, { + ...base, + theme: { dark: "vitesse-dark", light: "vitesse-light" }, + }), + ).toEqual([]); + expect(diffPersistedViewPreferences(base, { ...base, theme: "dracula" })).toEqual([ + { + configKey: "theme", + previousValue: '{ dark = "vitesse-dark", light = "vitesse-light" }', + nextValue: '"dracula"', + }, + ]); + }); + + test("rewrites a [theme] table in place instead of duplicating the key", () => { + const home = createTempDir("hunk-adaptive-theme-save-home-"); + const configPath = writeUserConfig(home, [ + "wrap_lines = false", + "", + "[theme]", + "# follow the terminal", + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + "", + "[custom_theme]", + 'label = "Keep me"', + ]); + + saveGlobalViewPreferences( + { + mode: "auto", + theme: { dark: "nord", light: "one-light", fallback: "nord" }, + showLineNumbers: true, + wrapLines: true, + showHunkHeaders: true, + showMenuBar: true, + showAgentNotes: false, + copyDecorations: false, + cursorLine: "row", + }, + { configPath }, + ); + + const saved = readFileSync(configPath, "utf8"); + expect(saved).toContain( + [ + "[theme]", + "# follow the terminal", + 'dark = "nord"', + 'light = "one-light"', + 'fallback = "nord"', + "", + "[custom_theme]", + 'label = "Keep me"', + ].join("\n"), + ); + expect(saved).not.toContain("theme = "); + expect(saved).toContain("wrap_lines = true"); + }); + + test("removes the [theme] table when a single theme replaces the pair", () => { + const home = createTempDir("hunk-adaptive-theme-collapse-home-"); + const configPath = writeUserConfig(home, [ + "[theme]", + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + "", + "[custom_theme]", + 'label = "Keep me"', + ]); + + saveGlobalViewPreferences( + { + mode: "auto", + theme: "dracula", + showLineNumbers: true, + wrapLines: false, + showHunkHeaders: true, + showMenuBar: true, + showAgentNotes: false, + copyDecorations: false, + cursorLine: "row", + }, + { configPath }, + ); + + const saved = readFileSync(configPath, "utf8"); + expect(saved).toContain('theme = "dracula"'); + expect(saved).not.toContain("[theme]"); + expect(saved).toContain("[custom_theme]"); + }); + + test("writes a pair as an inline table when the file has no [theme] section", () => { + const home = createTempDir("hunk-adaptive-theme-inline-home-"); + const configPath = writeUserConfig(home, ['theme = "dracula"', "wrap_lines = false"]); + + saveGlobalViewPreferences( + { + mode: "auto", + theme: { dark: "nord", light: "one-light" }, + showLineNumbers: true, + wrapLines: false, + showHunkHeaders: true, + showMenuBar: true, + showAgentNotes: false, + copyDecorations: false, + cursorLine: "row", + }, + { configPath }, + ); + + expect(readFileSync(configPath, "utf8")).toContain( + 'theme = { dark = "nord", light = "one-light" }', + ); + }); +}); + describe("config resolution", () => { test("merges global, repo, pager, command, and CLI overrides in the right order", () => { const home = createTempDir("hunk-config-home-"); diff --git a/packages/hunk/src/core/run/config.ts b/packages/hunk/src/core/run/config.ts index 940d26f87..449bc46dd 100644 --- a/packages/hunk/src/core/run/config.ts +++ b/packages/hunk/src/core/run/config.ts @@ -16,6 +16,13 @@ import { LEGACY_CUSTOM_SYNTAX_COLOR_KEYS, resolveSyntaxScopeOverrides, } from "../theme/legacySyntaxScopes"; +import { + ADAPTIVE_THEME_SELECTION_KEYS, + isAdaptiveThemeSelection, + readThemeSelection, + themeSelectionsEqual, + type ThemeSelection, +} from "../theme/selection"; import { resolveGlobalConfigPath } from "./paths"; import { LEGACY_CUSTOM_SYNTAX_NOTICES, type StartupNotice } from "../process/startupNotice"; import { @@ -74,7 +81,7 @@ export type UserKeyBinding = string | readonly string[] | false; /** The view options a session persists back to config when the reader saves them. */ export interface PersistedViewPreferences { mode: LayoutMode; - theme?: string; + theme?: ThemeSelection; showLineNumbers: boolean; wrapLines: boolean; showHunkHeaders: boolean; @@ -117,11 +124,25 @@ export function persistedViewPreferencesFromOptions( } const VIEW_PREFERENCES_PROMPT_CONFIG_KEY = "prompt_save_view_preferences"; + +type PersistedPreferenceValue = string | boolean | ThemeSelection | undefined; + const PERSISTED_VIEW_PREFERENCE_KEYS: Array<{ configKey: string; - value: (preferences: PersistedViewPreferences) => string | boolean | undefined; + value: (preferences: PersistedViewPreferences) => PersistedPreferenceValue; + equals?: (previous: PersistedPreferenceValue, next: PersistedPreferenceValue) => boolean; + upsert?: (source: string, value: PersistedPreferenceValue) => string; }> = [ - { configKey: "theme", value: (preferences) => preferences.theme }, + { + configKey: "theme", + value: (preferences) => preferences.theme, + equals: (previous, next) => + themeSelectionsEqual( + previous as ThemeSelection | undefined, + next as ThemeSelection | undefined, + ), + upsert: (source, value) => upsertThemeTomlValue(source, value as ThemeSelection), + }, { configKey: "mode", value: (preferences) => preferences.mode }, { configKey: "line_numbers", value: (preferences) => preferences.showLineNumbers }, { configKey: "wrap_lines", value: (preferences) => preferences.wrapLines }, @@ -189,17 +210,28 @@ function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } -/** Serialize one primitive TOML preference value. */ -function serializeTomlPreferenceValue(value: string | boolean) { +/** Serialize one primitive or inline-table TOML preference value. */ +function serializeTomlPreferenceValue(value: string | boolean | ThemeSelection) { if (typeof value === "boolean") { return value ? "true" : "false"; } + if (isAdaptiveThemeSelection(value)) { + const entries = ADAPTIVE_THEME_SELECTION_KEYS.filter((key) => value[key] !== undefined).map( + (key) => `${key} = ${JSON.stringify(value[key])}`, + ); + return `{ ${entries.join(", ")} }`; + } + return JSON.stringify(value); } /** Update one top-level TOML key while preserving sections and unrelated comments. */ -function upsertTopLevelTomlValue(source: string, key: string, value: string | boolean) { +function upsertTopLevelTomlValue( + source: string, + key: string, + value: string | boolean | ThemeSelection, +) { const lines = source.length > 0 ? source.split("\n") : []; const serialized = serializeTomlPreferenceValue(value); const assignment = `${key} = ${serialized}`; @@ -230,6 +262,72 @@ function upsertTopLevelTomlValue(source: string, key: string, value: string | bo return `${lines.join("\n").replace(/\n*$/, "")}\n`; } +function findThemeTableRange(lines: readonly string[]) { + const headerIndex = lines.findIndex((line) => /^\s*\[\s*theme\s*\]\s*(?:#.*)?$/.test(line)); + if (headerIndex < 0) { + return null; + } + + const nextHeaderOffset = lines.slice(headerIndex + 1).findIndex((line) => /^\s*\[/.test(line)); + const end = nextHeaderOffset < 0 ? lines.length : headerIndex + 1 + nextHeaderOffset; + return { headerIndex, end }; +} + +function applyThemeTableKey( + lines: string[], + range: { headerIndex: number; end: number }, + key: string, + value: string | undefined, +) { + const keyPattern = new RegExp(`^\\s*${key}\\s*=`); + const existingIndex = lines + .slice(range.headerIndex + 1, range.end) + .findIndex((line) => keyPattern.test(line)); + if (existingIndex >= 0) { + const absolute = range.headerIndex + 1 + existingIndex; + if (value === undefined) { + lines.splice(absolute, 1); + range.end -= 1; + return; + } + lines[absolute] = `${key} = ${JSON.stringify(value)}`; + return; + } + + if (value === undefined) { + return; + } + + let insertAt = range.end; + while (insertAt > range.headerIndex + 1 && (lines[insertAt - 1] ?? "").trim().length === 0) { + insertAt -= 1; + } + lines.splice(insertAt, 0, `${key} = ${JSON.stringify(value)}`); + range.end += 1; +} + +function upsertThemeTomlValue(source: string, value: ThemeSelection | undefined) { + if (value === undefined) { + return source; + } + + const lines = source.length > 0 ? source.split("\n") : []; + const range = findThemeTableRange(lines); + if (!range) { + return upsertTopLevelTomlValue(source, "theme", value); + } + + if (isAdaptiveThemeSelection(value)) { + for (const key of ADAPTIVE_THEME_SELECTION_KEYS) { + applyThemeTableKey(lines, range, key, value[key]); + } + return `${lines.join("\n").replace(/\n*$/, "")}\n`; + } + + lines.splice(range.headerIndex, range.end - range.headerIndex); + return upsertTopLevelTomlValue(`${lines.join("\n").replace(/\n*$/, "")}\n`, "theme", value); +} + /** Accept only the layout names Hunk already supports. */ function normalizeLayoutMode(value: unknown): LayoutMode | undefined { return value === "auto" || value === "split" || value === "stack" ? value : undefined; @@ -267,6 +365,19 @@ function normalizeBoolean(value: unknown) { return typeof value === "boolean" ? value : undefined; } +function normalizeThemeSelection(value: unknown) { + const read = readThemeSelection(value); + if (read === undefined) { + return undefined; + } + + if ("issue" in read) { + throw new Error(read.issue); + } + + return read.selection; +} + /** Accept only plain strings from config files. */ function normalizeString(value: unknown) { return typeof value === "string" && value.length > 0 ? value : undefined; @@ -350,10 +461,12 @@ export const CONFIG_REFERENCE_OPTIONS: readonly ConfigReferenceOption[] = [ { key: "theme", property: "theme", - type: "string", - accepted: "a built-in theme id or `custom`", + type: "string or table", + accepted: + "a built-in theme id, `custom`, `auto`, or a `[theme]` table setting `dark` and `light` (plus an optional `fallback`)", runtimeDefault: DEFAULT_THEME_ID, - description: "Select the active color theme.", + description: + "Select the active color theme, or one theme per terminal background. A `[theme]` table follows the terminal between its `dark` and `light` ids, using `fallback` (else `dark`) when the terminal does not report a background.", }, { key: "watch", @@ -974,7 +1087,7 @@ function normalizeConfigReferenceValue(property: keyof CommonOptions, value: unk case "vcs": return normalizeVcsMode(value); case "theme": - return normalizeString(value); + return normalizeThemeSelection(value); case "tabWidth": return normalizeTabWidth(value); case "fileGap": @@ -1133,7 +1246,10 @@ export function diffPersistedViewPreferences( for (const key of PERSISTED_VIEW_PREFERENCE_KEYS) { const previousValue = key.value(previous); const nextValue = key.value(next); - if (previousValue === nextValue) { + const unchanged = key.equals + ? key.equals(previousValue, nextValue) + : previousValue === nextValue; + if (unchanged) { continue; } @@ -1160,9 +1276,13 @@ export function saveGlobalViewPreferences( let nextSource = readConfigSource(configPath); for (const key of PERSISTED_VIEW_PREFERENCE_KEYS) { const value = key.value(preferences); - if (value !== undefined) { - nextSource = upsertTopLevelTomlValue(nextSource, key.configKey, value); + if (value === undefined) { + continue; } + + nextSource = key.upsert + ? key.upsert(nextSource, value) + : upsertTopLevelTomlValue(nextSource, key.configKey, value); } writeConfigSource(configPath, nextSource); @@ -1330,8 +1450,12 @@ export function resolveConfiguredCliInput( // Only the legacy `custom` id is a hard error: every other unknown id may still name a theme an // extension contributes later, so those fall back to the default theme instead of failing startup. + const themeSelection = resolvedOptions.theme; + const selectedThemeIds = isAdaptiveThemeSelection(themeSelection) + ? ADAPTIVE_THEME_SELECTION_KEYS.map((key) => themeSelection[key]) + : [themeSelection]; if ( - resolvedOptions.theme === LEGACY_CUSTOM_THEME_ID && + selectedThemeIds.includes(LEGACY_CUSTOM_THEME_ID) && !resolvedCustomThemes.some((theme) => theme.id === LEGACY_CUSTOM_THEME_ID) ) { throw new Error('Expected a [custom_theme] table when config selects theme = "custom".'); diff --git a/packages/hunk/src/core/theme/selection.test.ts b/packages/hunk/src/core/theme/selection.test.ts new file mode 100644 index 000000000..2f1de8900 --- /dev/null +++ b/packages/hunk/src/core/theme/selection.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, test } from "bun:test"; +import { + chooseThemeSelectionId, + isAdaptiveThemeSelection, + readThemeSelection, + themeSelectionsEqual, + themeSelectionNeedsTerminalMode, +} from "./selection"; + +describe("readThemeSelection", () => { + test("accepts one theme id and reports nothing for an unset or empty value", () => { + expect(readThemeSelection("nord")).toEqual({ selection: "nord" }); + expect(readThemeSelection("auto")).toEqual({ selection: "auto" }); + expect(readThemeSelection(undefined)).toBeUndefined(); + expect(readThemeSelection("")).toBeUndefined(); + }); + + test("accepts an adaptive pair with and without a fallback", () => { + expect(readThemeSelection({ dark: "catppuccin-mocha", light: "catppuccin-latte" })).toEqual({ + selection: { dark: "catppuccin-mocha", light: "catppuccin-latte" }, + }); + expect( + readThemeSelection({ dark: "vitesse-dark", light: "vitesse-light", fallback: "nord" }), + ).toEqual({ selection: { dark: "vitesse-dark", light: "vitesse-light", fallback: "nord" } }); + }); + + test("requires both backgrounds so neither terminal falls back to a Hunk default", () => { + const read = readThemeSelection({ dark: "nord" }); + expect(read).toEqual({ + issue: "Expected [theme] to set both `dark` and `light` to theme ids.", + }); + }); + + test("rejects unknown keys so a typo surfaces instead of doing nothing", () => { + const read = readThemeSelection({ dark: "nord", light: "one-light", defualt: "nord" }); + expect(read).toEqual({ + issue: "Expected [theme] to contain only dark, light, fallback. Unexpected: `defualt`.", + }); + }); + + test("rejects non-string ids and values that are neither an id nor a table", () => { + expect(readThemeSelection({ dark: "nord", light: "one-light", fallback: 7 })).toEqual({ + issue: "Expected theme.fallback to be a theme id.", + }); + expect(readThemeSelection(["nord"])).toEqual({ + issue: "Expected theme to be a theme id or a table of theme ids.", + }); + }); + + test("names the key path it was given so nested tables explain themselves", () => { + expect(readThemeSelection({ dark: 1 }, "pager.theme")).toEqual({ + issue: "Expected [pager.theme] to set both `dark` and `light` to theme ids.", + }); + }); +}); + +describe("chooseThemeSelectionId", () => { + const adaptive = { dark: "vitesse-dark", light: "vitesse-light" }; + + test("passes a plain id through for every background", () => { + expect(chooseThemeSelectionId("nord", "light")).toBe("nord"); + expect(chooseThemeSelectionId("nord", null)).toBe("nord"); + expect(chooseThemeSelectionId(undefined, "dark")).toBeUndefined(); + }); + + test("follows the detected background across an adaptive pair", () => { + expect(chooseThemeSelectionId(adaptive, "light")).toBe("vitesse-light"); + expect(chooseThemeSelectionId(adaptive, "dark")).toBe("vitesse-dark"); + }); + + test("takes fallback when the terminal never answered, and dark when none is set", () => { + expect(chooseThemeSelectionId({ ...adaptive, fallback: "nord" }, null)).toBe("nord"); + expect(chooseThemeSelectionId(adaptive, null)).toBe("vitesse-dark"); + expect(chooseThemeSelectionId(adaptive, undefined)).toBe("vitesse-dark"); + }); +}); + +describe("selection predicates", () => { + test("probes the terminal only when the answer can change the theme", () => { + expect(themeSelectionNeedsTerminalMode("auto")).toBe(true); + expect(themeSelectionNeedsTerminalMode({ dark: "nord", light: "one-light" })).toBe(true); + expect(themeSelectionNeedsTerminalMode("nord")).toBe(false); + expect(themeSelectionNeedsTerminalMode(undefined)).toBe(false); + }); + + test("narrows adaptive pairs away from ids", () => { + expect(isAdaptiveThemeSelection({ dark: "nord", light: "one-light" })).toBe(true); + expect(isAdaptiveThemeSelection("nord")).toBe(false); + expect(isAdaptiveThemeSelection(undefined)).toBe(false); + }); + + test("compares pairs by value so an untouched preference never looks dirty", () => { + expect( + themeSelectionsEqual( + { dark: "nord", light: "one-light" }, + { dark: "nord", light: "one-light" }, + ), + ).toBe(true); + expect( + themeSelectionsEqual( + { dark: "nord", light: "one-light" }, + { dark: "nord", light: "one-light", fallback: "nord" }, + ), + ).toBe(false); + expect(themeSelectionsEqual("nord", { dark: "nord", light: "one-light" })).toBe(false); + expect(themeSelectionsEqual("nord", "nord")).toBe(true); + }); +}); diff --git a/packages/hunk/src/core/theme/selection.ts b/packages/hunk/src/core/theme/selection.ts new file mode 100644 index 000000000..733965c03 --- /dev/null +++ b/packages/hunk/src/core/theme/selection.ts @@ -0,0 +1,99 @@ +export type ThemeSelectionMode = "light" | "dark"; + +export interface AdaptiveThemeSelection { + dark: string; + light: string; + /** Theme used when the terminal never answered the background probe. Defaults to `dark`. */ + fallback?: string; +} + +export type ThemeSelection = string | AdaptiveThemeSelection; + +export const AUTO_THEME_ID = "auto"; + +export const ADAPTIVE_THEME_SELECTION_KEYS = ["dark", "light", "fallback"] as const; + +export function isAdaptiveThemeSelection( + selection: ThemeSelection | undefined, +): selection is AdaptiveThemeSelection { + return typeof selection === "object" && selection !== null && !Array.isArray(selection); +} + +export function chooseThemeSelectionId( + selection: ThemeSelection | undefined, + mode: ThemeSelectionMode | null | undefined, +): string | undefined { + if (!isAdaptiveThemeSelection(selection)) { + return selection; + } + + if (mode === "light") return selection.light; + if (mode === "dark") return selection.dark; + return selection.fallback ?? selection.dark; +} + +export function themeSelectionNeedsTerminalMode(selection: ThemeSelection | undefined): boolean { + return selection === AUTO_THEME_ID || isAdaptiveThemeSelection(selection); +} + +export function themeSelectionsEqual( + left: ThemeSelection | undefined, + right: ThemeSelection | undefined, +): boolean { + if (isAdaptiveThemeSelection(left) && isAdaptiveThemeSelection(right)) { + return ( + left.dark === right.dark && left.light === right.light && left.fallback === right.fallback + ); + } + + return left === right; +} + +/** Read a `theme` config value into a selection, or explain why the table is unusable. */ +export function readThemeSelection( + value: unknown, + keyPath = "theme", +): { selection: ThemeSelection } | { issue: string } | undefined { + if (value === undefined) { + return undefined; + } + + if (typeof value === "string") { + return value.length > 0 ? { selection: value } : undefined; + } + + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return { issue: `Expected ${keyPath} to be a theme id or a table of theme ids.` }; + } + + const table = value as Record; + const unknownKeys = Object.keys(table).filter( + (key) => !(ADAPTIVE_THEME_SELECTION_KEYS as readonly string[]).includes(key), + ); + if (unknownKeys.length > 0) { + return { + issue: `Expected [${keyPath}] to contain only ${ADAPTIVE_THEME_SELECTION_KEYS.join(", ")}. Unexpected: ${unknownKeys + .map((key) => `\`${key}\``) + .join(", ")}.`, + }; + } + + const readId = (key: string) => { + const entry = table[key]; + return typeof entry === "string" && entry.length > 0 ? entry : undefined; + }; + const dark = readId("dark"); + const light = readId("light"); + if (dark === undefined || light === undefined) { + return { + issue: `Expected [${keyPath}] to set both \`dark\` and \`light\` to theme ids.`, + }; + } + + if ("fallback" in table && readId("fallback") === undefined) { + return { issue: `Expected ${keyPath}.fallback to be a theme id.` }; + } + + const fallback = readId("fallback"); + return { selection: fallback === undefined ? { dark, light } : { dark, light, fallback } }; +} diff --git a/packages/hunk/src/ui/App.tsx b/packages/hunk/src/ui/App.tsx index 3c68e7ccb..88d6b4b79 100644 --- a/packages/hunk/src/ui/App.tsx +++ b/packages/hunk/src/ui/App.tsx @@ -252,6 +252,7 @@ export function App({ activeTheme, baseTheme, themeId, + themeSelection, themeSelectorItems, themeSelectorOpen, themeSelectorSelectedIndex, @@ -271,7 +272,7 @@ export function App({ const currentViewPreferences = useMemo( () => ({ mode: layoutMode, - theme: themeId, + theme: themeSelection, showLineNumbers, wrapLines, showHunkHeaders, @@ -288,7 +289,7 @@ export function App({ showHunkHeaders, showLineNumbers, showMenuBar, - themeId, + themeSelection, wrapLines, ], ); diff --git a/packages/hunk/src/ui/hooks/useThemeSelectorController.test.tsx b/packages/hunk/src/ui/hooks/useThemeSelectorController.test.tsx index 7a0253242..a98dd6b67 100644 --- a/packages/hunk/src/ui/hooks/useThemeSelectorController.test.tsx +++ b/packages/hunk/src/ui/hooks/useThemeSelectorController.test.tsx @@ -56,6 +56,58 @@ function customTheme( const noNotice = () => {}; describe("useThemeSelectorController", () => { + test("draws an adaptive pair from the detected background and keeps the pair committed", async () => { + const adaptive = { dark: "vitesse-dark", light: "one-light" }; + const dark = await renderThemeSelectorController({ + initialTheme: adaptive, + initialThemeMode: "dark", + onTransientNotice: noNotice, + transparentBackground: false, + }); + + try { + expect(dark.controller.baseTheme.id).toBe("vitesse-dark"); + expect(dark.controller.themeId).toBe("vitesse-dark"); + expect(dark.controller.themeSelection).toEqual(adaptive); + } finally { + await destroyController(dark.setup); + } + + const light = await renderThemeSelectorController({ + initialTheme: adaptive, + initialThemeMode: "light", + onTransientNotice: noNotice, + transparentBackground: false, + }); + + try { + expect(light.controller.baseTheme.id).toBe("one-light"); + expect(light.controller.themeSelection).toEqual(adaptive); + } finally { + await destroyController(light.setup); + } + }); + + test("commits one picked theme over an adaptive pair", async () => { + const harness = await renderThemeSelectorController({ + initialTheme: { dark: "vitesse-dark", light: "one-light" }, + initialThemeMode: "dark", + onTransientNotice: noNotice, + transparentBackground: false, + }); + + try { + const draculaIndex = availableThemes().findIndex((theme) => theme.id === "dracula"); + await act(async () => harness.controller.acceptThemeSelectorItem(draculaIndex)); + + expect(harness.controller.themeSelection).toBe("dracula"); + expect(harness.controller.themeId).toBe("dracula"); + expect(harness.controller.baseTheme.id).toBe("dracula"); + } finally { + await destroyController(harness.setup); + } + }); + test("resolves auto initialization from the detected light or dark terminal mode", async () => { const light = await renderThemeSelectorController({ initialTheme: "auto", diff --git a/packages/hunk/src/ui/hooks/useThemeSelectorController.ts b/packages/hunk/src/ui/hooks/useThemeSelectorController.ts index a41d6ed5e..b752be6bc 100644 --- a/packages/hunk/src/ui/hooks/useThemeSelectorController.ts +++ b/packages/hunk/src/ui/hooks/useThemeSelectorController.ts @@ -1,11 +1,16 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import type { TerminalThemeMode } from "../../core/theme/detection"; +import { + AUTO_THEME_ID, + chooseThemeSelectionId, + type ThemeSelection, +} from "../../core/theme/selection"; import type { NamedCustomThemeConfig } from "../../extension-api/types"; import type { ThemeSelectorItem } from "../components/chrome/ThemeSelectorDialog"; import { availableThemes, resolveTheme, withTransparentSurfaces } from "../themes"; interface ThemeSelectorControllerState { - committedThemeId: string; + committedThemeSelection: ThemeSelection; open: boolean; previewThemeId: string | null; selectedThemeId: string | null; @@ -13,7 +18,7 @@ interface ThemeSelectorControllerState { export interface UseThemeSelectorControllerOptions { customThemes?: readonly NamedCustomThemeConfig[]; - initialTheme?: string; + initialTheme?: ThemeSelection; initialThemeMode?: TerminalThemeMode | null; onTransientNotice: (text: string) => void; /** Observe committed choices so a remounting surface can retain them. */ @@ -34,7 +39,7 @@ export function useThemeSelectorController({ // incoming record, but they must not reinterpret an in-session theme choice. const [detectedThemeMode] = useState(initialThemeMode); const [state, setState] = useState(() => ({ - committedThemeId: resolveTheme(initialTheme, initialThemeMode ?? null, customThemes).id, + committedThemeSelection: initialTheme ?? resolveTheme(undefined, initialThemeMode ?? null).id, open: false, previewThemeId: null, selectedThemeId: null, @@ -42,9 +47,13 @@ export function useThemeSelectorController({ const themeOptions = useMemo(() => availableThemes(customThemes), [customThemes]); const committedTheme = useMemo( - () => resolveTheme(state.committedThemeId, detectedThemeMode ?? null, customThemes), - [customThemes, detectedThemeMode, state.committedThemeId], + () => resolveTheme(state.committedThemeSelection, detectedThemeMode ?? null, customThemes), + [customThemes, detectedThemeMode, state.committedThemeSelection], ); + const committedThemeId = useMemo(() => { + const chosen = chooseThemeSelectionId(state.committedThemeSelection, detectedThemeMode ?? null); + return chosen === undefined || chosen === AUTO_THEME_ID ? committedTheme.id : chosen; + }, [committedTheme.id, detectedThemeMode, state.committedThemeSelection]); const committedIndex = themeOptions.findIndex((theme) => theme.id === committedTheme.id); const storedSelectedIndex = themeOptions.findIndex((theme) => theme.id === state.selectedThemeId); const selectedIndex = @@ -175,7 +184,7 @@ export function useThemeSelectorController({ selectedThemeIdRef.current = item.id; setState((current) => ({ ...current, - committedThemeId: item.id, + committedThemeSelection: item.id, open: false, previewThemeId: null, selectedThemeId: item.id, @@ -204,7 +213,8 @@ export function useThemeSelectorController({ return { activeTheme, baseTheme, - themeId: state.committedThemeId, + themeId: committedThemeId, + themeSelection: state.committedThemeSelection, themeSelectorItems: items, themeSelectorOpen: state.open, themeSelectorSelectedIndex: selectedIndex, diff --git a/packages/hunk/src/ui/themes.test.ts b/packages/hunk/src/ui/themes.test.ts index fc41aebe2..fb8fe19cf 100644 --- a/packages/hunk/src/ui/themes.test.ts +++ b/packages/hunk/src/ui/themes.test.ts @@ -98,6 +98,28 @@ describe("themes", () => { expect(resolveTheme("auto", "light").id).toBe(DEFAULT_LIGHT_THEME_ID); }); + test("follows an adaptive pair across terminal backgrounds", () => { + const adaptive = { dark: "vitesse-dark", light: "one-light" }; + expect(resolveTheme(adaptive, "dark").id).toBe("vitesse-dark"); + expect(resolveTheme(adaptive, "light").id).toBe("one-light"); + expect(resolveTheme(adaptive, null).id).toBe("vitesse-dark"); + expect(resolveTheme({ ...adaptive, fallback: "nord" }, null).id).toBe("nord"); + expect(resolveTheme({ dark: "graphite", light: "paper" }, "light").id).toBe( + DEFAULT_LIGHT_THEME_ID, + ); + expect(resolveTheme({ dark: "nope", light: "one-light" }, "dark").id).toBe( + DEFAULT_DARK_THEME_ID, + ); + }); + + test("resolves a custom theme named by one side of an adaptive pair", () => { + const resolved = resolveTheme({ dark: "ocean", light: "one-light" }, "dark", [ + { id: "ocean", base: "nord", label: "Ocean", accent: "#7fd1ff" }, + ]); + expect(resolved.id).toBe("ocean"); + expect(resolved.accent).toBe("#7fd1ff"); + }); + test("maps removed theme ids to compatible built-in themes", () => { expect(resolveTheme("graphite", null).id).toBe("github-dark-default"); expect(resolveTheme("paper", null).id).toBe("github-light-default"); diff --git a/packages/hunk/src/ui/themes.ts b/packages/hunk/src/ui/themes.ts index 8e17a5774..e466ac653 100644 --- a/packages/hunk/src/ui/themes.ts +++ b/packages/hunk/src/ui/themes.ts @@ -1,5 +1,6 @@ import type { ThemeMode } from "@opentui/core"; import { LEGACY_CUSTOM_THEME_ID } from "../core/theme/customThemes"; +import { chooseThemeSelectionId, type ThemeSelection } from "../core/theme/selection"; import { resolveSyntaxScopeOverrides } from "../core/theme/legacySyntaxScopes"; import type { NamedCustomThemeConfig } from "../extension-api/types"; import { blendHex, contrastRatio, hexColorDistance, relativeLuminance } from "./lib/color"; @@ -367,17 +368,18 @@ export function availableThemes(customThemes: readonly NamedCustomThemeConfig[] } /** - * Resolve a named theme, including terminal-background auto mode and custom themes. + * Resolve a theme selection, including terminal-background auto mode and custom themes. * * Custom themes are matched before bundled ids so a custom theme that reuses a * deprecated built-in alias still resolves to what the user actually defined. */ export function resolveTheme( - requested: string | undefined, + selection: ThemeSelection | undefined, themeMode: ThemeMode | null, customThemes: readonly NamedCustomThemeConfig[] = [], ) { - if (requested === "auto") { + const requested = chooseThemeSelectionId(selection, themeMode); + if (requested === undefined || requested === "auto") { return fallbackTheme(themeMode); } diff --git a/test/pty/chrome.test.ts b/test/pty/chrome.test.ts index d0b48a861..3d64a5b41 100644 --- a/test/pty/chrome.test.ts +++ b/test/pty/chrome.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test"; -import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { availableThemes } from "../../packages/hunk/src/ui/themes"; @@ -164,6 +164,60 @@ describe("PTY chrome", () => { } }); + test("an adaptive [theme] table stays committed until a theme is picked", async () => { + const configHome = mkdtempSync(join(tmpdir(), "hunk-tuistory-adaptive-theme-")); + const configPath = join(configHome, "hunk", "config.toml"); + mkdirSync(join(configHome, "hunk"), { recursive: true }); + writeFileSync( + configPath, + ["[theme]", 'dark = "vitesse-dark"', 'light = "one-light"', ""].join("\n"), + ); + const fixture = harness.createMultiHunkFilePair(); + const session = await harness.launchHunk({ + args: ["diff", "--files", fixture.before, fixture.after], + cwd: fixture.dir, + cols: 120, + rows: 24, + env: { XDG_CONFIG_HOME: configHome }, + }); + + try { + await session.waitForText(/line60/, { timeout: 15_000 }); + + // The pair resolves to one of its own ids, not a Hunk default. + await session.press("t"); + const selector = await session.waitForText(/Theme selector/, { timeout: 5_000 }); + expect(selector).toMatch(/(vitesse-dark|one-light)\s+active/); + await session.press("escape"); + await harness.waitForSnapshot(session, (text) => !text.includes("Theme selector"), 5_000); + + // Picking a theme collapses the pair, and the prompt shows that before writing. + await session.press("t"); + await session.waitForText(/Theme selector/, { timeout: 5_000 }); + await session.press("down"); + await session.press("enter"); + await harness.waitForSnapshot(session, (text) => !text.includes("Theme selector"), 5_000); + + await session.press("q"); + const prompt = await session.waitForText(/Save view preferences\?/, { timeout: 5_000 }); + expect(prompt).toContain('- theme = { dark = "vitesse-dark", light = "one-light" }'); + + await session.click(/enter\/s save/); + + const deadline = Date.now() + 5_000; + while (Date.now() < deadline && readFileSync(configPath, "utf8").includes("[theme]")) { + await sleep(50); + } + + const saved = readFileSync(configPath, "utf8"); + expect(saved).not.toContain("[theme]"); + expect(saved).toMatch(/^theme = "[a-z0-9-]+"$/m); + } finally { + session.close(); + rmSync(configHome, { recursive: true, force: true }); + } + }); + test("filter focus narrows the visible review stream in the live app", async () => { const fixture = harness.createTwoFileRepoFixture(); const session = await harness.launchHunk({ diff --git a/website/src/content/docs/docs/configure/themes.md b/website/src/content/docs/docs/configure/themes.md index fbc402114..86a3b226d 100644 --- a/website/src/content/docs/docs/configure/themes.md +++ b/website/src/content/docs/docs/configure/themes.md @@ -11,6 +11,21 @@ theme = "github-dark-default" Use `theme = "auto"` to query the terminal background at startup. Hunk chooses `github-light-default` for light terminals, `github-dark-default` for dark terminals, and falls back to dark if the terminal does not answer. +## Follow the terminal between two themes you chose + +Write `theme` as a table to name the theme for each background yourself: + +```toml +[theme] +dark = "catppuccin-mocha" +light = "catppuccin-latte" +fallback = "github-dark-default" +``` + +`dark` and `light` are required. Hunk queries the terminal background the way `auto` does, then draws the matching side. The optional `fallback` covers sessions where Hunk never gets an answer: terminals that ignore the query, and captured pager hosts such as LazyGit, where Hunk never asks. Without it those sessions use `dark`. Both sides accept built-in ids, custom theme ids, and the compatibility aliases. + +A `--theme ` flag overrides the table for one run. Picking a theme in the app replaces the pair with that single id, and the save-on-quit prompt shows the change before writing it. + ## Create a custom theme ```toml diff --git a/website/src/content/docs/docs/reference/config.md b/website/src/content/docs/docs/reference/config.md index c851864f0..06bc2610c 100644 --- a/website/src/content/docs/docs/reference/config.md +++ b/website/src/content/docs/docs/reference/config.md @@ -52,10 +52,10 @@ Select the version-control adapter explicitly. An explicit id outranks detection **`theme`** -Select the active color theme. +Select the active color theme, or one theme per terminal background. A `[theme]` table follows the terminal between its `dark` and `light` ids, using `fallback` (else `dark`) when the terminal does not report a background. -- **Type:** string -- **Accepted:** a built-in theme id or `custom` +- **Type:** string or table +- **Accepted:** a built-in theme id, `custom`, `auto`, or a `[theme]` table setting `dark` and `light` (plus an optional `fallback`) - **Built-in default:** `github-dark-default` --- From 6a24d5c151b308aef6bb5d4d545ab6dd23837453 Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Fri, 4 Sep 2026 10:49:11 -0600 Subject: [PATCH 2/9] fix(config): don't corrupt config.toml on save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The theme writer only recognized the `theme = ` and `[theme]` spellings, while the reader takes whatever Bun.TOML.parse produced. A config written as `theme.dark = "nord"` read fine, then gained a second `theme = "..."` assignment on the next preference save, after which Hunk refused to start with `BuildMessage: Cannot redefine key 'theme'` — an error naming neither the file nor the key. A quoted key inside `[theme]` failed the same way. Match every spelling TOML accepts for a key, and drop the extra lines a dotted key spreads a value over. Saves also deleted comments. The `[theme]` range ran to the next section header, so collapsing the table took the blank lines and comments that introduce whatever follows, and every rewritten key lost its trailing comment. Since one save rewrites all nine preferences, toggling `wrap_lines` stripped comments out of an untouched `[theme]` table. Comments now stay attached to the key or section they introduce, which is also why a collapsed table is written over its own header rather than appended — but only while `[theme]` is the first table, since a top-level key at any later position would scope into the table above it. Theme errors named `theme` whichever section or file held the bad table. That matters more now that a checked-in `.hunk/config.toml` can hard-fail startup for everyone in the repo, so they name both. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BLvy5LioGNhwJHzDPb7xxy --- packages/hunk/src/core/run/config.test.ts | 134 +++++++++++++++++++++ packages/hunk/src/core/run/config.ts | 139 ++++++++++++++++++---- 2 files changed, 248 insertions(+), 25 deletions(-) diff --git a/packages/hunk/src/core/run/config.test.ts b/packages/hunk/src/core/run/config.test.ts index 4aa2ce3ea..99207c6e2 100644 --- a/packages/hunk/src/core/run/config.test.ts +++ b/packages/hunk/src/core/run/config.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { getBundledVcsCatalog } from "../../app/vcsCatalog"; import type { CliInput } from "./commandInputs"; +import type { PersistedViewPreferences } from "./config"; import { diffPersistedViewPreferences, resolveConfiguredCliInput, @@ -352,6 +353,139 @@ describe("adaptive theme config", () => { expect(saved).toContain("[custom_theme]"); }); + function themePreferences(theme: PersistedViewPreferences["theme"]): PersistedViewPreferences { + return { + mode: "auto", + theme, + showLineNumbers: true, + wrapLines: false, + showHunkHeaders: true, + showMenuBar: true, + showAgentNotes: false, + copyDecorations: false, + cursorLine: "row", + }; + } + + test("replaces dotted theme keys instead of appending a second definition", () => { + const home = createTempDir("hunk-adaptive-theme-dotted-home-"); + const configPath = writeUserConfig(home, [ + "wrap_lines = false", + 'theme.dark = "vitesse-dark"', + 'theme.light = "vitesse-light"', + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { configPath }); + + const saved = readFileSync(configPath, "utf8"); + expect(() => Bun.TOML.parse(saved)).not.toThrow(); + expect(Bun.TOML.parse(saved)).toMatchObject({ theme: "dracula" }); + expect(saved).not.toContain("theme.light"); + }); + + test("replaces a quoted key inside the [theme] table rather than duplicating it", () => { + const home = createTempDir("hunk-adaptive-theme-quoted-home-"); + const configPath = writeUserConfig(home, [ + "[theme]", + '"dark" = "vitesse-dark"', + 'light = "vitesse-light"', + ]); + + saveGlobalViewPreferences(themePreferences({ dark: "nord", light: "one-light" }), { + configPath, + }); + + const saved = readFileSync(configPath, "utf8"); + expect(() => Bun.TOML.parse(saved)).not.toThrow(); + expect(Bun.TOML.parse(saved)).toMatchObject({ + theme: { dark: "nord", light: "one-light" }, + }); + }); + + test("keeps the next section's comments when the [theme] table collapses", () => { + const home = createTempDir("hunk-adaptive-theme-comments-home-"); + const configPath = writeUserConfig(home, [ + 'mode = "split"', + "", + "# theme picked per background", + "[theme]", + 'dark = "vitesse-dark" # night', + 'light = "vitesse-light"', + "", + "# my custom colors", + "[custom_theme]", + 'label = "Keep me"', + ]); + + saveGlobalViewPreferences(themePreferences({ dark: "nord", light: "one-light" }), { + configPath, + }); + + let saved = readFileSync(configPath, "utf8"); + expect(saved).toContain('dark = "nord" # night'); + expect(saved).toContain("# my custom colors"); + + saveGlobalViewPreferences(themePreferences("dracula"), { configPath }); + + saved = readFileSync(configPath, "utf8"); + expect(() => Bun.TOML.parse(saved)).not.toThrow(); + expect(saved).not.toContain("[theme]"); + // The collapsed key takes the table's place, so each comment still introduces what follows it. + expect(saved).toContain(["# theme picked per background", 'theme = "dracula"'].join("\n")); + expect(saved).toContain(["# my custom colors", "[custom_theme]"].join("\n")); + }); + + test("indents a key added to an indented [theme] table like its siblings", () => { + const home = createTempDir("hunk-adaptive-theme-indent-home-"); + const configPath = writeUserConfig(home, [ + "[theme]", + ' dark = "vitesse-dark"', + ' light = "vitesse-light"', + ]); + + saveGlobalViewPreferences( + themePreferences({ dark: "nord", light: "one-light", fallback: "dracula" }), + { configPath }, + ); + + const saved = readFileSync(configPath, "utf8"); + expect(() => Bun.TOML.parse(saved)).not.toThrow(); + expect(saved).toContain(' fallback = "dracula"'); + }); + + test("keeps a collapsed theme out of a table that precedes it", () => { + const home = createTempDir("hunk-adaptive-theme-late-table-home-"); + const configPath = writeUserConfig(home, [ + "[custom_theme]", + 'label = "Keep me"', + "", + "[theme]", + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { configPath }); + + const saved = readFileSync(configPath, "utf8"); + expect(Bun.TOML.parse(saved)).toMatchObject({ + theme: "dracula", + custom_theme: { label: "Keep me" }, + }); + }); + + test("collapsing a theme-only config leaves no leading blank line", () => { + const home = createTempDir("hunk-adaptive-theme-only-home-"); + const configPath = writeUserConfig(home, [ + "[theme]", + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { configPath }); + + expect(readFileSync(configPath, "utf8").startsWith('theme = "dracula"\n')).toBe(true); + }); + test("writes a pair as an inline table when the file has no [theme] section", () => { const home = createTempDir("hunk-adaptive-theme-inline-home-"); const configPath = writeUserConfig(home, ['theme = "dracula"', "wrap_lines = false"]); diff --git a/packages/hunk/src/core/run/config.ts b/packages/hunk/src/core/run/config.ts index 449bc46dd..249d55040 100644 --- a/packages/hunk/src/core/run/config.ts +++ b/packages/hunk/src/core/run/config.ts @@ -226,6 +226,42 @@ function serializeTomlPreferenceValue(value: string | boolean | ThemeSelection) return JSON.stringify(value); } +function tomlKeyPattern(key: string) { + const escaped = key.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`^\\s*(?:${escaped}|"${escaped}"|'${escaped}')\\s*(?:\\.|=)`); +} + +function findTrailingTomlComment(line: string) { + let inSingle = false; + let inDouble = false; + for (let index = 0; index < line.length; index += 1) { + const char = line[index]; + if (char === "\\" && inDouble) { + index += 1; + continue; + } + if (char === '"' && !inSingle) { + inDouble = !inDouble; + continue; + } + if (char === "'" && !inDouble) { + inSingle = !inSingle; + continue; + } + if (char === "#" && !inSingle && !inDouble) { + return line.slice(index).trimEnd(); + } + } + + return ""; +} + +function rewriteAssignmentLine(existing: string, assignment: string) { + const indent = existing.match(/^\s*/)?.[0] ?? ""; + const comment = findTrailingTomlComment(existing); + return `${indent}${assignment}${comment ? ` ${comment}` : ""}`; +} + /** Update one top-level TOML key while preserving sections and unrelated comments. */ function upsertTopLevelTomlValue( source: string, @@ -240,15 +276,27 @@ function upsertTopLevelTomlValue( firstTableIndex = lines.length; } - const keyPattern = new RegExp(`^\\s*${key.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\s*=`); + const keyPattern = tomlKeyPattern(key); + const matches: number[] = []; for (let index = 0; index < firstTableIndex; index += 1) { if (keyPattern.test(lines[index] ?? "")) { - lines[index] = assignment; - return `${lines.join("\n").replace(/\n*$/, "")}\n`; + matches.push(index); } } + const [first, ...duplicates] = matches; + if (first !== undefined) { + lines[first] = rewriteAssignmentLine(lines[first] ?? "", assignment); + for (const index of duplicates.reverse()) { + lines.splice(index, 1); + } + return `${lines.join("\n").replace(/\n*$/, "")}\n`; + } + let insertAt = firstTableIndex; + while (insertAt > 0 && (lines[insertAt - 1] ?? "").trim().startsWith("#")) { + insertAt -= 1; + } const hasTableSpacer = insertAt > 0 && lines[insertAt - 1] === ""; if (hasTableSpacer) { insertAt -= 1; @@ -269,7 +317,15 @@ function findThemeTableRange(lines: readonly string[]) { } const nextHeaderOffset = lines.slice(headerIndex + 1).findIndex((line) => /^\s*\[/.test(line)); - const end = nextHeaderOffset < 0 ? lines.length : headerIndex + 1 + nextHeaderOffset; + let end = nextHeaderOffset < 0 ? lines.length : headerIndex + 1 + nextHeaderOffset; + while (end > headerIndex + 1) { + const line = (lines[end - 1] ?? "").trim(); + if (line.length > 0 && !line.startsWith("#")) { + break; + } + end -= 1; + } + return { headerIndex, end }; } @@ -279,7 +335,7 @@ function applyThemeTableKey( key: string, value: string | undefined, ) { - const keyPattern = new RegExp(`^\\s*${key}\\s*=`); + const keyPattern = tomlKeyPattern(key); const existingIndex = lines .slice(range.headerIndex + 1, range.end) .findIndex((line) => keyPattern.test(line)); @@ -290,7 +346,10 @@ function applyThemeTableKey( range.end -= 1; return; } - lines[absolute] = `${key} = ${JSON.stringify(value)}`; + lines[absolute] = rewriteAssignmentLine( + lines[absolute] ?? "", + `${key} = ${JSON.stringify(value)}`, + ); return; } @@ -298,11 +357,8 @@ function applyThemeTableKey( return; } - let insertAt = range.end; - while (insertAt > range.headerIndex + 1 && (lines[insertAt - 1] ?? "").trim().length === 0) { - insertAt -= 1; - } - lines.splice(insertAt, 0, `${key} = ${JSON.stringify(value)}`); + const indent = (lines[range.end - 1] ?? "").match(/^\s*/)?.[0] ?? ""; + lines.splice(range.end, 0, `${indent}${key} = ${JSON.stringify(value)}`); range.end += 1; } @@ -324,8 +380,19 @@ function upsertThemeTomlValue(source: string, value: ThemeSelection | undefined) return `${lines.join("\n").replace(/\n*$/, "")}\n`; } + const firstTableIndex = lines.findIndex((line) => /^\s*\[/.test(line)); + if (firstTableIndex === range.headerIndex) { + lines.splice( + range.headerIndex, + range.end - range.headerIndex, + `theme = ${serializeTomlPreferenceValue(value)}`, + ); + return `${lines.join("\n").replace(/\n*$/, "")}\n`; + } + lines.splice(range.headerIndex, range.end - range.headerIndex); - return upsertTopLevelTomlValue(`${lines.join("\n").replace(/\n*$/, "")}\n`, "theme", value); + const remaining = lines.join("\n").replace(/^\n+/, "").replace(/\n*$/, ""); + return upsertTopLevelTomlValue(remaining.length > 0 ? `${remaining}\n` : "", "theme", value); } /** Accept only the layout names Hunk already supports. */ @@ -365,14 +432,14 @@ function normalizeBoolean(value: unknown) { return typeof value === "boolean" ? value : undefined; } -function normalizeThemeSelection(value: unknown) { - const read = readThemeSelection(value); +function normalizeThemeSelection(value: unknown, origin: ConfigValueOrigin = {}) { + const read = readThemeSelection(value, `${origin.section ?? ""}theme`); if (read === undefined) { return undefined; } if ("issue" in read) { - throw new Error(read.issue); + throw new Error(origin.file ? `${read.issue} (${origin.file})` : read.issue); } return read.selection; @@ -1077,8 +1144,17 @@ function resolveExtensionsConfig( }; } +interface ConfigValueOrigin { + file?: string; + section?: string; +} + /** Normalize one cataloged config value according to its runtime property. */ -function normalizeConfigReferenceValue(property: keyof CommonOptions, value: unknown) { +function normalizeConfigReferenceValue( + property: keyof CommonOptions, + value: unknown, + origin: ConfigValueOrigin = {}, +) { switch (property) { case "mode": return normalizeLayoutMode(value); @@ -1087,7 +1163,7 @@ function normalizeConfigReferenceValue(property: keyof CommonOptions, value: unk case "vcs": return normalizeVcsMode(value); case "theme": - return normalizeThemeSelection(value); + return normalizeThemeSelection(value, origin); case "tabWidth": return normalizeTabWidth(value); case "fileGap": @@ -1102,7 +1178,10 @@ function normalizeConfigReferenceValue(property: keyof CommonOptions, value: unk } /** Read the view preferences stored at one TOML object level. */ -function readConfigPreferences(source: Record): CommonOptions { +function readConfigPreferences( + source: Record, + origin: ConfigValueOrigin = {}, +): CommonOptions { const preferences: CommonOptions = {}; const mutable = preferences as Record; @@ -1113,7 +1192,7 @@ function readConfigPreferences(source: Record): CommonOptions { ]; let normalized: unknown; for (const key of runtimeKeys) { - normalized = normalizeConfigReferenceValue(option.property, source[key]); + normalized = normalizeConfigReferenceValue(option.property, source[key], origin); if (normalized !== undefined) { break; } @@ -1170,17 +1249,27 @@ function mergeOptions(base: CommonOptions, overrides: CommonOptions): CommonOpti } /** Apply one parsed config object, including command/pager sections, to the current invocation. */ -function resolveConfigLayer(source: Record, input: CliInput): CommonOptions { - let resolved = readConfigPreferences(source); +function resolveConfigLayer( + source: Record, + input: CliInput, + file?: string, +): CommonOptions { + let resolved = readConfigPreferences(source, { file }); const commandSection = CONFIG_COMMAND_SECTIONS[input.kind] ? source[input.kind] : undefined; if (isRecord(commandSection)) { - resolved = mergeOptions(resolved, readConfigPreferences(commandSection)); + resolved = mergeOptions( + resolved, + readConfigPreferences(commandSection, { file, section: `${input.kind}.` }), + ); } const pagerSection = source.pager; if (input.options.pager && isRecord(pagerSection)) { - resolved = mergeOptions(resolved, readConfigPreferences(pagerSection)); + resolved = mergeOptions( + resolved, + readConfigPreferences(pagerSection, { file, section: "pager." }), + ); } return resolved; @@ -1403,7 +1492,7 @@ export function resolveConfiguredCliInput( if (userConfigPath && sources.userConfig) { const userConfig = sources.userConfig; - const userLayer = resolveConfigLayer(userConfig, input); + const userLayer = resolveConfigLayer(userConfig, input, userConfigPath); explicitVcsId = userLayer.vcs ?? explicitVcsId; resolvedOptions = mergeOptions(resolvedOptions, userLayer); applyCustomThemeLayer(readCustomThemes(userConfig)); @@ -1413,7 +1502,7 @@ export function resolveConfiguredCliInput( if (repoConfigPath && sources.repoConfig) { const repoConfig = sources.repoConfig; - const repoLayer = resolveConfigLayer(repoConfig, input); + const repoLayer = resolveConfigLayer(repoConfig, input, repoConfigPath); explicitVcsId = repoLayer.vcs ?? explicitVcsId; resolvedOptions = mergeOptions(resolvedOptions, repoLayer); applyCustomThemeLayer(readCustomThemes(repoConfig)); From 6b5b8da35e1f2e5ab0c84678b79996432ad8d715 Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:21:35 -0600 Subject: [PATCH 3/9] fix(config): tidy the blank lines a config save leaves Saving view preferences could leave a config file slightly messier than it found it. Collapsing a [theme] table that sat between two other tables left the blank line from each side, and a file with no tables at all had new keys inserted above its trailing comment, because the backtrack that keeps a comment attached to the table it documents could not tell a real table index from the clamp used when no table exists. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NJVQRku6dLXkxiACCy177U --- packages/hunk/src/core/run/config.test.ts | 66 +++++++++++++++++++++++ packages/hunk/src/core/run/config.ts | 28 +++++++--- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/packages/hunk/src/core/run/config.test.ts b/packages/hunk/src/core/run/config.test.ts index 99207c6e2..b05c8baf7 100644 --- a/packages/hunk/src/core/run/config.test.ts +++ b/packages/hunk/src/core/run/config.test.ts @@ -131,6 +131,45 @@ describe("config persistence", () => { ); }); + test("appends after a trailing comment when the file has no table to document", () => { + const home = createTempDir("hunk-save-config-trailing-comment-home-"); + const configPath = join(home, ".config", "hunk", "config.toml"); + mkdirSync(join(home, ".config", "hunk"), { recursive: true }); + // No trailing newline, so the comment is the last line the writer sees. + writeFileSync(configPath, "# personal defaults"); + + saveGlobalViewPreferences( + { + mode: "split", + theme: "dracula", + showLineNumbers: false, + wrapLines: true, + showHunkHeaders: false, + showMenuBar: false, + showAgentNotes: true, + copyDecorations: true, + cursorLine: "row", + }, + { env: { HOME: home } }, + ); + + expect(readFileSync(configPath, "utf8")).toBe( + [ + "# personal defaults", + 'theme = "dracula"', + 'mode = "split"', + "line_numbers = false", + "wrap_lines = true", + "hunk_headers = false", + "menu_bar = false", + "agent_notes = true", + "copy_decorations = true", + 'cursor_line = "row"', + "", + ].join("\n"), + ); + }); + test("diffs view preference snapshots as the TOML assignments a save would rewrite", () => { const initial = { mode: "auto", @@ -473,6 +512,33 @@ describe("adaptive theme config", () => { }); }); + test("leaves one blank line where a collapsed [theme] table used to separate its neighbours", () => { + const home = createTempDir("hunk-adaptive-theme-seam-home-"); + const configPath = writeUserConfig(home, [ + "[custom_theme]", + 'label = "Keep me"', + "", + "[theme]", + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + "", + "[extensions]", + "enabled = true", + "", + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { configPath }); + + const saved = readFileSync(configPath, "utf8"); + expect(saved).toContain('label = "Keep me"\n\n[extensions]'); + expect(saved).not.toMatch(/\n\n\n/); + expect(Bun.TOML.parse(saved)).toMatchObject({ + theme: "dracula", + custom_theme: { label: "Keep me" }, + extensions: { enabled: true }, + }); + }); + test("collapsing a theme-only config leaves no leading blank line", () => { const home = createTempDir("hunk-adaptive-theme-only-home-"); const configPath = writeUserConfig(home, [ diff --git a/packages/hunk/src/core/run/config.ts b/packages/hunk/src/core/run/config.ts index 249d55040..029b90d95 100644 --- a/packages/hunk/src/core/run/config.ts +++ b/packages/hunk/src/core/run/config.ts @@ -271,10 +271,8 @@ function upsertTopLevelTomlValue( const lines = source.length > 0 ? source.split("\n") : []; const serialized = serializeTomlPreferenceValue(value); const assignment = `${key} = ${serialized}`; - let firstTableIndex = lines.findIndex((line) => /^\s*\[/.test(line)); - if (firstTableIndex < 0) { - firstTableIndex = lines.length; - } + const tableIndex = lines.findIndex((line) => /^\s*\[/.test(line)); + const firstTableIndex = tableIndex < 0 ? lines.length : tableIndex; const keyPattern = tomlKeyPattern(key); const matches: number[] = []; @@ -294,8 +292,10 @@ function upsertTopLevelTomlValue( } let insertAt = firstTableIndex; - while (insertAt > 0 && (lines[insertAt - 1] ?? "").trim().startsWith("#")) { - insertAt -= 1; + if (tableIndex >= 0) { + while (insertAt > 0 && (lines[insertAt - 1] ?? "").trim().startsWith("#")) { + insertAt -= 1; + } } const hasTableSpacer = insertAt > 0 && lines[insertAt - 1] === ""; if (hasTableSpacer) { @@ -329,6 +329,21 @@ function findThemeTableRange(lines: readonly string[]) { return { headerIndex, end }; } +function collapseBlankSeam(lines: string[], index: number) { + let start = index; + while (start > 0 && (lines[start - 1] ?? "").trim().length === 0) { + start -= 1; + } + + let end = index; + while (end < lines.length && (lines[end] ?? "").trim().length === 0) { + end += 1; + } + + const separators = start > 0 && end < lines.length ? [""] : []; + lines.splice(start, end - start, ...separators); +} + function applyThemeTableKey( lines: string[], range: { headerIndex: number; end: number }, @@ -391,6 +406,7 @@ function upsertThemeTomlValue(source: string, value: ThemeSelection | undefined) } lines.splice(range.headerIndex, range.end - range.headerIndex); + collapseBlankSeam(lines, range.headerIndex); const remaining = lines.join("\n").replace(/^\n+/, "").replace(/\n*$/, ""); return upsertTopLevelTomlValue(remaining.length > 0 ? `${remaining}\n` : "", "theme", value); } From 7a925e180d85486ae258b1e23bf1ed70d6464dee Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:26:35 -0600 Subject: [PATCH 4/9] fix(theme): refresh a review with the theme pair An in-session refresh rebuilt its reload input from the theme id the selection currently resolved to, so an adaptive pair arrived at the reload as whichever side the terminal happened to be on. Nothing collapses today because every refresh path keeps the mounted App, but the descriptor is the wrong thing to freeze. Carry the committed selection instead, and leave the resolved id to extension events, which want a concrete theme. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NJVQRku6dLXkxiACCy177U --- packages/hunk/src/ui/App.tsx | 2 +- .../hunk/src/ui/currentReviewRefresh.test.ts | 19 +++++++- packages/hunk/src/ui/currentReviewRefresh.ts | 5 +- ...useCurrentReviewRefreshController.test.tsx | 47 ++++++++++++++++--- .../useCurrentReviewRefreshController.ts | 2 +- 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/packages/hunk/src/ui/App.tsx b/packages/hunk/src/ui/App.tsx index 88d6b4b79..347cd7cae 100644 --- a/packages/hunk/src/ui/App.tsx +++ b/packages/hunk/src/ui/App.tsx @@ -904,7 +904,7 @@ export function App({ sourceLabel: bootstrap.changeset.sourceLabel, view: { layoutMode, - themeId, + themeSelection, showAgentNotes, showHunkHeaders, showLineNumbers, diff --git a/packages/hunk/src/ui/currentReviewRefresh.test.ts b/packages/hunk/src/ui/currentReviewRefresh.test.ts index 3bb008312..935d05dcf 100644 --- a/packages/hunk/src/ui/currentReviewRefresh.test.ts +++ b/packages/hunk/src/ui/currentReviewRefresh.test.ts @@ -8,7 +8,7 @@ import { const currentView: CurrentReviewViewOptions = { layoutMode: "split", - themeId: "nord", + themeSelection: "nord", showAgentNotes: false, showHunkHeaders: false, showLineNumbers: false, @@ -41,6 +41,23 @@ describe("current review refresh descriptor", () => { expect(input.options).toEqual({ mode: "stack", theme: "dracula", watch: true, tabWidth: 8 }); }); + test("carries an adaptive theme pair through a refresh instead of freezing one side", () => { + const input: CliInput = { + kind: "diff", + left: "before.ts", + right: "after.ts", + options: { theme: "dracula" }, + }; + const adaptive = { dark: "vitesse-dark", light: "vitesse-light" }; + + const refreshed = withCurrentReviewViewOptions(input, { + ...currentView, + themeSelection: adaptive, + }); + + expect(refreshed.options.theme).toEqual(adaptive); + }); + test("attaches the source path only to VCS inputs", () => { const fileRequest = deriveWorkspaceRefreshRequest({ input: { diff --git a/packages/hunk/src/ui/currentReviewRefresh.ts b/packages/hunk/src/ui/currentReviewRefresh.ts index 2265b0703..1acf4b198 100644 --- a/packages/hunk/src/ui/currentReviewRefresh.ts +++ b/packages/hunk/src/ui/currentReviewRefresh.ts @@ -12,12 +12,13 @@ import type { SessionReloadReason } from "../extension-api/types"; import type { CliInput, LayoutMode } from "../core/run/commandInputs"; import { canReloadInput } from "../core/run/inputReload"; +import type { ThemeSelection } from "../core/theme/selection"; import { isVcsReviewInput } from "../core/vcs"; /** Live view settings that must survive an in-session review refresh. */ export interface CurrentReviewViewOptions { layoutMode: LayoutMode; - themeId: string; + themeSelection: ThemeSelection; showAgentNotes: boolean; showHunkHeaders: boolean; showLineNumbers: boolean; @@ -53,7 +54,7 @@ export function withCurrentReviewViewOptions( options: { ...input.options, mode: view.layoutMode, - theme: view.themeId, + theme: view.themeSelection, agentNotes: view.showAgentNotes, hunkHeaders: view.showHunkHeaders, lineNumbers: view.showLineNumbers, diff --git a/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.test.tsx b/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.test.tsx index 023ac86b5..2e7d79427 100644 --- a/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.test.tsx +++ b/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.test.tsx @@ -3,6 +3,7 @@ import { testRender } from "@opentui/react/test-utils"; import { act, useState } from "react"; import { createWatchTestRuntime } from "../../../../../test/helpers/watchTest"; import type { CliInput } from "../../core/run/commandInputs"; +import type { ThemeSelection } from "../../core/theme/selection"; import type { ReloadSessionOptions, ReloadedSessionResult } from "../../session/types"; import type { WorkspaceRefreshRequest } from "../currentReviewRefresh"; import { @@ -32,12 +33,12 @@ function RefreshHarness({ input: CliInput; onController: (controller: CurrentReviewRefreshController) => void; onRegister: (request: WorkspaceRefreshRequest) => () => void; - onSetTheme?: (setTheme: (themeId: string) => void) => void; + onSetTheme?: (setTheme: (themeSelection: ThemeSelection) => void) => void; onReload: (input: CliInput, options?: ReloadSessionOptions) => Promise; onWatchReloadPending?: () => void; watchRuntime?: Parameters[0]["watchRuntime"]; }) { - const [themeId, setThemeId] = useState("dracula"); + const [themeSelection, setThemeSelection] = useState("dracula"); const controller = useCurrentReviewRefreshController({ input, onRegisterWorkspaceRefreshRequest: onRegister, @@ -47,7 +48,7 @@ function RefreshHarness({ sourceLabel: "/repo", view: { layoutMode: "stack", - themeId, + themeSelection, showAgentNotes: true, showHunkHeaders: true, showLineNumbers: true, @@ -57,11 +58,11 @@ function RefreshHarness({ watchRuntime, }); onController(controller); - onSetTheme?.(setThemeId); + onSetTheme?.(setThemeSelection); return ( - {themeId} + {JSON.stringify(themeSelection)} ); } @@ -79,7 +80,7 @@ describe("useCurrentReviewRefreshController", () => { const cleaned: WorkspaceRefreshRequest[] = []; let active: WorkspaceRefreshRequest | undefined; let controller!: CurrentReviewRefreshController; - let setTheme!: (themeId: string) => void; + let setTheme!: (themeSelection: ThemeSelection) => void; const reloads: Array<{ input: CliInput; options?: ReloadSessionOptions }> = []; const setup = await testRender( { expect(active).toBeUndefined(); }); + test("re-registers an adaptive theme pair as a pair so a refresh still follows the terminal", async () => { + const registered: WorkspaceRefreshRequest[] = []; + let setTheme!: (themeSelection: ThemeSelection) => void; + const adaptive = { dark: "vitesse-dark", light: "vitesse-light" }; + const setup = await testRender( + {}} + onRegister={(request) => { + registered.push(request); + return () => {}; + }} + onSetTheme={(value) => { + setTheme = value; + }} + onReload={async () => reloadedResult} + />, + { width: 20, height: 4 }, + ); + + try { + await act(async () => setup.renderOnce()); + await act(async () => { + setTheme(adaptive); + await setup.renderOnce(); + }); + + expect(registered).toHaveLength(2); + expect(registered[1]?.nextInput.options.theme).toEqual(adaptive); + } finally { + await act(async () => setup.renderer.destroy()); + } + }); + test("manual reload reports a rejection while the general operation preserves it", async () => { let controller!: CurrentReviewRefreshController; const failure = new Error("reload failed"); diff --git a/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.ts b/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.ts index fcead646c..38190e735 100644 --- a/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.ts +++ b/packages/hunk/src/ui/hooks/useCurrentReviewRefreshController.ts @@ -68,7 +68,7 @@ export function useCurrentReviewRefreshController({ view.showHunkHeaders, view.showLineNumbers, view.showMenuBar, - view.themeId, + view.themeSelection, view.wrapLines, ], ); From bf875cd04b7dd865524cc3fbffe66dfe34fda9bb Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:14:26 -0600 Subject: [PATCH 5/9] fix(history): carry the theme selection through history Keep the selection beside the input in the history surface, resolve it per terminal in the interactive surface, and let static output fall through to the pair's fallback side since it never probes the terminal. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6 --- .../hunk/src/app/historyBootstrap.test.ts | 108 ++++++++++++------ packages/hunk/src/app/historyBootstrap.ts | 6 +- .../hunk/src/ui/history/runStaticHistory.ts | 2 +- .../hunk/src/ui/history/staticProjection.ts | 5 +- packages/hunk/src/ui/history/types.ts | 2 + packages/hunk/src/ui/log/LogApp.tsx | 6 +- packages/hunk/src/ui/log/controller.test.ts | 2 +- packages/hunk/src/ui/log/controller.ts | 9 +- .../src/ui/session/HunkSessionHost.test.tsx | 29 +++++ 9 files changed, 120 insertions(+), 49 deletions(-) diff --git a/packages/hunk/src/app/historyBootstrap.test.ts b/packages/hunk/src/app/historyBootstrap.test.ts index 3d6c13adc..afec9a20b 100644 --- a/packages/hunk/src/app/historyBootstrap.test.ts +++ b/packages/hunk/src/app/historyBootstrap.test.ts @@ -17,6 +17,44 @@ const input: HistoryCommandInput = { extensionPaths: [], }; +/** Build one history-capable adapter whose cursor open/close counts are observable. */ +function createTestHistoryCatalog(cwd: string) { + const closeCounts: number[] = []; + let opens = 0; + const makeSource = (): VcsHistorySource => { + const index = opens++; + closeCounts[index] = 0; + return { + async read() { + return { commits: [], done: true }; + }, + async close() { + closeCounts[index]! += 1; + }, + }; + }; + const adapter: VcsAdapter = { + id: "test", + name: "Test", + detect: () => ({ id: "test", repoRoot: cwd }), + operations: {}, + history: { + async open() { + return makeSource(); + }, + async planReview(commit) { + return { kind: "revision-show", revisionId: commit.revisionId }; + }, + }, + }; + const catalog: VcsCatalog = { + adapters: [adapter], + defaultAdapterId: "test", + reservedIds: new Set(["test"]), + }; + return { catalog, closeCounts, opens: () => opens }; +} + describe("history bootstrap cursor ownership", () => { test("cancels refresh before opening and closes each active provider cursor once", async () => { const cwd = mkdtempSync(join(tmpdir(), "hunk-history-bootstrap-")); @@ -27,39 +65,7 @@ describe("history bootstrap cursor ownership", () => { configPath, 'theme = "github-dark-dimmed"\nline_numbers = false\nprompt_save_view_preferences = false\n', ); - const closeCounts: number[] = []; - let opens = 0; - const makeSource = (): VcsHistorySource => { - const index = opens++; - closeCounts[index] = 0; - return { - async read() { - return { commits: [], done: true }; - }, - async close() { - closeCounts[index]! += 1; - }, - }; - }; - const adapter: VcsAdapter = { - id: "test", - name: "Test", - detect: () => ({ id: "test", repoRoot: cwd }), - operations: {}, - history: { - async open() { - return makeSource(); - }, - async planReview(commit) { - return { kind: "revision-show", revisionId: commit.revisionId }; - }, - }, - }; - const catalog: VcsCatalog = { - adapters: [adapter], - defaultAdapterId: "test", - reservedIds: new Set(["test"]), - }; + const { catalog, closeCounts, opens } = createTestHistoryCatalog(cwd); try { const bootstrap = await loadHistoryBootstrap({ @@ -68,7 +74,8 @@ describe("history bootstrap cursor ownership", () => { env: { ...process.env, XDG_CONFIG_HOME: configHome }, baseVcsCatalog: catalog, }); - expect(bootstrap.input.theme).toBe("github-dark-dimmed"); + expect(bootstrap.input.theme).toBeUndefined(); + expect(bootstrap.themeSelection).toBe("github-dark-dimmed"); expect(bootstrap.initialViewPreferences).toMatchObject({ theme: "github-dark-dimmed", showLineNumbers: false, @@ -79,10 +86,10 @@ describe("history bootstrap cursor ownership", () => { const cancelled = new AbortController(); cancelled.abort(); await expect(bootstrap.reopenSource(cancelled.signal)).rejects.toThrow(); - expect(opens).toBe(1); + expect(opens()).toBe(1); await bootstrap.reopenSource(); - expect(opens).toBe(2); + expect(opens()).toBe(2); expect(closeCounts).toEqual([1, 0]); await bootstrap.close(); await bootstrap.close(); @@ -96,4 +103,33 @@ describe("history bootstrap cursor ownership", () => { rmSync(configHome, { recursive: true, force: true }); } }); + + test("keeps a configured [theme] pair intact for the history surface", async () => { + const cwd = mkdtempSync(join(tmpdir(), "hunk-history-bootstrap-")); + const configHome = mkdtempSync(join(tmpdir(), "hunk-history-config-")); + mkdirSync(join(configHome, "hunk"), { recursive: true }); + writeFileSync( + join(configHome, "hunk", "config.toml"), + '[theme]\ndark = "github-dark-dimmed"\nlight = "github-light-default"\n', + ); + const { catalog } = createTestHistoryCatalog(cwd); + + try { + const bootstrap = await loadHistoryBootstrap({ + input, + cwd, + env: { ...process.env, XDG_CONFIG_HOME: configHome }, + baseVcsCatalog: catalog, + }); + const pair = { dark: "github-dark-dimmed", light: "github-light-default" }; + expect(bootstrap.input.theme).toBeUndefined(); + expect(bootstrap.themeSelection).toEqual(pair); + expect(bootstrap.initialViewPreferences.theme).toEqual(pair); + await bootstrap.close(); + await bootstrap.extensionSession.shutdown(); + } finally { + rmSync(cwd, { recursive: true, force: true }); + rmSync(configHome, { recursive: true, force: true }); + } + }); }); diff --git a/packages/hunk/src/app/historyBootstrap.ts b/packages/hunk/src/app/historyBootstrap.ts index f06f11066..38ed732f8 100644 --- a/packages/hunk/src/app/historyBootstrap.ts +++ b/packages/hunk/src/app/historyBootstrap.ts @@ -4,6 +4,7 @@ import { type PersistedViewPreferences, } from "../core/run/config"; import { collectSessionCustomThemes } from "../core/theme/customThemes"; +import type { ThemeSelection } from "../core/theme/selection"; import type { ExtensionVcsHistoryCommit, ExtensionVcsHistoryReviewAction, @@ -38,6 +39,7 @@ export interface HistoryBootstrap { extensionSession: ExtensionSession; notices: readonly string[]; customThemes: readonly NamedCustomThemeConfig[]; + themeSelection?: ThemeSelection; /** Launch baseline retained so the owning history surface can persist theme changes on quit. */ initialViewPreferences: PersistedViewPreferences; viewPreferencesConfigPath?: string; @@ -133,10 +135,10 @@ export async function loadHistoryBootstrap({ throw error; } - const resolvedTheme = resolved.configured.input.options.theme; let closed = false; return { - input: resolvedTheme ? { ...input, theme: resolvedTheme } : input, + input, + themeSelection: resolved.configured.input.options.theme, source, providerId: sanitizeTerminalLine(adapter.id), providerName: sanitizeTerminalLine(adapter.name), diff --git a/packages/hunk/src/ui/history/runStaticHistory.ts b/packages/hunk/src/ui/history/runStaticHistory.ts index cdd8b56e0..977644f94 100644 --- a/packages/hunk/src/ui/history/runStaticHistory.ts +++ b/packages/hunk/src/ui/history/runStaticHistory.ts @@ -66,7 +66,7 @@ export async function runStaticHistory( const stdoutIsTTY = Boolean(deps.stdout.isTTY); const ascii = input.ascii || deps.env.TERM === "dumb"; const color = resolveHistoryColor({ mode: input.color, stdoutIsTTY, env: deps.env }); - const theme = resolveHistoryTheme(input.theme, bootstrap.customThemes); + const theme = resolveHistoryTheme(bootstrap.themeSelection, bootstrap.customThemes); const terminalColumns = deps.stdout.columns; const width = stdoutIsTTY ? terminalColumns && terminalColumns > 0 diff --git a/packages/hunk/src/ui/history/staticProjection.ts b/packages/hunk/src/ui/history/staticProjection.ts index a01d6f80e..a50a5f86b 100644 --- a/packages/hunk/src/ui/history/staticProjection.ts +++ b/packages/hunk/src/ui/history/staticProjection.ts @@ -1,4 +1,5 @@ import type { HistoryGraphRow } from "../../core/history/types"; +import type { ThemeSelection } from "../../core/theme/selection"; import type { NamedCustomThemeConfig } from "../../extension-api/types"; import { sanitizeTerminalLine, sanitizeTerminalText } from "../../lib/terminalText"; import { fitText, measureTextWidth } from "../lib/text"; @@ -14,10 +15,10 @@ export interface HistoryProjectionOptions { /** Resolve history colors from the same built-in and custom themes as review. */ export function resolveHistoryTheme( - themeId: string | undefined, + selection: ThemeSelection | undefined, customThemes: readonly NamedCustomThemeConfig[] = [], ) { - return resolveTheme(themeId, null, customThemes); + return resolveTheme(selection, null, customThemes); } /** Convert a validated #rrggbb theme color to a terminal SGR foreground. */ diff --git a/packages/hunk/src/ui/history/types.ts b/packages/hunk/src/ui/history/types.ts index 43e84810a..74ceb54df 100644 --- a/packages/hunk/src/ui/history/types.ts +++ b/packages/hunk/src/ui/history/types.ts @@ -1,5 +1,6 @@ import type { HistoryCommandInput } from "../../core/run/commandInputs"; import type { PersistedViewPreferences } from "../../core/run/config"; +import type { ThemeSelection } from "../../core/theme/selection"; import type { VcsHistorySource } from "../../core/vcs/types"; import type { ExtensionSession } from "../../extensions/session"; import type { @@ -20,6 +21,7 @@ export interface HistoryRuntime { repoRoot: string; notices: readonly string[]; customThemes: readonly NamedCustomThemeConfig[]; + themeSelection?: ThemeSelection; /** Resolved launch preferences retained while history owns the session-wide quit flow. */ initialViewPreferences: PersistedViewPreferences; viewPreferencesConfigPath?: string; diff --git a/packages/hunk/src/ui/log/LogApp.tsx b/packages/hunk/src/ui/log/LogApp.tsx index 4645c19d4..bb95cb73c 100644 --- a/packages/hunk/src/ui/log/LogApp.tsx +++ b/packages/hunk/src/ui/log/LogApp.tsx @@ -90,7 +90,7 @@ export function LogApp({ const pendingExitCode = useRef(undefined); const themeController = useThemeSelectorController({ customThemes: runtime.customThemes, - initialTheme: snapshot.themeId, + initialTheme: snapshot.theme, initialThemeMode: renderer.themeMode, onTransientNotice: setTransientNotice, onThemeCommitted: (id) => controller.setTheme(id), @@ -109,8 +109,8 @@ export function LogApp({ const responsiveLayout = resolveLogResponsiveLayout(terminal.width, terminal.height); const viewportBodyHeight = responsiveLayout.bodyHeight; const currentViewPreferences = useMemo( - () => ({ ...runtime.initialViewPreferences, theme: themeController.themeId }), - [runtime.initialViewPreferences, themeController.themeId], + () => ({ ...runtime.initialViewPreferences, theme: themeController.themeSelection }), + [runtime.initialViewPreferences, themeController.themeSelection], ); const viewPreferenceQuit = useViewPreferenceQuitController({ currentPreferences: currentViewPreferences, diff --git a/packages/hunk/src/ui/log/controller.test.ts b/packages/hunk/src/ui/log/controller.test.ts index bd6ade31f..33d74e945 100644 --- a/packages/hunk/src/ui/log/controller.test.ts +++ b/packages/hunk/src/ui/log/controller.test.ts @@ -173,7 +173,7 @@ describe("LogController", () => { controller.setTheme("github-dark"); await controller.refresh(); expect(controller.getSnapshot().rows).toHaveLength(1); - expect(controller.getSnapshot().themeId).toBe("github-dark"); + expect(controller.getSnapshot().theme).toBe("github-dark"); await controller.close(); await controller.close(); expect(closeCount()).toBe(1); diff --git a/packages/hunk/src/ui/log/controller.ts b/packages/hunk/src/ui/log/controller.ts index dbba55e90..bf5c727ec 100644 --- a/packages/hunk/src/ui/log/controller.ts +++ b/packages/hunk/src/ui/log/controller.ts @@ -1,5 +1,6 @@ import { createHistoryLaneCheckpoint, planHistoryPage } from "../../core/history/lanePlanner"; import type { HistoryGraphRow, HistoryLaneCheckpoint } from "../../core/history/types"; +import type { ThemeSelection } from "../../core/theme/selection"; import { sanitizeTerminalLine } from "../../lib/terminalText"; import type { HistoryRuntime } from "../history/types"; import { planLogViewportGeometry } from "./geometry"; @@ -21,7 +22,7 @@ export interface LogSnapshot { historyDone: boolean; loading: boolean; notice: string; - themeId?: string; + theme?: ThemeSelection; presentation: LogPresentation; } @@ -51,7 +52,7 @@ export class LogController { historyDone: false, loading: false, notice: runtime.notices[0] ?? "", - themeId: runtime.input.theme, + theme: runtime.themeSelection, presentation: { graph: false, unicode: !runtime.input.ascii && process.env.TERM !== "dumb", @@ -258,8 +259,8 @@ export class LogController { } } - setTheme(themeId: string) { - this.publish({ themeId }); + setTheme(theme: ThemeSelection) { + this.publish({ theme }); } togglePresentation(key: keyof LogPresentation) { diff --git a/packages/hunk/src/ui/session/HunkSessionHost.test.tsx b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx index 6dc9e4532..c79277ae0 100644 --- a/packages/hunk/src/ui/session/HunkSessionHost.test.tsx +++ b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx @@ -421,6 +421,35 @@ test("blocks reopening until dirty-quit cancellation settles", async () => { } }); +test("quits history without a save prompt when a configured theme pair was never changed", async () => { + const history = await createHistoryRoute(); + const pair = { dark: "github-dark-default", light: "github-light-default" }; + history.runtime.themeSelection = pair; + history.runtime.initialViewPreferences = persistedViewPreferencesFromOptions({ theme: pair }); + history.controller = new LogController(history.runtime); + await history.controller.loadMore(); + const quit = mock(() => undefined); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.typeText("q")); + await setup.renderOnce(); + expect(setup.captureCharFrame()).not.toContain("Save view preferences?"); + await settle(setup); + expect(quit).toHaveBeenCalledTimes(1); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + } +}); + test("preserves the original exit status while a saved-preferences quit is delayed", async () => { const history = await createHistoryRoute(); const configHome = mkdtempSync(join(tmpdir(), "hunk-log-delayed-quit-")); From a24b0cc85cddbb99c294acf2860df71056b19b73 Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:25:54 -0600 Subject: [PATCH 6/9] fix(config): prove a config edit before writing it The line-based writer matched `[theme]` and `theme =` by regex, so it missed quoted headers like `["theme"]` and added a second root key, mistook a `[theme]` example inside a multiline string for the real table, and dropped comments on the header and inside the table when a single id replaced a pair. It also rewrote every preference on every save, so a mode-only change could still corrupt an untouched theme. Replace the regex matching with a small line scanner that tracks multiline strings and bracketed values and compares headers and keys by parsed path, hoist a collapsed table's comments onto the key, and only rewrite keys that differ from the loaded baseline. Before replacing the file, parse the candidate and require it to equal the original document with just the edited keys changed; otherwise leave the file alone and say what to set by hand. Writes go through a sibling temp file so a crash cannot truncate the config. Saves also learn the command and pager tables that shaped the session, so a key those tables define is written back there instead of to a root key the next run would override. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6 --- packages/hunk/src/core/run/config.test.ts | 212 ++++++++- packages/hunk/src/core/run/config.ts | 363 ++++++-------- .../hunk/src/core/run/tomlSourceEdit.test.ts | 191 ++++++++ packages/hunk/src/core/run/tomlSourceEdit.ts | 448 ++++++++++++++++++ .../hooks/useViewPreferenceQuitController.ts | 19 +- 5 files changed, 1006 insertions(+), 227 deletions(-) create mode 100644 packages/hunk/src/core/run/tomlSourceEdit.test.ts create mode 100644 packages/hunk/src/core/run/tomlSourceEdit.ts diff --git a/packages/hunk/src/core/run/config.test.ts b/packages/hunk/src/core/run/config.test.ts index b05c8baf7..f173ff05e 100644 --- a/packages/hunk/src/core/run/config.test.ts +++ b/packages/hunk/src/core/run/config.test.ts @@ -211,6 +211,200 @@ describe("adaptive theme config", () => { return configPath; } + test('leaves an unchanged ["theme"] table byte-for-byte when only another key changes', () => { + const home = createTempDir("hunk-quoted-theme-home-"); + const configPath = writeUserConfig(home, [ + 'mode = "auto"', + "", + '["theme"]', + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + "", + ]); + const pair = { dark: "vitesse-dark", light: "vitesse-light" }; + const baseline = themePreferences(pair); + + saveGlobalViewPreferences({ ...baseline, mode: "split" }, { configPath, baseline }); + + expect(readFileSync(configPath, "utf8")).toBe( + [ + 'mode = "split"', + "", + '["theme"]', + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + "", + ].join("\n"), + ); + }); + + test('collapses a quoted ["theme"] header instead of adding a second root theme', () => { + const home = createTempDir("hunk-quoted-theme-collapse-home-"); + const configPath = writeUserConfig(home, [ + '["theme"]', + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { + configPath, + baseline: themePreferences({ dark: "vitesse-dark", light: "vitesse-light" }), + }); + + const saved = readFileSync(configPath, "utf8"); + expect(saved).toBe('theme = "dracula"\n'); + expect(Bun.TOML.parse(saved)).toEqual({ theme: "dracula" }); + }); + + test("ignores a [theme] example inside a multiline extension string", () => { + const home = createTempDir("hunk-multiline-theme-home-"); + const configPath = writeUserConfig(home, [ + "[extension.notes]", + 'template = """', + "[theme]", + 'dark = "example"', + 'light = "example"', + '"""', + "", + "[theme]", + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + "", + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { + configPath, + baseline: themePreferences({ dark: "vitesse-dark", light: "vitesse-light" }), + }); + + const saved = readFileSync(configPath, "utf8"); + expect(saved).toBe( + [ + 'theme = "dracula"', + "", + "[extension.notes]", + 'template = """', + "[theme]", + 'dark = "example"', + 'light = "example"', + '"""', + "", + ].join("\n"), + ); + expect(Bun.TOML.parse(saved)).toMatchObject({ theme: "dracula" }); + }); + + test("writes a theme picked under a [diff.theme] override back into the [diff] table", () => { + const home = createTempDir("hunk-scoped-theme-home-"); + const configPath = writeUserConfig(home, [ + 'theme = "github-dark-default"', + "", + "[diff]", + 'mode = "stack"', + "", + "[diff.theme]", + 'dark = "vitesse-dark"', + 'light = "vitesse-light"', + "", + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { + configPath, + baseline: themePreferences({ dark: "vitesse-dark", light: "vitesse-light" }), + scope: { command: "diff" }, + }); + + const saved = readFileSync(configPath, "utf8"); + expect(saved).toBe( + [ + 'theme = "github-dark-default"', + "", + "[diff]", + 'mode = "stack"', + 'theme = "dracula"', + "", + ].join("\n"), + ); + const resolved = resolveConfiguredCliInput( + { kind: "diff", left: "a", right: "b", options: {} }, + { cwd: home, env: { HOME: home } }, + ); + expect(resolved.input.options.theme).toBe("dracula"); + expect(resolved.viewPreferenceScope).toEqual({ command: "diff" }); + }); + + test("prefers the [pager] table over the command table when both define the key", () => { + const home = createTempDir("hunk-pager-theme-home-"); + const configPath = writeUserConfig(home, [ + "[patch]", + 'theme = "one-light"', + "", + "[pager]", + 'theme = "vitesse-dark" # in less', + "", + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { + configPath, + baseline: themePreferences("vitesse-dark"), + scope: { command: "patch", pager: true }, + }); + + expect(readFileSync(configPath, "utf8")).toBe( + ["[patch]", 'theme = "one-light"', "", "[pager]", 'theme = "dracula" # in less', ""].join( + "\n", + ), + ); + }); + + test("keeps root keys at the root when the command table does not define them", () => { + const home = createTempDir("hunk-scoped-other-key-home-"); + const configPath = writeUserConfig(home, [ + 'theme = "one-light"', + "", + "[diff]", + 'mode = "stack"', + "", + ]); + + saveGlobalViewPreferences(themePreferences("dracula"), { + configPath, + baseline: themePreferences("one-light"), + scope: { command: "diff" }, + }); + + expect(readFileSync(configPath, "utf8")).toBe( + ['theme = "dracula"', "", "[diff]", 'mode = "stack"', ""].join("\n"), + ); + }); + + test("refuses to save when the rewritten file would not parse back to the same document", () => { + const home = createTempDir("hunk-unsafe-save-home-"); + // An array of tables named `theme` is not something the writer can turn into a plain key. + const original = ["[[theme]]", 'dark = "vitesse-dark"', 'light = "vitesse-light"', ""].join( + "\n", + ); + const configPath = writeUserConfig(home, [original]); + + expect(() => + saveGlobalViewPreferences(themePreferences("dracula"), { + configPath, + baseline: themePreferences("one-light"), + }), + ).toThrow(/Could not update theme in .* without changing the rest of the file/); + expect(readFileSync(configPath, "utf8")).toBe(original); + }); + + test("refuses to save into a config file that no longer parses", () => { + const home = createTempDir("hunk-broken-config-home-"); + const configPath = writeUserConfig(home, ["mode = ", ""]); + + expect(() => saveGlobalViewPreferences(themePreferences("dracula"), { configPath })).toThrow( + /because it is not valid TOML/, + ); + expect(readFileSync(configPath, "utf8")).toBe("mode = \n"); + }); + test("reads a [theme] table into an adaptive selection", () => { const home = createTempDir("hunk-adaptive-theme-home-"); const repo = createTempDir("hunk-adaptive-theme-repo-"); @@ -469,9 +663,21 @@ describe("adaptive theme config", () => { saved = readFileSync(configPath, "utf8"); expect(() => Bun.TOML.parse(saved)).not.toThrow(); expect(saved).not.toContain("[theme]"); - // The collapsed key takes the table's place, so each comment still introduces what follows it. - expect(saved).toContain(["# theme picked per background", 'theme = "dracula"'].join("\n")); - expect(saved).toContain(["# my custom colors", "[custom_theme]"].join("\n")); + // The collapsed key takes the table's place, so each comment still introduces what follows + // it, and a comment that rode on a removed value line survives as a line of its own. + expect(saved).toContain( + [ + "", + "# theme picked per background", + "# night", + 'theme = "dracula"', + "", + "# my custom colors", + "[custom_theme]", + 'label = "Keep me"', + "", + ].join("\n"), + ); }); test("indents a key added to an indented [theme] table like its siblings", () => { diff --git a/packages/hunk/src/core/run/config.ts b/packages/hunk/src/core/run/config.ts index 029b90d95..c4d6a534b 100644 --- a/packages/hunk/src/core/run/config.ts +++ b/packages/hunk/src/core/run/config.ts @@ -23,7 +23,14 @@ import { themeSelectionsEqual, type ThemeSelection, } from "../theme/selection"; +import { HunkUserError } from "./errors"; import { resolveGlobalConfigPath } from "./paths"; +import { + serializeTomlValue, + tomlTableDefinesKey, + upsertTomlValue, + type TomlWritableValue, +} from "./tomlSourceEdit"; import { LEGACY_CUSTOM_SYNTAX_NOTICES, type StartupNotice } from "../process/startupNotice"; import { DEFAULT_FILE_GAP, @@ -127,11 +134,18 @@ const VIEW_PREFERENCES_PROMPT_CONFIG_KEY = "prompt_save_view_preferences"; type PersistedPreferenceValue = string | boolean | ThemeSelection | undefined; +/** Express one preference value in the shape the TOML writer accepts. */ +function toWritablePreferenceValue( + value: Exclude, +): TomlWritableValue { + if (typeof value === "boolean" || typeof value === "string") return value; + return Object.fromEntries(ADAPTIVE_THEME_SELECTION_KEYS.map((key) => [key, value[key]])); +} + const PERSISTED_VIEW_PREFERENCE_KEYS: Array<{ configKey: string; value: (preferences: PersistedViewPreferences) => PersistedPreferenceValue; equals?: (previous: PersistedPreferenceValue, next: PersistedPreferenceValue) => boolean; - upsert?: (source: string, value: PersistedPreferenceValue) => string; }> = [ { configKey: "theme", @@ -141,7 +155,6 @@ const PERSISTED_VIEW_PREFERENCE_KEYS: Array<{ previous as ThemeSelection | undefined, next as ThemeSelection | undefined, ), - upsert: (source, value) => upsertThemeTomlValue(source, value as ThemeSelection), }, { configKey: "mode", value: (preferences) => preferences.mode }, { configKey: "line_numbers", value: (preferences) => preferences.showLineNumbers }, @@ -176,6 +189,11 @@ export interface ExtensionBootstrapConfigOptions extends ConfigResolutionOptions const CONFIG_FALLBACK_VCS_ID = "git"; const EMPTY_CONFIG_VCS_CATALOG = createVcsCatalog([], CONFIG_FALLBACK_VCS_ID, []); +export interface ViewPreferenceScope { + command?: keyof typeof CONFIG_COMMAND_SECTIONS; + pager?: boolean; +} + export interface HunkConfigResolution { input: CliInput; /** Config-defined custom themes in declaration order, user layer before repo layer. */ @@ -204,213 +222,13 @@ export interface HunkConfigResolution { projectRoot?: string; repoConfigPath?: string; viewPreferencesConfigPath?: string; + viewPreferenceScope?: ViewPreferenceScope; } function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } -/** Serialize one primitive or inline-table TOML preference value. */ -function serializeTomlPreferenceValue(value: string | boolean | ThemeSelection) { - if (typeof value === "boolean") { - return value ? "true" : "false"; - } - - if (isAdaptiveThemeSelection(value)) { - const entries = ADAPTIVE_THEME_SELECTION_KEYS.filter((key) => value[key] !== undefined).map( - (key) => `${key} = ${JSON.stringify(value[key])}`, - ); - return `{ ${entries.join(", ")} }`; - } - - return JSON.stringify(value); -} - -function tomlKeyPattern(key: string) { - const escaped = key.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - return new RegExp(`^\\s*(?:${escaped}|"${escaped}"|'${escaped}')\\s*(?:\\.|=)`); -} - -function findTrailingTomlComment(line: string) { - let inSingle = false; - let inDouble = false; - for (let index = 0; index < line.length; index += 1) { - const char = line[index]; - if (char === "\\" && inDouble) { - index += 1; - continue; - } - if (char === '"' && !inSingle) { - inDouble = !inDouble; - continue; - } - if (char === "'" && !inDouble) { - inSingle = !inSingle; - continue; - } - if (char === "#" && !inSingle && !inDouble) { - return line.slice(index).trimEnd(); - } - } - - return ""; -} - -function rewriteAssignmentLine(existing: string, assignment: string) { - const indent = existing.match(/^\s*/)?.[0] ?? ""; - const comment = findTrailingTomlComment(existing); - return `${indent}${assignment}${comment ? ` ${comment}` : ""}`; -} - -/** Update one top-level TOML key while preserving sections and unrelated comments. */ -function upsertTopLevelTomlValue( - source: string, - key: string, - value: string | boolean | ThemeSelection, -) { - const lines = source.length > 0 ? source.split("\n") : []; - const serialized = serializeTomlPreferenceValue(value); - const assignment = `${key} = ${serialized}`; - const tableIndex = lines.findIndex((line) => /^\s*\[/.test(line)); - const firstTableIndex = tableIndex < 0 ? lines.length : tableIndex; - - const keyPattern = tomlKeyPattern(key); - const matches: number[] = []; - for (let index = 0; index < firstTableIndex; index += 1) { - if (keyPattern.test(lines[index] ?? "")) { - matches.push(index); - } - } - - const [first, ...duplicates] = matches; - if (first !== undefined) { - lines[first] = rewriteAssignmentLine(lines[first] ?? "", assignment); - for (const index of duplicates.reverse()) { - lines.splice(index, 1); - } - return `${lines.join("\n").replace(/\n*$/, "")}\n`; - } - - let insertAt = firstTableIndex; - if (tableIndex >= 0) { - while (insertAt > 0 && (lines[insertAt - 1] ?? "").trim().startsWith("#")) { - insertAt -= 1; - } - } - const hasTableSpacer = insertAt > 0 && lines[insertAt - 1] === ""; - if (hasTableSpacer) { - insertAt -= 1; - } - lines.splice( - insertAt, - 0, - assignment, - ...(hasTableSpacer || insertAt === lines.length ? [] : [""]), - ); - return `${lines.join("\n").replace(/\n*$/, "")}\n`; -} - -function findThemeTableRange(lines: readonly string[]) { - const headerIndex = lines.findIndex((line) => /^\s*\[\s*theme\s*\]\s*(?:#.*)?$/.test(line)); - if (headerIndex < 0) { - return null; - } - - const nextHeaderOffset = lines.slice(headerIndex + 1).findIndex((line) => /^\s*\[/.test(line)); - let end = nextHeaderOffset < 0 ? lines.length : headerIndex + 1 + nextHeaderOffset; - while (end > headerIndex + 1) { - const line = (lines[end - 1] ?? "").trim(); - if (line.length > 0 && !line.startsWith("#")) { - break; - } - end -= 1; - } - - return { headerIndex, end }; -} - -function collapseBlankSeam(lines: string[], index: number) { - let start = index; - while (start > 0 && (lines[start - 1] ?? "").trim().length === 0) { - start -= 1; - } - - let end = index; - while (end < lines.length && (lines[end] ?? "").trim().length === 0) { - end += 1; - } - - const separators = start > 0 && end < lines.length ? [""] : []; - lines.splice(start, end - start, ...separators); -} - -function applyThemeTableKey( - lines: string[], - range: { headerIndex: number; end: number }, - key: string, - value: string | undefined, -) { - const keyPattern = tomlKeyPattern(key); - const existingIndex = lines - .slice(range.headerIndex + 1, range.end) - .findIndex((line) => keyPattern.test(line)); - if (existingIndex >= 0) { - const absolute = range.headerIndex + 1 + existingIndex; - if (value === undefined) { - lines.splice(absolute, 1); - range.end -= 1; - return; - } - lines[absolute] = rewriteAssignmentLine( - lines[absolute] ?? "", - `${key} = ${JSON.stringify(value)}`, - ); - return; - } - - if (value === undefined) { - return; - } - - const indent = (lines[range.end - 1] ?? "").match(/^\s*/)?.[0] ?? ""; - lines.splice(range.end, 0, `${indent}${key} = ${JSON.stringify(value)}`); - range.end += 1; -} - -function upsertThemeTomlValue(source: string, value: ThemeSelection | undefined) { - if (value === undefined) { - return source; - } - - const lines = source.length > 0 ? source.split("\n") : []; - const range = findThemeTableRange(lines); - if (!range) { - return upsertTopLevelTomlValue(source, "theme", value); - } - - if (isAdaptiveThemeSelection(value)) { - for (const key of ADAPTIVE_THEME_SELECTION_KEYS) { - applyThemeTableKey(lines, range, key, value[key]); - } - return `${lines.join("\n").replace(/\n*$/, "")}\n`; - } - - const firstTableIndex = lines.findIndex((line) => /^\s*\[/.test(line)); - if (firstTableIndex === range.headerIndex) { - lines.splice( - range.headerIndex, - range.end - range.headerIndex, - `theme = ${serializeTomlPreferenceValue(value)}`, - ); - return `${lines.join("\n").replace(/\n*$/, "")}\n`; - } - - lines.splice(range.headerIndex, range.end - range.headerIndex); - collapseBlankSeam(lines, range.headerIndex); - const remaining = lines.join("\n").replace(/^\n+/, "").replace(/\n*$/, ""); - return upsertTopLevelTomlValue(remaining.length > 0 ? `${remaining}\n` : "", "theme", value); -} - /** Accept only the layout names Hunk already supports. */ function normalizeLayoutMode(value: unknown): LayoutMode | undefined { return value === "auto" || value === "split" || value === "stack" ? value : undefined; @@ -1325,10 +1143,97 @@ function resolveWritableConfigPath(configuredPath: string | undefined, env: Node return configPath; } -/** Write an updated config source after ensuring the parent directory exists. */ +/** Write an updated config source through a sibling temp file so a crash never leaves it half-written. */ function writeConfigSource(configPath: string, source: string) { fs.mkdirSync(dirname(configPath), { recursive: true }); - fs.writeFileSync(configPath, source); + const stagingPath = `${configPath}.${process.pid}.tmp`; + try { + fs.writeFileSync(stagingPath, source); + fs.renameSync(stagingPath, configPath); + } finally { + fs.rmSync(stagingPath, { force: true }); + } +} + +interface ConfigEdit { + configKey: string; + value: TomlWritableValue; +} + +function chooseConfigEditTable( + source: string, + scope: ViewPreferenceScope | undefined, + key: string, +) { + const candidates: string[][] = [ + ...(scope?.pager ? [["pager"]] : []), + ...(scope?.command ? [[scope.command]] : []), + ]; + return candidates.find((table) => tomlTableDefinesKey(source, table, key)) ?? []; +} + +function expectedParsedValue(value: TomlWritableValue): unknown { + if (typeof value !== "object") return value; + return Object.fromEntries(Object.entries(value).filter(([, entry]) => entry !== undefined)); +} + +/** Set one nested key on a parsed TOML object, creating intermediate tables. */ +function setParsedValue(target: Record, path: readonly string[], value: unknown) { + let cursor = target; + for (const segment of path.slice(0, -1)) { + const next = cursor[segment]; + if (!isRecord(next)) { + cursor[segment] = {}; + } + cursor = cursor[segment] as Record; + } + cursor[path[path.length - 1]!] = value; +} + +/** Apply config edits to the file at `configPath` and write the result only after proving it. */ +function writeConfigEdits( + configPath: string, + edits: readonly ConfigEdit[], + scope: ViewPreferenceScope | undefined, +) { + const source = readConfigSource(configPath); + let expected: Record; + try { + const parsed = source.length > 0 ? Bun.TOML.parse(source) : {}; + if (!isRecord(parsed)) throw new Error("Expected a TOML table."); + expected = structuredClone(parsed); + } catch (error) { + throw new HunkUserError(`Could not update ${configPath} because it is not valid TOML.`, [ + error instanceof Error ? error.message : String(error), + ]); + } + + const unsafe = (keyPath: string) => + new HunkUserError( + `Could not update ${keyPath} in ${configPath} without changing the rest of the file, so it was left as it was.`, + [`Set ${keyPath} in ${configPath} by hand.`], + ); + let next = source; + for (const edit of edits) { + const table = chooseConfigEditTable(next, scope, edit.configKey); + const keyPath = [...table, edit.configKey].join("."); + const updated = upsertTomlValue(next, table, edit.configKey, edit.value); + if (updated === null) throw unsafe(keyPath); + next = updated; + setParsedValue(expected, [...table, edit.configKey], expectedParsedValue(edit.value)); + } + if (next === source) return; + + let reparsed: unknown; + try { + reparsed = Bun.TOML.parse(next); + } catch { + throw unsafe(edits.map((edit) => edit.configKey).join(", ")); + } + if (!Bun.deepEquals(reparsed, expected, true)) { + throw unsafe(edits.map((edit) => edit.configKey).join(", ")); + } + writeConfigSource(configPath, next); } /** One view preference the quit prompt would rewrite, as TOML assignment text. */ @@ -1361,36 +1266,48 @@ export function diffPersistedViewPreferences( changes.push({ configKey: key.configKey, previousValue: - previousValue === undefined ? "unset" : serializeTomlPreferenceValue(previousValue), - nextValue: nextValue === undefined ? "unset" : serializeTomlPreferenceValue(nextValue), + previousValue === undefined + ? "unset" + : serializeTomlValue(toWritablePreferenceValue(previousValue)), + nextValue: + nextValue === undefined + ? "unset" + : serializeTomlValue(toWritablePreferenceValue(nextValue)), }); } return changes; } +export interface SaveViewPreferencesOptions extends Pick { + configPath?: string; + baseline?: PersistedViewPreferences; + scope?: ViewPreferenceScope; +} + /** Persist accepted in-app view preferences to the selected Hunk config file. */ export function saveGlobalViewPreferences( preferences: PersistedViewPreferences, { configPath: configuredPath, env = process.env, - }: Pick & { configPath?: string } = {}, + baseline, + scope, + }: SaveViewPreferencesOptions = {}, ) { const configPath = resolveWritableConfigPath(configuredPath, env); - let nextSource = readConfigSource(configPath); + const edits: ConfigEdit[] = []; for (const key of PERSISTED_VIEW_PREFERENCE_KEYS) { const value = key.value(preferences); - if (value === undefined) { - continue; + if (value === undefined) continue; + if (baseline) { + const previous = key.value(baseline); + if (key.equals ? key.equals(previous, value) : previous === value) continue; } - - nextSource = key.upsert - ? key.upsert(nextSource, value) - : upsertTopLevelTomlValue(nextSource, key.configKey, value); + edits.push({ configKey: key.configKey, value: toWritablePreferenceValue(value) }); } - writeConfigSource(configPath, nextSource); + writeConfigEdits(configPath, edits, scope); return configPath; } @@ -1403,13 +1320,11 @@ export function saveViewPreferencesPromptPreference( }: Pick & { configPath?: string } = {}, ) { const configPath = resolveWritableConfigPath(configuredPath, env); - const nextSource = upsertTopLevelTomlValue( - readConfigSource(configPath), - VIEW_PREFERENCES_PROMPT_CONFIG_KEY, - promptSaveViewPreferences, + writeConfigEdits( + configPath, + [{ configKey: VIEW_PREFERENCES_PROMPT_CONFIG_KEY, value: promptSaveViewPreferences }], + undefined, ); - - writeConfigSource(configPath, nextSource); return configPath; } @@ -1599,5 +1514,9 @@ export function resolveConfiguredCliInput( // choices user-scoped so Hunk does not create project policy files from an interactive prompt. viewPreferencesConfigPath: repoConfigPath && fs.existsSync(repoConfigPath) ? repoConfigPath : userConfigPath, + viewPreferenceScope: { + ...(CONFIG_COMMAND_SECTIONS[input.kind] ? { command: input.kind } : {}), + ...(resolvedOptions.pager ? { pager: true } : {}), + }, }; } diff --git a/packages/hunk/src/core/run/tomlSourceEdit.test.ts b/packages/hunk/src/core/run/tomlSourceEdit.test.ts new file mode 100644 index 000000000..36f94c36c --- /dev/null +++ b/packages/hunk/src/core/run/tomlSourceEdit.test.ts @@ -0,0 +1,191 @@ +import { describe, expect, test } from "bun:test"; +import { scanTomlSource, tomlTableDefinesKey, upsertTomlValue } from "./tomlSourceEdit"; + +const lines = (...text: string[]) => `${text.join("\n")}\n`; + +/** Parse a source and assert it round-trips through Bun's TOML parser. */ +function parsed(source: string | null) { + expect(source).not.toBeNull(); + return Bun.TOML.parse(source!) as Record; +} + +describe("scanTomlSource", () => { + test("reads headers by parsed path whatever quoting they use", () => { + const scanned = scanTomlSource( + ['["theme"]', "[ theme ]", "[diff.'theme']", "[[items]]"].join("\n"), + ); + expect( + scanned.map((line) => (line.kind === "header" ? [line.path, line.arrayOfTables] : line.kind)), + ).toEqual([ + [["theme"], false], + [["theme"], false], + [["diff", "theme"], false], + [["items"], true], + ]); + }); + + test("keeps table-like text inside multiline strings out of the structure", () => { + const scanned = scanTomlSource( + [ + "[extension.docs]", + 'readme = """', + "[theme]", + 'dark = "x"', + '"""', + "paths = [", + ' "[theme]",', + "]", + "next = 'it''s' # done", + ].join("\n"), + ); + expect(scanned.map((line) => line.kind)).toEqual([ + "header", + "assignment", + "continuation", + "continuation", + "continuation", + "assignment", + "continuation", + "continuation", + "assignment", + ]); + const last = scanned.at(-1); + expect(last?.kind === "assignment" && last.comment).toBe("# done"); + }); + + test("marks lines it cannot classify instead of guessing", () => { + expect(scanTomlSource("[theme\n= 3").map((line) => line.kind)).toEqual(["unknown", "unknown"]); + }); +}); + +describe("upsertTomlValue", () => { + test('rewrites a quoted ["theme"] header\'s keys in place', () => { + const source = lines('mode = "split"', "", '["theme"]', 'dark = "a"', 'light = "b"'); + const next = upsertTomlValue(source, [], "theme", { dark: "c", light: "d", fallback: "e" }); + expect(next).toBe( + lines('mode = "split"', "", '["theme"]', 'dark = "c"', 'light = "d"', 'fallback = "e"'), + ); + expect(parsed(next)).toEqual({ + mode: "split", + theme: { dark: "c", light: "d", fallback: "e" }, + }); + }); + + test("collapses a quoted header to a root key instead of adding a second definition", () => { + const source = lines('["theme"]', 'dark = "a"', 'light = "b"'); + const next = upsertTomlValue(source, [], "theme", "solo"); + expect(next).toBe(lines('theme = "solo"')); + }); + + test("leaves a [theme] example inside a multiline string alone", () => { + const source = lines( + "[extension.docs]", + 'readme = """', + "[theme]", + 'dark = "x"', + '"""', + "", + "[theme]", + 'dark = "a"', + 'light = "b"', + ); + const next = upsertTomlValue(source, [], "theme", "solo"); + expect(next).toBe( + lines( + 'theme = "solo"', + "", + "[extension.docs]", + 'readme = """', + "[theme]", + 'dark = "x"', + '"""', + ), + ); + const document = parsed(next); + expect(document).toMatchObject({ theme: "solo" }); + expect(document).toHaveProperty( + ["extension", "docs", "readme"], + expect.stringContaining("[theme]"), + ); + }); + + test("moves every comment of a collapsed table onto the key", () => { + const source = lines( + 'mode = "split"', + "", + "# picked per background", + "[theme] # both sides", + "# the night side", + 'dark = "a" # dim', + 'light = "b"', + "", + "# colours", + "[custom_theme]", + 'label = "keep"', + ); + const next = upsertTomlValue(source, [], "theme", "solo"); + expect(next).toBe( + lines( + 'mode = "split"', + "", + "# picked per background", + "# the night side", + "# dim", + 'theme = "solo" # both sides', + "", + "# colours", + "[custom_theme]", + 'label = "keep"', + ), + ); + }); + + test("writes into a command table and collapses its [diff.theme] sub-table there", () => { + const source = lines( + 'theme = "root"', + "", + "[diff]", + 'mode = "stack"', + "", + "[diff.theme]", + 'dark = "a"', + 'light = "b"', + ); + expect(tomlTableDefinesKey(source, ["diff"], "theme")).toBe(true); + expect(tomlTableDefinesKey(source, ["diff"], "line_numbers")).toBe(false); + const next = upsertTomlValue(source, ["diff"], "theme", "solo"); + expect(next).toBe(lines('theme = "root"', "", "[diff]", 'mode = "stack"', 'theme = "solo"')); + expect(parsed(next)).toEqual({ theme: "root", diff: { mode: "stack", theme: "solo" } }); + }); + + test("rewrites a scoped assignment in place, keeping its comment", () => { + const source = lines("[pager]", 'theme = "a" # for less', 'mode = "stack"'); + const next = upsertTomlValue(source, ["pager"], "theme", { dark: "c", light: "d" }); + expect(next).toBe( + lines("[pager]", 'theme = { dark = "c", light = "d" } # for less', 'mode = "stack"'), + ); + }); + + test("folds dotted and duplicate definitions into the first one", () => { + const source = lines('theme.dark = "a" # night', "wrap_lines = true", 'theme.light = "b"'); + const next = upsertTomlValue(source, [], "theme", "solo"); + expect(next).toBe(lines('theme = "solo" # night', "wrap_lines = true")); + }); + + test("adds a root key above the first header and keeps that header's comment attached", () => { + const source = lines("# personal", "", "# my colours", "[custom_theme]", 'label = "keep"'); + const next = upsertTomlValue(source, [], "mode", "split"); + expect(next).toBe( + lines("# personal", 'mode = "split"', "", "# my colours", "[custom_theme]", 'label = "keep"'), + ); + }); + + test("starts an empty file with the key alone", () => { + expect(upsertTomlValue("", [], "mode", "split")).toBe(lines('mode = "split"')); + }); + + test("refuses scopes that do not exist and keys held in arrays of tables", () => { + expect(upsertTomlValue(lines('mode = "split"'), ["diff"], "theme", "solo")).toBeNull(); + expect(upsertTomlValue(lines("[[theme]]", 'dark = "a"'), [], "theme", "solo")).toBeNull(); + }); +}); diff --git a/packages/hunk/src/core/run/tomlSourceEdit.ts b/packages/hunk/src/core/run/tomlSourceEdit.ts new file mode 100644 index 000000000..08fc6575e --- /dev/null +++ b/packages/hunk/src/core/run/tomlSourceEdit.ts @@ -0,0 +1,448 @@ +/** + * Rewrites one key of a TOML config source while leaving every other byte alone. + * + * Hunk persists a handful of view preferences into a file the user also edits by hand, so this + * writer edits lines instead of re-serializing: comments, ordering, indentation, and unrelated + * tables survive a save. A line scanner tracks multiline strings and bracketed values so a table + * header or `key = value` inside a string is never mistaken for structure, and headers and keys + * compare by parsed path, so `["theme"]`, `[ theme ]`, and `[theme]` name the same table. + * + * The writer is deliberately narrow: it handles the string, boolean, and flat inline-table values + * Hunk persists, and one key per call. It never touches the filesystem, and callers verify each + * result by parsing it (see `config.ts`) rather than trusting the edit. + */ + +/** A value Hunk knows how to write: a primitive, or a flat table whose `undefined` keys are removed. */ +export type TomlWritableValue = string | boolean | Readonly>; + +interface ValueScanState { + /** Delimiter of the multiline string the scanner is inside, if any. */ + multiline: '"""' | "'''" | null; + /** Unclosed `[`/`{` depth, so a multi-line array continues onto later lines. */ + depth: number; +} + +export type ScannedTomlLine = + | { kind: "blank" | "comment" | "continuation" | "unknown"; text: string } + | { + kind: "header"; + text: string; + path: readonly string[]; + arrayOfTables: boolean; + comment: string; + } + | { + kind: "assignment"; + text: string; + /** Key path relative to the enclosing table, so `theme.dark = …` is `["theme", "dark"]`. */ + path: readonly string[]; + comment: string; + indent: string; + }; + +const BARE_KEY_CHARS = /[A-Za-z0-9_-]/; + +/** Parse a bare, quoted, or dotted key path starting at `start`; returns null when none is there. */ +function parseKeyPath(text: string, start: number): { path: string[]; end: number } | null { + const path: string[] = []; + let index = start; + for (;;) { + while (text[index] === " " || text[index] === "\t") index += 1; + const char = text[index]; + if (char === '"' || char === "'") { + const close = text.indexOf(char, index + 1); + if (close < 0) return null; + path.push(text.slice(index + 1, close)); + index = close + 1; + } else { + let end = index; + while (end < text.length && BARE_KEY_CHARS.test(text[end] ?? "")) end += 1; + if (end === index) return null; + path.push(text.slice(index, end)); + index = end; + } + while (text[index] === " " || text[index] === "\t") index += 1; + if (text[index] !== ".") break; + index += 1; + } + return { path, end: index }; +} + +/** Walk a value from `start`, tracking strings and brackets, and report the trailing comment. */ +function scanValue( + text: string, + start: number, + state: ValueScanState, +): { comment: string; state: ValueScanState } { + let { multiline, depth } = state; + let index = start; + while (index < text.length) { + const char = text[index] ?? ""; + if (multiline) { + if (multiline === '"""' && char === "\\") { + index += 2; + continue; + } + if (text.startsWith(multiline, index)) { + // TOML lets a multiline string end in up to two extra quotes that belong to its content. + let close = index + 3; + while (close - index < 5 && text[close] === multiline[0]) close += 1; + index = close; + multiline = null; + continue; + } + index += 1; + continue; + } + if (text.startsWith('"""', index) || text.startsWith("'''", index)) { + multiline = text.slice(index, index + 3) as '"""' | "'''"; + index += 3; + continue; + } + if (char === '"') { + index += 1; + while (index < text.length && text[index] !== '"') { + if (text[index] === "\\") index += 1; + index += 1; + } + index += 1; + continue; + } + if (char === "'") { + const close = text.indexOf("'", index + 1); + index = close < 0 ? text.length : close + 1; + continue; + } + if (char === "[" || char === "{") depth += 1; + else if (char === "]" || char === "}") depth = Math.max(0, depth - 1); + else if (char === "#") + return { comment: text.slice(index).trimEnd(), state: { multiline, depth } }; + index += 1; + } + return { comment: "", state: { multiline, depth } }; +} + +/** Classify every line of a TOML source without interpreting values. */ +export function scanTomlSource(source: string): ScannedTomlLine[] { + const lines = source.length > 0 ? source.split("\n") : []; + let state: ValueScanState = { multiline: null, depth: 0 }; + return lines.map((text): ScannedTomlLine => { + if (state.multiline || state.depth > 0) { + state = scanValue(text, 0, state).state; + return { kind: "continuation", text }; + } + const trimmed = text.trim(); + if (trimmed.length === 0) return { kind: "blank", text }; + if (trimmed.startsWith("#")) return { kind: "comment", text }; + if (trimmed.startsWith("[")) { + const arrayOfTables = trimmed.startsWith("[["); + const open = text.indexOf("[") + (arrayOfTables ? 2 : 1); + const key = parseKeyPath(text, open); + if (!key) return { kind: "unknown", text }; + const closer = arrayOfTables ? "]]" : "]"; + if (!text.startsWith(closer, key.end)) return { kind: "unknown", text }; + const rest = text.slice(key.end + closer.length).trim(); + if (rest.length > 0 && !rest.startsWith("#")) return { kind: "unknown", text }; + return { kind: "header", text, path: key.path, arrayOfTables, comment: rest }; + } + const key = parseKeyPath(text, 0); + if (!key || text[key.end] !== "=") return { kind: "unknown", text }; + const scanned = scanValue(text, key.end + 1, { multiline: null, depth: 0 }); + state = scanned.state; + return { + kind: "assignment", + text, + path: key.path, + comment: scanned.comment, + indent: text.match(/^\s*/)?.[0] ?? "", + }; + }); +} + +function pathsEqual(left: readonly string[], right: readonly string[]) { + return left.length === right.length && left.every((segment, index) => segment === right[index]); +} + +function pathStartsWith(path: readonly string[], prefix: readonly string[]) { + return path.length >= prefix.length && prefix.every((segment, index) => segment === path[index]); +} + +/** Serialize one writable value as TOML assignment text. */ +export function serializeTomlValue(value: TomlWritableValue) { + if (typeof value === "boolean") return value ? "true" : "false"; + if (typeof value === "string") return JSON.stringify(value); + const entries = Object.entries(value) + .filter((entry): entry is [string, string] => entry[1] !== undefined) + .map(([key, entry]) => `${key} = ${JSON.stringify(entry)}`); + return `{ ${entries.join(", ")} }`; +} + +/** The lines one table owns: `start` is the first body line, `end` is exclusive. */ +interface TableBlock { + headerIndex: number; + start: number; + end: number; +} + +/** Locate the body of `[path]`, or the root table when `path` is empty. */ +function findTableBlock( + lines: readonly ScannedTomlLine[], + path: readonly string[], +): TableBlock | null { + const nextHeaderAfter = (index: number) => { + const offset = lines.slice(index).findIndex((line) => line.kind === "header"); + return offset < 0 ? lines.length : index + offset; + }; + if (path.length === 0) { + return { headerIndex: -1, start: 0, end: nextHeaderAfter(0) }; + } + const headerIndex = lines.findIndex( + (line) => line.kind === "header" && !line.arrayOfTables && pathsEqual(line.path, path), + ); + if (headerIndex < 0) return null; + return { headerIndex, start: headerIndex + 1, end: nextHeaderAfter(headerIndex + 1) }; +} + +/** Step `end` back over the blank and comment lines that introduce whatever follows the block. */ +function trimBlockEnd(lines: readonly ScannedTomlLine[], start: number, end: number) { + while (end > start) { + const kind = lines[end - 1]?.kind; + if (kind !== "blank" && kind !== "comment") break; + end -= 1; + } + return end; +} + +/** Step `start` back over the contiguous comment lines that document a header. */ +function leadingCommentStart(lines: readonly ScannedTomlLine[], headerIndex: number) { + let start = headerIndex; + while (start > 0 && lines[start - 1]?.kind === "comment") start -= 1; + return start; +} + +/** Report whether the table at `scope` defines `key`, as an assignment or a sub-table. */ +export function tomlTableDefinesKey( + source: string, + scope: readonly string[], + key: string, +): boolean { + const lines = scanTomlSource(source); + const block = findTableBlock(lines, scope); + const target = [...scope, key]; + if ( + block && + lines + .slice(block.start, block.end) + .some((line) => line.kind === "assignment" && line.path[0] === key) + ) { + return true; + } + return lines.some((line) => line.kind === "header" && pathStartsWith(line.path, target)); +} + +/** Replace a whole-line run with `replacement`, leaving at most one blank line between neighbours. */ +function collapseBlankSeam(lines: string[], index: number) { + let start = index; + while (start > 0 && (lines[start - 1] ?? "").trim().length === 0) start -= 1; + let end = index; + while (end < lines.length && (lines[end] ?? "").trim().length === 0) end += 1; + const separators = start > 0 && end < lines.length ? [""] : []; + lines.splice(start, end - start, ...separators); +} + +function joinLines(lines: readonly string[]) { + return `${lines.join("\n").replace(/\n*$/, "")}\n`; +} + +/** Rewrite one assignment line in place, keeping its indent and trailing comment. */ +function rewriteAssignment(line: ScannedTomlLine & { kind: "assignment" }, assignment: string) { + return `${line.indent}${assignment}${line.comment ? ` ${line.comment}` : ""}`; +} + +/** Insert lines into the table at `scope`, after its last definition. */ +function insertIntoTable( + lines: string[], + scope: readonly string[], + newLines: readonly string[], +): boolean { + const scanned = scanTomlSource(joinLines(lines)); + const block = findTableBlock(scanned, scope); + if (!block) return false; + if (block.headerIndex < 0) { + // Root keys sit above the first header, but leave that header's comment block attached to it + // and keep (or add) one blank line between the keys and the header. + let insertAt = block.end; + if (insertAt < scanned.length) { + insertAt = leadingCommentStart(scanned, insertAt); + } + const hasSpacer = insertAt > 0 && scanned[insertAt - 1]?.kind === "blank"; + if (hasSpacer) insertAt -= 1; + // A hoisted comment block reads as its own paragraph, so keep a blank line above it. + const leadIn = + insertAt > 0 && newLines[0]?.trim().startsWith("#") && scanned[insertAt - 1]?.kind !== "blank" + ? [""] + : []; + lines.splice( + insertAt, + 0, + ...leadIn, + ...newLines, + ...(hasSpacer || insertAt === scanned.length ? [] : [""]), + ); + return true; + } + const insertAt = trimBlockEnd(scanned, block.start, block.end); + const sibling = scanned + .slice(block.start, insertAt) + .reverse() + .find((line) => line.kind === "assignment"); + const indent = sibling?.kind === "assignment" ? sibling.indent : ""; + lines.splice(insertAt, 0, ...newLines.map((line) => `${indent}${line}`)); + return true; +} + +/** + * Write `key = value` into the table at `scope`, replacing every existing definition of the key: + * plain, quoted, or dotted assignments and `[scope.key]` sub-tables alike. Comments on a removed + * sub-table move with the key so nothing the user wrote is lost. Returns null when the scope table + * does not exist or the key is defined in a form this writer cannot rewrite. + */ +export function upsertTomlValue( + source: string, + scope: readonly string[], + key: string, + value: TomlWritableValue, +): string | null { + const scanned = scanTomlSource(source); + const block = findTableBlock(scanned, scope); + if (!block) return null; + const target = [...scope, key]; + if ( + scanned.some( + (line) => line.kind === "header" && line.arrayOfTables && pathStartsWith(line.path, target), + ) + ) { + return null; + } + const lines = scanned.map((line) => line.text); + const assignment = `${key} = ${serializeTomlValue(value)}`; + + const direct: number[] = []; + const dotted: number[] = []; + for (let index = block.start; index < block.end; index += 1) { + const line = scanned[index]; + if (line?.kind !== "assignment" || line.path[0] !== key) continue; + (line.path.length === 1 ? direct : dotted).push(index); + } + const subTableHeaders = scanned + .map((line, index) => + line.kind === "header" && pathStartsWith(line.path, target) ? index : -1, + ) + .filter((index) => index >= 0); + const exactSubTable = subTableHeaders.find((index) => { + const line = scanned[index]; + return line?.kind === "header" && pathsEqual(line.path, target); + }); + + // A flat table value keeps an existing `[scope.key]` sub-table and edits its keys in place. + if (typeof value === "object" && exactSubTable !== undefined && subTableHeaders.length === 1) { + const table = findTableBlock(scanned, target); + if (!table) return null; + const removals = [...direct, ...dotted]; + let end = trimBlockEnd(scanned, table.start, table.end); + const inserted: string[] = []; + for (const [entryKey, entry] of Object.entries(value)) { + const existing = scanned.findIndex( + (line, index) => + index >= table.start && + index < end && + line.kind === "assignment" && + pathsEqual(line.path, [entryKey]), + ); + const existingLine = scanned[existing]; + if (existingLine?.kind === "assignment") { + if (entry === undefined) removals.push(existing); + else + lines[existing] = rewriteAssignment( + existingLine, + `${entryKey} = ${JSON.stringify(entry)}`, + ); + continue; + } + if (entry !== undefined) inserted.push(`${entryKey} = ${JSON.stringify(entry)}`); + } + const sibling = scanned + .slice(table.start, end) + .reverse() + .find((line) => line.kind === "assignment"); + const indent = sibling?.kind === "assignment" ? sibling.indent : ""; + lines.splice(end, 0, ...inserted.map((line) => `${indent}${line}`)); + for (const index of removals.sort((left, right) => right - left)) { + lines.splice(index, 1); + if (index < end) end -= 1; + } + return joinLines(lines); + } + + // Otherwise one assignment line carries the value. Reuse the first existing assignment so its + // position and trailing comment survive, and fold every other definition away. + const hoisted: string[] = []; + let headerComment = ""; + const removalRanges: Array<{ start: number; end: number }> = []; + for (const headerIndex of subTableHeaders) { + const header = scanned[headerIndex]; + if (header?.kind !== "header") continue; + const start = leadingCommentStart(scanned, headerIndex); + const nextHeader = scanned.findIndex( + (line, index) => index > headerIndex && line.kind === "header", + ); + const end = trimBlockEnd( + scanned, + headerIndex + 1, + nextHeader < 0 ? scanned.length : nextHeader, + ); + for (let index = start; index < headerIndex; index += 1) hoisted.push(lines[index] ?? ""); + if (header.comment) headerComment ||= header.comment; + for (let index = headerIndex + 1; index < end; index += 1) { + const line = scanned[index]; + if (line?.kind === "comment") hoisted.push(line.text); + else if (line?.kind === "assignment" && line.comment) + hoisted.push(`${line.indent}${line.comment}`); + } + removalRanges.push({ start, end }); + } + + const [keep, ...duplicates] = [...direct, ...dotted].sort((left, right) => left - right); + const singleRemovals = duplicates.map((index) => ({ start: index, end: index + 1 })); + const withComment = headerComment ? `${assignment} ${headerComment}` : assignment; + + if (keep !== undefined) { + const line = scanned[keep]; + if (line?.kind !== "assignment") return null; + lines[keep] = rewriteAssignment( + line, + line.comment || !headerComment ? assignment : withComment, + ); + const ranges = [...removalRanges, ...singleRemovals].sort( + (left, right) => right.start - left.start, + ); + let insertAt = keep; + for (const range of ranges) { + lines.splice(range.start, range.end - range.start); + if (range.start < insertAt) insertAt -= range.end - range.start; + if (range.end - range.start > 1) collapseBlankSeam(lines, range.start); + } + // Hoisted comments explain the value that now lives on the rewritten line. + lines.splice(insertAt, 0, ...hoisted.map((comment) => `${line.indent}${comment.trim()}`)); + return joinLines(lines); + } + + for (const range of [...removalRanges, ...singleRemovals].sort( + (left, right) => right.start - left.start, + )) { + lines.splice(range.start, range.end - range.start); + collapseBlankSeam(lines, range.start); + } + if (!insertIntoTable(lines, scope, [...hoisted, withComment])) return null; + return joinLines(lines); +} diff --git a/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts b/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts index d77037ece..59ea6cffe 100644 --- a/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts +++ b/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts @@ -9,6 +9,7 @@ import { saveViewPreferencesPromptPreference, type PersistedViewPreferences, type ViewPreferenceChange, + type ViewPreferenceScope, } from "../../core/run/config"; const POST_PERSISTENCE_QUIT_DELAY_MS = 120; @@ -48,6 +49,7 @@ export interface ViewPreferenceQuitController { export interface UseViewPreferenceQuitControllerOptions { currentPreferences: PersistedViewPreferences; configPath?: string; + configScope?: ViewPreferenceScope; pagerMode: boolean; promptSaveViewPreferences: boolean; transientViewPreferences: boolean; @@ -74,6 +76,7 @@ function buildViewPreferenceDiffLines( export function useViewPreferenceQuitController({ currentPreferences, configPath, + configScope, pagerMode, promptSaveViewPreferences, transientViewPreferences, @@ -133,14 +136,26 @@ export function useViewPreferenceQuitController({ if (quitPendingRef.current) return; try { - const savedPath = saveGlobalViewPreferences(currentPreferences, { configPath }); + const savedPath = saveGlobalViewPreferences(currentPreferences, { + configPath, + baseline: savedPreferences, + scope: configScope, + }); setSavedPreferences(currentPreferences); showNotice(`Saved view preferences to ${savedPath}`); scheduleQuit(); } catch (error) { showError(error instanceof Error ? error.message : "Failed to save view preferences."); } - }, [configPath, currentPreferences, scheduleQuit, showError, showNotice]); + }, [ + configPath, + configScope, + currentPreferences, + savedPreferences, + scheduleQuit, + showError, + showNotice, + ]); /** Leave without persisting either the current preferences or prompt policy. */ const discardViewPreferencesAndQuit = useCallback(() => { From fd55c2a726dca3627aba5801824aae37c3f5d7d9 Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:25:54 -0600 Subject: [PATCH 7/9] fix(config): save a theme where the session read it Carry the resolved command and pager scope from config resolution into the review and history bootstraps and on to the quit prompt, so a theme picked under a `[diff.theme]` or `[pager]` override lands in that table rather than in a root key the override would shadow on the next run. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6 --- packages/hunk/src/app/historyBootstrap.ts | 3 +++ packages/hunk/src/app/sessionBootstrap.ts | 1 + packages/hunk/src/core/bootstrap.ts | 3 ++- packages/hunk/src/ui/App.tsx | 1 + packages/hunk/src/ui/history/types.ts | 3 ++- packages/hunk/src/ui/log/LogApp.tsx | 1 + 6 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/hunk/src/app/historyBootstrap.ts b/packages/hunk/src/app/historyBootstrap.ts index 38ed732f8..ce550e6c2 100644 --- a/packages/hunk/src/app/historyBootstrap.ts +++ b/packages/hunk/src/app/historyBootstrap.ts @@ -2,6 +2,7 @@ import type { HistoryCommandInput } from "../core/run/commandInputs"; import { persistedViewPreferencesFromOptions, type PersistedViewPreferences, + type ViewPreferenceScope, } from "../core/run/config"; import { collectSessionCustomThemes } from "../core/theme/customThemes"; import type { ThemeSelection } from "../core/theme/selection"; @@ -43,6 +44,7 @@ export interface HistoryBootstrap { /** Launch baseline retained so the owning history surface can persist theme changes on quit. */ initialViewPreferences: PersistedViewPreferences; viewPreferencesConfigPath?: string; + viewPreferenceScope?: ViewPreferenceScope; promptSaveViewPreferences: boolean; planReview( commit: ExtensionVcsHistoryCommit, @@ -148,6 +150,7 @@ export async function loadHistoryBootstrap({ customThemes: sessionThemes.themes, initialViewPreferences: persistedViewPreferencesFromOptions(resolved.configured.input.options), viewPreferencesConfigPath: resolved.configured.viewPreferencesConfigPath, + viewPreferenceScope: resolved.configured.viewPreferenceScope, promptSaveViewPreferences: resolved.configured.input.options.promptSaveViewPreferences !== false, notices: [ diff --git a/packages/hunk/src/app/sessionBootstrap.ts b/packages/hunk/src/app/sessionBootstrap.ts index 25ad868b5..1acb9fbfd 100644 --- a/packages/hunk/src/app/sessionBootstrap.ts +++ b/packages/hunk/src/app/sessionBootstrap.ts @@ -96,6 +96,7 @@ export async function loadConfiguredSessionBootstrap({ bootstrap.initialThemeMode = initialThemeMode ?? bootstrap.initialThemeMode; bootstrap.extensions = extensions; bootstrap.viewPreferencesConfigPath = configured.viewPreferencesConfigPath; + bootstrap.viewPreferenceScope = configured.viewPreferenceScope; bootstrap.keybindings = configured.keybindings; return { applied, bootstrap, input, previousFileLanguages, sessionThemes, sessionVcs }; diff --git a/packages/hunk/src/core/bootstrap.ts b/packages/hunk/src/core/bootstrap.ts index f541efe0f..27fdddac7 100644 --- a/packages/hunk/src/core/bootstrap.ts +++ b/packages/hunk/src/core/bootstrap.ts @@ -14,7 +14,7 @@ import type { ExtensionReviewDescriptor, NamedCustomThemeConfig } from "../extension-api/types"; import type { Changeset } from "./changeset/model"; import type { CliInput, CursorLine, LayoutMode, SidebarVisibility } from "./run/commandInputs"; -import type { UserKeyBinding } from "./run/config"; +import type { UserKeyBinding, ViewPreferenceScope } from "./run/config"; import type { StartupNotice } from "./process/startupNotice"; import type { TerminalThemeMode } from "./theme/detection"; import type { ThemeSelection } from "./theme/selection"; @@ -59,6 +59,7 @@ export interface AppBootstrap { /** Validated metadata describing a delegated or history-selected review source. */ review?: ExtensionReviewDescriptor; viewPreferencesConfigPath?: string; + viewPreferenceScope?: ViewPreferenceScope; /** The user's `[keybindings]` table, resolved against command defaults in App. */ keybindings?: Record; /** App-owned extension state carried without coupling core to the extension host. */ diff --git a/packages/hunk/src/ui/App.tsx b/packages/hunk/src/ui/App.tsx index 347cd7cae..75b2ad714 100644 --- a/packages/hunk/src/ui/App.tsx +++ b/packages/hunk/src/ui/App.tsx @@ -362,6 +362,7 @@ export function App({ const viewPreferenceQuit = useViewPreferenceQuitController({ currentPreferences: currentViewPreferences, configPath: bootstrap.viewPreferencesConfigPath, + configScope: bootstrap.viewPreferenceScope, pagerMode, promptSaveViewPreferences: bootstrap.input.options.promptSaveViewPreferences !== false && !returnToHistory, diff --git a/packages/hunk/src/ui/history/types.ts b/packages/hunk/src/ui/history/types.ts index 74ceb54df..5fe4be9b1 100644 --- a/packages/hunk/src/ui/history/types.ts +++ b/packages/hunk/src/ui/history/types.ts @@ -1,5 +1,5 @@ import type { HistoryCommandInput } from "../../core/run/commandInputs"; -import type { PersistedViewPreferences } from "../../core/run/config"; +import type { PersistedViewPreferences, ViewPreferenceScope } from "../../core/run/config"; import type { ThemeSelection } from "../../core/theme/selection"; import type { VcsHistorySource } from "../../core/vcs/types"; import type { ExtensionSession } from "../../extensions/session"; @@ -25,6 +25,7 @@ export interface HistoryRuntime { /** Resolved launch preferences retained while history owns the session-wide quit flow. */ initialViewPreferences: PersistedViewPreferences; viewPreferencesConfigPath?: string; + viewPreferenceScope?: ViewPreferenceScope; promptSaveViewPreferences: boolean; /** Command-owned extension authority borrowed by embedded reviews. */ extensionSession: ExtensionSession; diff --git a/packages/hunk/src/ui/log/LogApp.tsx b/packages/hunk/src/ui/log/LogApp.tsx index bb95cb73c..e65785294 100644 --- a/packages/hunk/src/ui/log/LogApp.tsx +++ b/packages/hunk/src/ui/log/LogApp.tsx @@ -115,6 +115,7 @@ export function LogApp({ const viewPreferenceQuit = useViewPreferenceQuitController({ currentPreferences: currentViewPreferences, configPath: runtime.viewPreferencesConfigPath, + configScope: runtime.viewPreferenceScope, pagerMode: false, promptSaveViewPreferences: runtime.promptSaveViewPreferences, transientViewPreferences: resolveExtensionSessionOptions( From 376c16cac316892c20a627d9fa0803330b4f7859 Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:27:42 -0600 Subject: [PATCH 8/9] fix(theme): probe the terminal after config settles Startup asked the terminal for its background right after the first config resolution, but an extension VCS backend can settle a repo root the bundled catalog could not, and that repo's config is only read in the second resolution. A `[theme]` pair introduced there never triggered the probe, so a light terminal drew the dark or fallback side. Move the probe after extension-backed resolution so it sees the final selection. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6 --- packages/hunk/src/app/startup.test.ts | 60 +++++++++++++++++++++++++++ packages/hunk/src/app/startup.ts | 38 ++++++++--------- 2 files changed, 78 insertions(+), 20 deletions(-) diff --git a/packages/hunk/src/app/startup.test.ts b/packages/hunk/src/app/startup.test.ts index 3a556fe36..eff25e023 100644 --- a/packages/hunk/src/app/startup.test.ts +++ b/packages/hunk/src/app/startup.test.ts @@ -806,6 +806,66 @@ describe("startup planning", () => { expect(probes).toBe(1); }); + test("probes the terminal for a pair that only extension-discovered repo config introduces", async () => { + const cliInput: CliInput = { + kind: "patch", + file: "-", + options: { theme: "github-dark-default", pager: true }, + }; + // A user extension recognizes the checkout the bundled catalog could not, so config resolves + // a second time against that repo's `.hunk/config.toml`, which is where the pair lives. + const extensionResult = createEmptyExtensionLoadResult(); + extensionResult.registry.vcsAdapters.push({ + extensionId: "probe", + adapter: { + id: "probe", + name: "Probe", + detect: (cwd) => ({ id: "probe", repoRoot: cwd }), + operations: {}, + }, + }); + let resolutions = 0; + let probes = 0; + + const plan = await prepareStartupPlan(["bun", "hunk", "patch", "-"], { + parseCliImpl: async () => cliInput as ParsedCliInput, + resolveRuntimeCliInputImpl: (input) => input, + resolveConfiguredCliInputImpl: (input) => { + resolutions += 1; + return createTestConfigResolution( + resolutions === 1 + ? input + : { + ...input, + options: { ...input.options, theme: { dark: "vitesse-dark", light: "one-light" } }, + }, + { extensions: { enabled: true, paths: [], repoPaths: [], extensionConfigs: {} } }, + ); + }, + loadStartupExtensionsImpl: async () => extensionResult, + loadAppBootstrapImpl: async (input) => createBootstrap(input), + openControllingTerminalImpl: () => ({ stdin: {} as never, close: () => {} }), + detectTerminalThemeModeFromBackgroundImpl: async () => { + probes += 1; + return "light"; + }, + usesPipedPatchInputImpl: () => false, + stdinIsTTY: false, + stdoutIsTTY: true, + stdout: { write: () => true } as never, + }); + + expect(resolutions).toBe(2); + expect(probes).toBe(1); + expect(plan).toMatchObject({ + kind: "app", + bootstrap: { + initialThemeMode: "light", + input: { options: { theme: { dark: "vitesse-dark", light: "one-light" } } }, + }, + }); + }); + test("skips the background probe when one theme covers every terminal", async () => { const cliInput: CliInput = { kind: "patch", diff --git a/packages/hunk/src/app/startup.ts b/packages/hunk/src/app/startup.ts index a0c5b0e8b..3e550945a 100644 --- a/packages/hunk/src/app/startup.ts +++ b/packages/hunk/src/app/startup.ts @@ -555,26 +555,6 @@ export async function prepareStartupPlan( controllingTerminal = openControllingTerminalImpl(); } - // Embedded reviews inherit their owner's detected mode so bootstrap never queries a terminal - // whose input and renderer are already exclusively owned. - let initialThemeMode: AppBootstrap["initialThemeMode"] = deps.terminalThemeMode; - if ( - !initialThemeMode && - themeSelectionNeedsTerminalMode(cliInput.options.theme) && - stdoutIsTTY - ) { - const themeInput = controllingTerminal?.stdin ?? (stdinIsTTY ? process.stdin : null); - if (themeInput) { - initialThemeMode = - (await whileStartupOwnsExtensions(() => - detectTerminalThemeModeFromBackgroundImpl({ - input: themeInput, - output: stdout, - }), - )) ?? undefined; - } - } - // Extensions load before the changeset so later stages can hand their VCS adapters and // changeset transforms to the loading pipeline. External adapters may settle a root the // bundled catalog could not; the shared resolver then appends newly discovered repo @@ -616,6 +596,24 @@ export async function prepareStartupPlan( deps.signal.throwIfAborted(); } + // Probe the terminal background only now that the theme selection is final: an extension + // VCS backend may have settled a repo root whose `.hunk/config.toml` introduces an adaptive + // pair the first resolution never saw. Embedded reviews inherit their owner's detected mode + // so bootstrap never queries a terminal whose input and renderer are already exclusively owned. + let initialThemeMode: AppBootstrap["initialThemeMode"] = deps.terminalThemeMode; + if (!initialThemeMode && themeSelectionNeedsTerminalMode(cliInput.options.theme) && stdoutIsTTY) { + const themeInput = controllingTerminal?.stdin ?? (stdinIsTTY ? process.stdin : null); + if (themeInput) { + initialThemeMode = + (await whileStartupOwnsExtensions(() => + detectTerminalThemeModeFromBackgroundImpl({ + input: themeInput, + output: stdout, + }), + )) ?? undefined; + } + } + let preparedSession: SessionBootstrapResult; try { preparedSession = await loadConfiguredSessionBootstrap({ From d2a509b9c75bdfdb06b3bfff1c0de50883aebba1 Mon Sep 17 00:00:00 2001 From: Mason McElvain <52104630+masonmcelvain@users.noreply.github.com> Date: Mon, 7 Sep 2026 08:27:43 -0600 Subject: [PATCH 9/9] docs(theme): describe how a saved theme lands in config Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_017mPP65ngDNSnKqBG3LU1K6 --- .changeset/tidy-hounds-repeat.md | 2 ++ docs/themes.md | 6 +++++- scripts/generate/generate-docs.ts | 2 ++ website/src/content/docs/docs/configure/themes.md | 2 +- website/src/content/docs/docs/reference/config.md | 2 ++ 5 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.changeset/tidy-hounds-repeat.md b/.changeset/tidy-hounds-repeat.md index c2972fa29..23255a3db 100644 --- a/.changeset/tidy-hounds-repeat.md +++ b/.changeset/tidy-hounds-repeat.md @@ -3,3 +3,5 @@ --- Accept a `[theme]` table that names one theme per terminal background, so `dark` and `light` terminals each get a theme you chose instead of only Hunk's GitHub defaults. `fallback` covers terminals that never report a background. + +Saving view preferences from the app now rewrites only the keys you changed, keeps the comments around them, writes a key back into the `[pager]` or command table that defined it, and refuses to touch `config.toml` at all if the result would not read back as the same file with just those keys changed. diff --git a/docs/themes.md b/docs/themes.md index 4f8ca53ee..fde6db97b 100644 --- a/docs/themes.md +++ b/docs/themes.md @@ -35,7 +35,11 @@ any built-in id, a custom theme id, and the compatibility aliases. A `--theme ` flag overrides the table for that run, and picking a theme in the app (`t`, or `View -> Themes…`) replaces the pair with the single id you -chose — the save-on-quit prompt shows that before writing anything. +chose — the save-on-quit prompt shows that before writing anything. The save +rewrites only the keys you changed, keeps the comments around them, and writes +a key back into the command table or `[pager]` table that defined it, so the +next run of that command sees your pick. If the file cannot be updated without +changing anything else, Hunk leaves it untouched and asks you to edit it by hand. Older theme ids such as `graphite` and `paper` remain accepted as compatibility aliases. diff --git a/scripts/generate/generate-docs.ts b/scripts/generate/generate-docs.ts index a8f2b1b8b..513390339 100644 --- a/scripts/generate/generate-docs.ts +++ b/scripts/generate/generate-docs.ts @@ -315,6 +315,8 @@ ${sectionRows.length > 0 ? ["| Table | Applies to |", "| --- | --- |", ...sectio \`[pager]\` is an additional overlay for any review opened with pager-style chrome. It is applied after the matching command table in the same file. +When you save view preferences from the app, each changed key is written back into the most specific table that already defines it: \`[pager]\` first, then the command table, then the root. Keys no table defines are added at the root. + ## Custom themes Set \`theme = "custom"\` and add a root \`[custom_theme]\` table. \`custom_theme.base\` accepts one of these built-in ids and defaults to \`${CONFIG_REFERENCE_CUSTOM_THEME.defaultBase}\` when layered custom themes need a base: diff --git a/website/src/content/docs/docs/configure/themes.md b/website/src/content/docs/docs/configure/themes.md index 86a3b226d..1ba0dc0de 100644 --- a/website/src/content/docs/docs/configure/themes.md +++ b/website/src/content/docs/docs/configure/themes.md @@ -24,7 +24,7 @@ fallback = "github-dark-default" `dark` and `light` are required. Hunk queries the terminal background the way `auto` does, then draws the matching side. The optional `fallback` covers sessions where Hunk never gets an answer: terminals that ignore the query, and captured pager hosts such as LazyGit, where Hunk never asks. Without it those sessions use `dark`. Both sides accept built-in ids, custom theme ids, and the compatibility aliases. -A `--theme ` flag overrides the table for one run. Picking a theme in the app replaces the pair with that single id, and the save-on-quit prompt shows the change before writing it. +A `--theme ` flag overrides the table for one run. Picking a theme in the app replaces the pair with that single id, and the save-on-quit prompt shows the change before writing it. The save touches only the keys you changed, keeps your comments, and writes a key back into the command table or `[pager]` table that defined it. If the file cannot be updated without changing anything else, Hunk leaves it untouched and asks you to edit it by hand. ## Create a custom theme diff --git a/website/src/content/docs/docs/reference/config.md b/website/src/content/docs/docs/reference/config.md index 06bc2610c..fc96e66dd 100644 --- a/website/src/content/docs/docs/reference/config.md +++ b/website/src/content/docs/docs/reference/config.md @@ -222,6 +222,8 @@ Enable moved-line coloring when the renderer supports it. `[pager]` is an additional overlay for any review opened with pager-style chrome. It is applied after the matching command table in the same file. +When you save view preferences from the app, each changed key is written back into the most specific table that already defines it: `[pager]` first, then the command table, then the root. Keys no table defines are added at the root. + ## Custom themes Set `theme = "custom"` and add a root `[custom_theme]` table. `custom_theme.base` accepts one of these built-in ids and defaults to `github-dark-default` when layered custom themes need a base: