diff --git a/.changeset/quiet-logs-save.md b/.changeset/quiet-logs-save.md new file mode 100644 index 000000000..4d03b727e --- /dev/null +++ b/.changeset/quiet-logs-save.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Prompt to save theme changes when quitting an interactive `hunk log` session, matching other review commands. diff --git a/packages/hunk/src/app/historyBootstrap.test.ts b/packages/hunk/src/app/historyBootstrap.test.ts index e25487906..3d6c13adc 100644 --- a/packages/hunk/src/app/historyBootstrap.test.ts +++ b/packages/hunk/src/app/historyBootstrap.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { HistoryCommandInput } from "../core/run/commandInputs"; @@ -21,6 +21,12 @@ 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-")); const configHome = mkdtempSync(join(tmpdir(), "hunk-history-config-")); + const configPath = join(configHome, "hunk", "config.toml"); + mkdirSync(join(configHome, "hunk"), { recursive: true }); + writeFileSync( + configPath, + 'theme = "github-dark-dimmed"\nline_numbers = false\nprompt_save_view_preferences = false\n', + ); const closeCounts: number[] = []; let opens = 0; const makeSource = (): VcsHistorySource => { @@ -62,6 +68,14 @@ describe("history bootstrap cursor ownership", () => { env: { ...process.env, XDG_CONFIG_HOME: configHome }, baseVcsCatalog: catalog, }); + expect(bootstrap.input.theme).toBe("github-dark-dimmed"); + expect(bootstrap.initialViewPreferences).toMatchObject({ + theme: "github-dark-dimmed", + showLineNumbers: false, + }); + expect(bootstrap.viewPreferencesConfigPath).toBe(configPath); + expect(bootstrap.promptSaveViewPreferences).toBe(false); + const cancelled = new AbortController(); cancelled.abort(); await expect(bootstrap.reopenSource(cancelled.signal)).rejects.toThrow(); diff --git a/packages/hunk/src/app/historyBootstrap.ts b/packages/hunk/src/app/historyBootstrap.ts index 08bb25b7e..f06f11066 100644 --- a/packages/hunk/src/app/historyBootstrap.ts +++ b/packages/hunk/src/app/historyBootstrap.ts @@ -1,4 +1,8 @@ import type { HistoryCommandInput } from "../core/run/commandInputs"; +import { + persistedViewPreferencesFromOptions, + type PersistedViewPreferences, +} from "../core/run/config"; import { collectSessionCustomThemes } from "../core/theme/customThemes"; import type { ExtensionVcsHistoryCommit, @@ -34,6 +38,10 @@ export interface HistoryBootstrap { extensionSession: ExtensionSession; notices: readonly string[]; customThemes: readonly NamedCustomThemeConfig[]; + /** Launch baseline retained so the owning history surface can persist theme changes on quit. */ + initialViewPreferences: PersistedViewPreferences; + viewPreferencesConfigPath?: string; + promptSaveViewPreferences: boolean; planReview( commit: ExtensionVcsHistoryCommit, options?: ExtensionVcsHistoryReviewOptions, @@ -136,6 +144,10 @@ export async function loadHistoryBootstrap({ repoRoot, extensionSession, customThemes: sessionThemes.themes, + initialViewPreferences: persistedViewPreferencesFromOptions(resolved.configured.input.options), + viewPreferencesConfigPath: resolved.configured.viewPreferencesConfigPath, + promptSaveViewPreferences: + resolved.configured.input.options.promptSaveViewPreferences !== false, notices: [ ...(mergeStartupNotices(resolved.configured.startupNotices, resolved.extensions) ?? []).map( (notice) => sanitizeTerminalLine(notice.message), diff --git a/packages/hunk/src/core/run/config.ts b/packages/hunk/src/core/run/config.ts index 4ed7f695f..940d26f87 100644 --- a/packages/hunk/src/core/run/config.ts +++ b/packages/hunk/src/core/run/config.ts @@ -99,6 +99,23 @@ const DEFAULT_VIEW_PREFERENCES: PersistedViewPreferences = { cursorLine: "row", }; +/** Project resolved launch options into the complete preference shape used by persistence. */ +export function persistedViewPreferencesFromOptions( + options: CommonOptions, +): PersistedViewPreferences { + return { + mode: options.mode ?? DEFAULT_VIEW_PREFERENCES.mode, + ...(options.theme === undefined ? {} : { theme: options.theme }), + showLineNumbers: options.lineNumbers ?? DEFAULT_VIEW_PREFERENCES.showLineNumbers, + wrapLines: options.wrapLines ?? DEFAULT_VIEW_PREFERENCES.wrapLines, + showHunkHeaders: options.hunkHeaders ?? DEFAULT_VIEW_PREFERENCES.showHunkHeaders, + showMenuBar: options.menuBar ?? DEFAULT_VIEW_PREFERENCES.showMenuBar, + showAgentNotes: options.agentNotes ?? DEFAULT_VIEW_PREFERENCES.showAgentNotes, + copyDecorations: options.copyDecorations ?? DEFAULT_VIEW_PREFERENCES.copyDecorations, + cursorLine: options.cursorLine ?? DEFAULT_VIEW_PREFERENCES.cursorLine, + }; +} + const VIEW_PREFERENCES_PROMPT_CONFIG_KEY = "prompt_save_view_preferences"; const PERSISTED_VIEW_PREFERENCE_KEYS: Array<{ configKey: string; diff --git a/packages/hunk/src/ui/App.tsx b/packages/hunk/src/ui/App.tsx index 1f3ad5030..2aebb0ce1 100644 --- a/packages/hunk/src/ui/App.tsx +++ b/packages/hunk/src/ui/App.tsx @@ -42,6 +42,7 @@ import type { ReloadedSessionResult, ReloadSessionOptions } from "../session/typ import { MenuBar } from "./components/chrome/MenuBar"; import { ConfirmDialog, confirmDialogHeight } from "./components/chrome/ConfirmDialog"; import { ExtensionDialog } from "./components/chrome/ExtensionDialog"; +import { ViewPreferenceQuitDialog } from "./components/chrome/ViewPreferenceQuitDialog"; import { ExtensionToast } from "./components/chrome/ExtensionToast"; import { StatusBar } from "./components/chrome/StatusBar"; import { DiffPane } from "./components/panes/DiffPane"; @@ -357,17 +358,7 @@ export function App({ const closeHelp = useCallback(() => { setShowHelp(false); }, []); - const { - changedViewPreferences, - saveConfigPromptOpen, - viewPreferenceDiffLines, - viewPreferencesConfigLabel, - requestQuit, - saveViewPreferencesAndQuit, - discardViewPreferencesAndQuit, - neverAskToSaveViewPreferencesAndQuit, - closeSaveConfigPrompt, - } = useViewPreferenceQuitController({ + const viewPreferenceQuit = useViewPreferenceQuitController({ currentPreferences: currentViewPreferences, configPath: bootstrap.viewPreferencesConfigPath, pagerMode, @@ -380,6 +371,14 @@ export function App({ closeHelp, homeDirectory: process.env.HOME, }); + const { + saveConfigPromptOpen, + requestQuit, + saveViewPreferencesAndQuit, + discardViewPreferencesAndQuit, + neverAskToSaveViewPreferencesAndQuit, + closeSaveConfigPrompt, + } = viewPreferenceQuit; const notifyExtensionMode = useCallback( (message: string, type?: ExtensionNotifyType) => extensions?.context.notify(message, type), [extensions], @@ -1527,45 +1526,12 @@ export function App({ ) : null} {saveConfigPromptOpen ? ( - - - - You changed {changedViewPreferences.length} view{" "} - {changedViewPreferences.length === 1 ? "setting" : "settings"} during this review. - - - - - Save {changedViewPreferences.length === 1 ? "it" : "them"} to your config before - quitting? - - - - - {viewPreferencesConfigLabel} - - {viewPreferenceDiffLines.map((line) => ( - - - {line.text} - - - ))} - + /> ) : null} {extensionTrustPromptOpen && extensionTrustPromptRoot ? ( diff --git a/packages/hunk/src/ui/AppHost.interactions.test.tsx b/packages/hunk/src/ui/AppHost.interactions.test.tsx index c37bb0b68..b00106ed5 100644 --- a/packages/hunk/src/ui/AppHost.interactions.test.tsx +++ b/packages/hunk/src/ui/AppHost.interactions.test.tsx @@ -3786,7 +3786,7 @@ describe("App interactions", () => { let frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("Save view preferences?"), ); - expect(frame).toContain("You changed 1 view setting during this review."); + expect(frame).toContain("You changed 1 view setting during this session."); expect(frame).toContain("q discard"); expect(frame).toContain("n never ask"); expect(frame).toContain('- theme = "github-dark-default"'); diff --git a/packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx b/packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx new file mode 100644 index 000000000..d00d4d210 --- /dev/null +++ b/packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx @@ -0,0 +1,65 @@ +import type { ViewPreferenceQuitController } from "../../hooks/useViewPreferenceQuitController"; +import type { AppTheme } from "../../themes"; +import { ConfirmDialog, confirmDialogHeight } from "./ConfirmDialog"; + +/** Render the shared save-or-discard prompt for changed view preferences. */ +export function ViewPreferenceQuitDialog({ + controller, + terminalHeight, + terminalWidth, + theme, +}: { + controller: ViewPreferenceQuitController; + terminalHeight: number; + terminalWidth: number; + theme: AppTheme; +}) { + const { + changedViewPreferences, + viewPreferenceDiffLines, + viewPreferencesConfigLabel, + saveViewPreferencesAndQuit, + discardViewPreferencesAndQuit, + neverAskToSaveViewPreferencesAndQuit, + closeSaveConfigPrompt, + } = controller; + + return ( + + + + You changed {changedViewPreferences.length} view{" "} + {changedViewPreferences.length === 1 ? "setting" : "settings"} during this session. + + + + + Save {changedViewPreferences.length === 1 ? "it" : "them"} to your config before quitting? + + + + + {viewPreferencesConfigLabel} + + {viewPreferenceDiffLines.map((line) => ( + + {line.text} + + ))} + + ); +} diff --git a/packages/hunk/src/ui/history/runStaticHistory.test.ts b/packages/hunk/src/ui/history/runStaticHistory.test.ts index 8d9379b82..3e9095202 100644 --- a/packages/hunk/src/ui/history/runStaticHistory.test.ts +++ b/packages/hunk/src/ui/history/runStaticHistory.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { EventEmitter } from "node:events"; import type { HistoryCommit } from "../../core/history/types"; +import { persistedViewPreferencesFromOptions } from "../../core/run/config"; import { createTestExtensionSession } from "../../../../../test/helpers/extension-session"; import type { HistoryRuntime } from "./types"; import { runStaticHistory } from "./runStaticHistory"; @@ -32,6 +33,8 @@ function runtime(commits: HistoryCommit[], maxCount?: number, closeFailure?: Err repoRoot: "/repo", notices: [], customThemes: [], + initialViewPreferences: persistedViewPreferencesFromOptions({}), + promptSaveViewPreferences: true, async planReview(commit) { return { kind: "revision-show", revisionId: commit.revisionId }; }, diff --git a/packages/hunk/src/ui/history/types.ts b/packages/hunk/src/ui/history/types.ts index 169cf8c94..43e84810a 100644 --- a/packages/hunk/src/ui/history/types.ts +++ b/packages/hunk/src/ui/history/types.ts @@ -1,4 +1,5 @@ import type { HistoryCommandInput } from "../../core/run/commandInputs"; +import type { PersistedViewPreferences } from "../../core/run/config"; import type { VcsHistorySource } from "../../core/vcs/types"; import type { ExtensionSession } from "../../extensions/session"; import type { @@ -19,6 +20,10 @@ export interface HistoryRuntime { repoRoot: string; notices: readonly string[]; customThemes: readonly NamedCustomThemeConfig[]; + /** Resolved launch preferences retained while history owns the session-wide quit flow. */ + initialViewPreferences: PersistedViewPreferences; + viewPreferencesConfigPath?: string; + promptSaveViewPreferences: boolean; /** Command-owned extension authority borrowed by embedded reviews. */ extensionSession: ExtensionSession; planReview( diff --git a/packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts b/packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts index 23327df90..3333e1a40 100644 --- a/packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -17,6 +17,7 @@ import type { ExtensionDialogRequest } from "../lib/extensionDialogs"; import { toExtensionKeyEvent } from "../lib/extensionKeyEvent"; import { isEscapeKey, isSaveDraftNoteKey } from "../lib/keyboard"; import { routeKeyOwnership, type KeyOwner } from "../lib/keyRouting"; +import { handleViewPreferenceQuitPromptKey } from "../lib/viewPreferenceQuitKeys"; type FocusArea = "files" | "filter" | "note"; @@ -260,27 +261,12 @@ export function useAppKeyboardShortcuts({ return "notMine"; } - if (key.name === "return" || key.name === "enter" || key.name === "s" || key.sequence === "s") { - saveViewPreferencesAndQuit(); - return "mine"; - } - - // "q" again quits and discards, so a double-tap of the quit key always exits. - if (key.name === "q" || key.sequence === "q") { - discardViewPreferencesAndQuit(); - return "mine"; - } - - if (key.name === "n" || key.sequence === "n") { - neverAskToSaveViewPreferencesAndQuit(); - return "mine"; - } - - if (isEscapeKey(key)) { - closeSaveConfigPrompt(); - return "mine"; - } - + handleViewPreferenceQuitPromptKey(key, { + saveViewPreferencesAndQuit, + discardViewPreferencesAndQuit, + neverAskToSaveViewPreferencesAndQuit, + closeSaveConfigPrompt, + }); return "mine"; }; diff --git a/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts b/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts index af1b64d33..d77037ece 100644 --- a/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts +++ b/packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts @@ -1,6 +1,6 @@ /** * Coordinates view-preference dirty state, persistence choices, prompt state, and safe delayed quits. - * App continues to render the dialog and own its keyboard and UI composition. + * Each interactive surface renders the shared dialog and owns its keyboard routing. */ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { @@ -31,7 +31,7 @@ export interface ViewPreferenceDiffLine { text: string; } -/** Dirty-state projection and quit actions consumed by App's existing UI composition. */ +/** Dirty-state projection and quit actions consumed by an interactive surface. */ export interface ViewPreferenceQuitController { changedViewPreferences: ViewPreferenceChange[]; saveConfigPromptOpen: boolean; @@ -44,7 +44,7 @@ export interface ViewPreferenceQuitController { closeSaveConfigPrompt: () => void; } -/** App-owned facts and side effects required by the view-preference quit workflow. */ +/** Surface-owned facts and side effects required by the view-preference quit workflow. */ export interface UseViewPreferenceQuitControllerOptions { currentPreferences: PersistedViewPreferences; configPath?: string; @@ -70,7 +70,7 @@ function buildViewPreferenceDiffLines( ]); } -/** Own view-preference dirty state and the save-or-discard quit workflow for one mounted App. */ +/** Own view-preference dirty state and the save-or-discard quit workflow for one surface. */ export function useViewPreferenceQuitController({ currentPreferences, configPath, diff --git a/packages/hunk/src/ui/lib/viewPreferenceQuitKeys.test.ts b/packages/hunk/src/ui/lib/viewPreferenceQuitKeys.test.ts new file mode 100644 index 000000000..1de3a0880 --- /dev/null +++ b/packages/hunk/src/ui/lib/viewPreferenceQuitKeys.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, mock, test } from "bun:test"; +import type { KeyEvent } from "@opentui/core"; +import { handleViewPreferenceQuitPromptKey } from "./viewPreferenceQuitKeys"; + +/** Build one minimal OpenTUI key event for prompt-dispatch tests. */ +function key(name: string, sequence = ""): KeyEvent { + return { name, sequence } as KeyEvent; +} + +/** Build observable prompt actions without coupling tests to persistence. */ +function actions() { + return { + saveViewPreferencesAndQuit: mock(() => undefined), + discardViewPreferencesAndQuit: mock(() => undefined), + neverAskToSaveViewPreferencesAndQuit: mock(() => undefined), + closeSaveConfigPrompt: mock(() => undefined), + }; +} + +describe("view preference quit prompt keys", () => { + test.each([ + [key("return"), "saveViewPreferencesAndQuit"], + [key("enter"), "saveViewPreferencesAndQuit"], + [key("s"), "saveViewPreferencesAndQuit"], + [key("unknown", "s"), "saveViewPreferencesAndQuit"], + [key("q"), "discardViewPreferencesAndQuit"], + [key("unknown", "q"), "discardViewPreferencesAndQuit"], + [key("n"), "neverAskToSaveViewPreferencesAndQuit"], + [key("unknown", "n"), "neverAskToSaveViewPreferencesAndQuit"], + [key("escape"), "closeSaveConfigPrompt"], + ] as const)("dispatches %o to %s", (input, expectedAction) => { + const promptActions = actions(); + + handleViewPreferenceQuitPromptKey(input, promptActions); + + expect(promptActions[expectedAction]).toHaveBeenCalledTimes(1); + expect( + Object.values(promptActions).reduce((count, action) => count + action.mock.calls.length, 0), + ).toBe(1); + }); + + test("does nothing for an unrecognized key", () => { + const promptActions = actions(); + + handleViewPreferenceQuitPromptKey(key("x", "x"), promptActions); + + expect(Object.values(promptActions).every((action) => action.mock.calls.length === 0)).toBe( + true, + ); + }); +}); diff --git a/packages/hunk/src/ui/lib/viewPreferenceQuitKeys.ts b/packages/hunk/src/ui/lib/viewPreferenceQuitKeys.ts new file mode 100644 index 000000000..1fc6d7ed6 --- /dev/null +++ b/packages/hunk/src/ui/lib/viewPreferenceQuitKeys.ts @@ -0,0 +1,35 @@ +import type { KeyEvent } from "@opentui/core"; +import type { ViewPreferenceQuitController } from "../hooks/useViewPreferenceQuitController"; +import { isEscapeKey } from "./keyboard"; + +type ViewPreferenceQuitPromptActions = Pick< + ViewPreferenceQuitController, + | "saveViewPreferencesAndQuit" + | "discardViewPreferencesAndQuit" + | "neverAskToSaveViewPreferencesAndQuit" + | "closeSaveConfigPrompt" +>; + +/** Dispatch one key owned by the save-view-preferences prompt. */ +export function handleViewPreferenceQuitPromptKey( + key: KeyEvent, + actions: ViewPreferenceQuitPromptActions, +): void { + if (key.name === "return" || key.name === "enter" || key.name === "s" || key.sequence === "s") { + actions.saveViewPreferencesAndQuit(); + return; + } + + // A repeated quit key discards, so double-tapping q always exits. + if (key.name === "q" || key.sequence === "q") { + actions.discardViewPreferencesAndQuit(); + return; + } + + if (key.name === "n" || key.sequence === "n") { + actions.neverAskToSaveViewPreferencesAndQuit(); + return; + } + + if (isEscapeKey(key)) actions.closeSaveConfigPrompt(); +} diff --git a/packages/hunk/src/ui/log/LogApp.tsx b/packages/hunk/src/ui/log/LogApp.tsx index 6e38d29a3..4645c19d4 100644 --- a/packages/hunk/src/ui/log/LogApp.tsx +++ b/packages/hunk/src/ui/log/LogApp.tsx @@ -1,17 +1,24 @@ import type { KeyEvent, MouseEvent as TuiMouseEvent } from "@opentui/core"; import { useKeyboard, useRenderer, useTerminalDimensions } from "@opentui/react"; import { basename } from "node:path"; -import { useEffect, useRef, useState, useSyncExternalStore } from "react"; +import { useEffect, useMemo, useRef, useState, useSyncExternalStore } from "react"; import type { ExtensionVcsHistoryCommit } from "../../extension-api/types"; import { sanitizeTerminalLine } from "../../lib/terminalText"; +import { resolveExtensionSessionOptions } from "../../extensions/apply"; import { HelpDialog } from "../components/chrome/HelpDialog"; import { MenuBar } from "../components/chrome/MenuBar"; import { MenuDropdown } from "../components/chrome/MenuDropdown"; import type { AppMenus, MenuEntry } from "../components/chrome/menu"; import { ThemeSelectorDialog } from "../components/chrome/ThemeSelectorDialog"; +import { ViewPreferenceQuitDialog } from "../components/chrome/ViewPreferenceQuitDialog"; import { useMenuController } from "../hooks/useMenuController"; import { useThemeSelectorController } from "../hooks/useThemeSelectorController"; +import { + useViewPreferenceQuitController, + type ViewPreferenceQuitScheduler, +} from "../hooks/useViewPreferenceQuitController"; import { fitText, measureTextWidth } from "../lib/text"; +import { handleViewPreferenceQuitPromptKey } from "../lib/viewPreferenceQuitKeys"; import type { HistoryRuntime } from "../history/types"; import type { LogController } from "./controller"; import { LOG_HELP_SECTIONS } from "./logHelp"; @@ -43,6 +50,7 @@ function HistoryGraphLine({ text, colors }: { text: string; colors: readonly str export type LogAppOutcome = | { kind: "quit"; exitCode?: number } + | { kind: "cancel-open-review" } | { kind: "open-review"; commit: ExtensionVcsHistoryCommit; @@ -57,11 +65,13 @@ export function LogApp({ runtime, onOutcome, useColor, + quitScheduler, }: { controller: LogController; runtime: HistoryRuntime; onOutcome: (outcome: LogAppOutcome) => void | Promise; useColor: boolean; + quitScheduler?: ViewPreferenceQuitScheduler; }) { const snapshot = useSyncExternalStore(controller.subscribe, controller.getSnapshot); const terminal = useTerminalDimensions(); @@ -76,6 +86,8 @@ export function LogApp({ // open two child reviews. Quit remains available while the host settles pending work. const reviewPending = useRef(false); const reviewQuitEnabled = useRef(false); + const quitRequestCaptured = useRef(false); + const pendingExitCode = useRef(undefined); const themeController = useThemeSelectorController({ customThemes: runtime.customThemes, initialTheme: snapshot.themeId, @@ -96,6 +108,30 @@ export function LogApp({ const selectedRow = snapshot.rows[snapshot.selected]; const responsiveLayout = resolveLogResponsiveLayout(terminal.width, terminal.height); const viewportBodyHeight = responsiveLayout.bodyHeight; + const currentViewPreferences = useMemo( + () => ({ ...runtime.initialViewPreferences, theme: themeController.themeId }), + [runtime.initialViewPreferences, themeController.themeId], + ); + const viewPreferenceQuit = useViewPreferenceQuitController({ + currentPreferences: currentViewPreferences, + configPath: runtime.viewPreferencesConfigPath, + pagerMode: false, + promptSaveViewPreferences: runtime.promptSaveViewPreferences, + transientViewPreferences: resolveExtensionSessionOptions( + runtime.extensionSession.current.registry, + ).transientViewPreferences, + onQuit: () => { + const exitCode = pendingExitCode.current; + pendingExitCode.current = undefined; + quitRequestCaptured.current = false; + void onOutcome({ kind: "quit", ...(exitCode === undefined ? {} : { exitCode }) }); + }, + showNotice: setTransientNotice, + showError: setTransientNotice, + closeHelp: () => setShowHelp(false), + homeDirectory: process.env.HOME, + quitScheduler, + }); const copySelected = (row = controller.getSelectedRow()) => { const currentRow = row; @@ -144,6 +180,37 @@ export function LogApp({ }, [transientNotice]); const clearTransientNotice = () => setTransientNotice(""); + /** Stop pending review preparation before beginning the history-owned quit decision. */ + const requestLogQuit = (exitCode?: number) => { + if (!quitRequestCaptured.current) { + quitRequestCaptured.current = true; + pendingExitCode.current = exitCode; + } + if (reviewPending.current && reviewQuitEnabled.current) { + reviewQuitEnabled.current = false; + void Promise.resolve(onOutcome({ kind: "cancel-open-review" })).then( + () => { + reviewPending.current = false; + setOpeningCommit(null); + }, + (error) => { + reviewPending.current = false; + setOpeningCommit(null); + controller.setNotice(error instanceof Error ? error.message : String(error)); + }, + ); + } + viewPreferenceQuit.requestQuit(); + }; + const closeLogSaveConfigPrompt = () => { + quitRequestCaptured.current = false; + pendingExitCode.current = undefined; + viewPreferenceQuit.closeSaveConfigPrompt(); + }; + const logViewPreferenceQuit = { + ...viewPreferenceQuit, + closeSaveConfigPrompt: closeLogSaveConfigPrompt, + }; const executeCommand = (id: LogCommandId, exitCode?: number) => { clearTransientNotice(); if (!isLogCommandEnabled(id, controller.getSnapshot())) return; @@ -158,7 +225,7 @@ export function LogApp({ void controller.refresh(); break; case "quit": - onOutcome({ kind: "quit", ...(exitCode === undefined ? {} : { exitCode }) }); + requestLogQuit(exitCode); break; case "theme": themeController.openThemeSelector(); @@ -318,6 +385,11 @@ export function LogApp({ }; const name = key.name; const sequence = key.sequence ?? ""; + if (viewPreferenceQuit.saveConfigPromptOpen) { + handleViewPreferenceQuitPromptKey(key, logViewPreferenceQuit); + consume(); + return; + } if (reviewPending.current) { if (!reviewQuitEnabled.current) { consume(); @@ -693,6 +765,14 @@ export function LogApp({ onClose={() => setShowHelp(false)} /> ) : null} + {viewPreferenceQuit.saveConfigPromptOpen ? ( + + ) : null} ); } diff --git a/packages/hunk/src/ui/log/controller.test.ts b/packages/hunk/src/ui/log/controller.test.ts index 3454a335a..bd6ade31f 100644 --- a/packages/hunk/src/ui/log/controller.test.ts +++ b/packages/hunk/src/ui/log/controller.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; import { createTestExtensionSession } from "../../../../../test/helpers/extension-session"; +import { persistedViewPreferencesFromOptions } from "../../core/run/config"; import type { HistoryRuntime } from "../history/types"; import { LogController } from "./controller"; @@ -43,6 +44,8 @@ function createRuntime(subjects = ["first", "second", "third"]) { repoRoot: "/repo", notices: [], customThemes: [], + initialViewPreferences: persistedViewPreferencesFromOptions({}), + promptSaveViewPreferences: true, async planReview(commit) { return { kind: "revision-show", revisionId: commit.revisionId }; }, diff --git a/packages/hunk/src/ui/session/HunkSessionHost.test.tsx b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx index 031b66656..f42ad2ccf 100644 --- a/packages/hunk/src/ui/session/HunkSessionHost.test.tsx +++ b/packages/hunk/src/ui/session/HunkSessionHost.test.tsx @@ -1,9 +1,13 @@ import { expect, mock, test } from "bun:test"; import { testRender } from "@opentui/react/test-utils"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { act } from "react"; import { createTestVcsAppBootstrap } from "../../../../../test/helpers/app-bootstrap"; import { createTestDiffFile } from "../../../../../test/helpers/diff-helpers"; import { createTestExtensionSession } from "../../../../../test/helpers/extension-session"; +import { persistedViewPreferencesFromOptions } from "../../core/run/config"; import { createEmptyExtensionLoadResult } from "../../extensions/types"; import type { HistoryRuntime } from "../history/types"; import { LogController } from "../log/controller"; @@ -52,6 +56,8 @@ async function createHistoryRoute() { repoRoot: "/repo", notices: [], customThemes: [], + initialViewPreferences: persistedViewPreferencesFromOptions({}), + promptSaveViewPreferences: true, async planReview() { return { kind: "revision-show", revisionId: "revision-a" }; }, @@ -350,6 +356,112 @@ test("waits for non-cooperative provider planning before menu quit", async () => } }); +test("blocks reopening until dirty-quit cancellation settles", async () => { + const history = await createHistoryRoute(); + let resolvePlanning!: (value: { kind: "revision-show"; revisionId: string }) => void; + const planning = new Promise<{ kind: "revision-show"; revisionId: string }>((resolve) => { + resolvePlanning = resolve; + }); + history.runtime.planReview = mock(() => planning); + const prepareReview = mock(async () => { + throw new Error("cancelled planning reached preparation"); + }); + const quit = mock(() => undefined); + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.typeText("t")); + await setup.renderOnce(); + expect(setup.captureCharFrame()).toContain("Theme selector"); + await act(async () => setup.mockInput.pressArrow("down")); + await setup.renderOnce(); + expect(setup.captureCharFrame()).toContain("› github-dark-dimmed"); + await act(async () => setup.mockInput.pressEnter()); + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + await Bun.sleep(10); + await selectHistoryMenuQuit(setup); + await setup.renderOnce(); + expect(setup.captureCharFrame()).toContain("Save view preferences?"); + + await act(async () => setup.mockInput.pressKey("escape")); + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + expect(setup.captureCharFrame()).not.toContain("Save view preferences?"); + + resolvePlanning({ kind: "revision-show", revisionId: "revision-a" }); + await settle(setup); + await act(async () => Bun.sleep(20)); + await setup.renderOnce(); + expect(prepareReview).not.toHaveBeenCalled(); + expect(setup.captureCharFrame()).toContain("History row"); + expect(setup.captureCharFrame()).not.toContain("Preparing review"); + expect(quit).not.toHaveBeenCalled(); + } 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-")); + history.runtime.viewPreferencesConfigPath = join(configHome, "config.toml"); + const quit = mock(() => undefined); + let scheduledQuit: (() => void) | undefined; + const setup = await testRender( + , + { width: 100, height: 20 }, + ); + try { + await setup.renderOnce(); + await act(async () => setup.mockInput.typeText("t")); + await setup.renderOnce(); + await act(async () => setup.mockInput.pressArrow("down")); + await setup.renderOnce(); + await act(async () => setup.mockInput.pressEnter()); + await setup.renderOnce(); + await act(async () => setup.mockInput.typeText("q")); + await setup.renderOnce(); + expect(setup.captureCharFrame()).toContain("Save view preferences?"); + + await act(async () => setup.mockInput.typeText("s")); + await setup.renderOnce(); + expect(setup.captureCharFrame()).toContain("Saved view preferences"); + await act(async () => setup.mockInput.pressKey("c", { ctrl: true })); + expect(scheduledQuit).toBeDefined(); + await act(async () => scheduledQuit?.()); + + expect(quit).toHaveBeenCalledTimes(1); + expect(quit).toHaveBeenCalledWith(undefined); + } finally { + setup.renderer.destroy(); + await history.controller.close(); + rmSync(configHome, { recursive: true, force: true }); + } +}); + test("waits for non-cooperative provider planning after an external signal", async () => { const history = await createHistoryRoute(); let resolvePlanning!: (value: { kind: "revision-show"; revisionId: string }) => void; diff --git a/packages/hunk/src/ui/session/HunkSessionHost.tsx b/packages/hunk/src/ui/session/HunkSessionHost.tsx index 1814885ec..ea7c7384c 100644 --- a/packages/hunk/src/ui/session/HunkSessionHost.tsx +++ b/packages/hunk/src/ui/session/HunkSessionHost.tsx @@ -14,6 +14,7 @@ import type { ExtensionSession } from "../../extensions/session"; import type { ExtensionLoadResult } from "../../extensions/types"; import { AppHost } from "../AppHost"; import type { HistoryRuntime } from "../history/types"; +import type { ViewPreferenceQuitScheduler } from "../hooks/useViewPreferenceQuitController"; import { interactiveLogUsesColor } from "../log/colorPolicy"; import { LogApp, type LogAppOutcome } from "../log/LogApp"; import type { LogController } from "../log/controller"; @@ -46,6 +47,7 @@ type ActiveSurfaceRoute = HistorySurfaceRoute | ActiveReviewSurfaceRoute; export interface HunkSessionHostDeps { prepareReview?: typeof prepareEmbeddedHistoryReview; createReviewRuntime?: typeof createReviewSessionRuntime; + viewPreferenceQuitScheduler?: ViewPreferenceQuitScheduler; } /** @@ -86,6 +88,7 @@ export function HunkSessionHost({ const preparingRef = useRef(false); const preparationControllerRef = useRef(null); const preparationGenerationRef = useRef(0); + const preparationSettlementRef = useRef | null>(null); const nextInstanceRef = useRef(initialRoute.kind === "review" ? initialRoute.instanceId + 1 : 1); const quitRequestedRef = useRef(false); const shutdownPendingRef = useRef(false); @@ -142,6 +145,15 @@ export function HunkSessionHost({ historyRoute: HistorySurfaceRoute, outcome: LogAppOutcome, ) => { + if (outcome.kind === "cancel-open-review") { + const settlement = preparationSettlementRef.current; + preparationGenerationRef.current += 1; + preparationControllerRef.current?.abort( + new Error("Hunk review preparation was cancelled before a history quit decision."), + ); + await settlement; + return; + } if (outcome.kind === "quit") { requestQuit(outcome.exitCode); return; @@ -156,6 +168,11 @@ export function HunkSessionHost({ return; } preparingRef.current = true; + let settlePreparation!: () => void; + const preparationSettlement = new Promise((resolve) => { + settlePreparation = resolve; + }); + preparationSettlementRef.current = preparationSettlement; const generation = ++preparationGenerationRef.current; const preparationController = new AbortController(); preparationControllerRef.current = preparationController; @@ -232,6 +249,10 @@ export function HunkSessionHost({ preparationControllerRef.current = null; } preparingRef.current = false; + if (preparationSettlementRef.current === preparationSettlement) { + preparationSettlementRef.current = null; + } + settlePreparation(); if (shutdownPendingRef.current && routeRef.current.kind === "history") { completeQuit(); } @@ -291,6 +312,7 @@ export function HunkSessionHost({ runtime={route.runtime} useColor={interactiveLogUsesColor(route.runtime.input.color, process.env)} onOutcome={(outcome) => handleHistoryOutcome(route, outcome)} + quitScheduler={deps.viewPreferenceQuitScheduler} /> ); } diff --git a/test/pty/log-integration.test.ts b/test/pty/log-integration.test.ts index 53834281d..62441d0bf 100644 --- a/test/pty/log-integration.test.ts +++ b/test/pty/log-integration.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test"; -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createPtyHarness, rightmostColumnOf } from "./harness"; @@ -218,6 +218,39 @@ describe("interactive hunk log", () => { } }); + test("prompts to save a changed theme when the history session quits", async () => { + const cwd = createHistoryRepo(); + const configHome = mkdtempSync(join(tmpdir(), "hunk-log-view-preferences-")); + tempDirs.push(configHome); + const session = await harness.launchHunk({ + args: ["log", "--color", "never", "--no-extensions"], + cwd, + cols: 100, + rows: 20, + env: { XDG_CONFIG_HOME: configHome }, + }); + + try { + await session.waitForText(/Second history commit/, { timeout: 15_000 }); + await session.press("t"); + await session.waitForText(/Theme selector/, { timeout: 5_000 }); + await session.press("down"); + await session.press("enter"); + await session.press("q"); + const prompt = await session.waitForText(/Save view preferences\?/, { timeout: 5_000 }); + expect(prompt).toContain('- theme = "github-dark-default"'); + expect(prompt).toContain('+ theme = "github-dark-dimmed"'); + + await session.press("s"); + const configPath = join(configHome, "hunk", "config.toml"); + const deadline = Date.now() + 5_000; + while (Date.now() < deadline && !existsSync(configPath)) await Bun.sleep(50); + expect(readFileSync(configPath, "utf8")).toContain('theme = "github-dark-dimmed"'); + } finally { + session.close(); + } + }); + test("adapts GitHub-style grouped rows and right-aligned ids on resize", async () => { const cwd = createHistoryRepo(); const displayId = Bun.spawnSync(["git", "rev-parse", "--short=8", "HEAD"], {