Skip to content

fix(ui): save log view preferences #1015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
benvinegar merged 2 commits into from
Sep 7, 2026
Merged

fix(ui): save log view preferences #1015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58ba85a
fix(ui): save log view preferences
benvinegar Sep 7, 2026
9dd6522
fix(ui): settle log quit races
benvinegar Sep 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/quiet-logs-save.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": patch
---

Prompt to save theme changes when quitting an interactive `hunk log` session, matching other review commands.
16 changes: 15 additions & 1 deletion packages/hunk/src/app/historyBootstrap.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 => {
Expand Down Expand Up @@ -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();
Expand Down
12 changes: 12 additions & 0 deletions packages/hunk/src/app/historyBootstrap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down
17 changes: 17 additions & 0 deletions packages/hunk/src/core/run/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
60 changes: 13 additions & 47 deletions packages/hunk/src/ui/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand All @@ -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],
Expand Down Expand Up @@ -1527,45 +1526,12 @@ export function App({
) : null}

{saveConfigPromptOpen ? (
<ConfirmDialog
actions={[
{ keyLabel: "enter/s", label: "save", run: saveViewPreferencesAndQuit },
{ keyLabel: "q", label: "discard", run: discardViewPreferencesAndQuit },
{ keyLabel: "n", label: "never ask", run: neverAskToSaveViewPreferencesAndQuit },
{ keyLabel: "esc", label: "cancel", run: closeSaveConfigPrompt },
]}
height={confirmDialogHeight(4 + viewPreferenceDiffLines.length)}
<ViewPreferenceQuitDialog
controller={viewPreferenceQuit}
terminalHeight={terminal.height}
terminalWidth={terminal.width}
theme={baseTheme}
title="Save view preferences?"
width={68}
onClose={closeSaveConfigPrompt}
>
<box style={{ width: "100%", height: 1 }}>
<text fg={baseTheme.muted}>
You changed {changedViewPreferences.length} view{" "}
{changedViewPreferences.length === 1 ? "setting" : "settings"} during this review.
</text>
</box>
<box style={{ width: "100%", height: 1 }}>
<text fg={baseTheme.muted}>
Save {changedViewPreferences.length === 1 ? "it" : "them"} to your config before
quitting?
</text>
</box>
<box style={{ width: "100%", height: 1 }} />
<box style={{ width: "100%", height: 1 }}>
<text fg={baseTheme.badgeNeutral}>{viewPreferencesConfigLabel}</text>
</box>
{viewPreferenceDiffLines.map((line) => (
<box key={line.text} style={{ width: "100%", height: 1 }}>
<text fg={line.removed ? baseTheme.badgeRemoved : baseTheme.badgeAdded}>
{line.text}
</text>
</box>
))}
</ConfirmDialog>
/>
) : null}

{extensionTrustPromptOpen && extensionTrustPromptRoot ? (
Expand Down
2 changes: 1 addition & 1 deletion packages/hunk/src/ui/AppHost.interactions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"');
Expand Down
65 changes: 65 additions & 0 deletions packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import type { ViewPreferenceQuitController } from "../../hooks/useViewPreferenceQuitController";

@greptile-apps greptile-apps Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Filename Violates Dash-Case Rule

The new ViewPreferenceQuitDialog.tsx file violates the repository directive requiring dash-case names for .ts and .tsx files. This repository requirement must be satisfied before merging: rename the file to view-preference-quit-dialog.tsx and update its imports.

Context Used: guidelines.mdc Cursor rule (source)

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/hunk/src/ui/components/chrome/ViewPreferenceQuitDialog.tsx
Line: 1

Comment:
**Filename Violates Dash-Case Rule**

The new `ViewPreferenceQuitDialog.tsx` file violates the repository directive requiring dash-case names for `.ts` and `.tsx` files. This repository requirement must be satisfied before merging: rename the file to `view-preference-quit-dialog.tsx` and update its imports.

**Context Used:** guidelines.mdc Cursor rule ([source](https://github.com/modem-dev/modem/blob/main/.cursor/rules/guidelines.mdc))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@benvinegar benvinegar Sep 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this. Hunk’s React component files conventionally use PascalCase in this directory (ConfirmDialog.tsx, ThemeSelectorDialog.tsx, HelpDialog.tsx, etc.), and the cited dash-case rule is from a different repository rather than Hunk’s checked-in contributor guidance. ViewPreferenceQuitDialog.tsx follows the local convention.

Responded by Pi using openai-codex/gpt-5.6-sol.

This comment was generated by Pi using GPT-5.6 Sol

@greptile-apps greptile-apps Bot Sep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The surrounding chrome components use PascalCase filenames (ConfirmDialog.tsx, ThemeSelectorDialog.tsx, HelpDialog.tsx), and the checked-in Hunk guidance does not establish a dash-case requirement here. ViewPreferenceQuitDialog.tsx follows the local convention, so I’m withdrawing this finding; no rename is needed.

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 (
<ConfirmDialog
actions={[
{ keyLabel: "enter/s", label: "save", run: saveViewPreferencesAndQuit },
{ keyLabel: "q", label: "discard", run: discardViewPreferencesAndQuit },
{ keyLabel: "n", label: "never ask", run: neverAskToSaveViewPreferencesAndQuit },
{ keyLabel: "esc", label: "cancel", run: closeSaveConfigPrompt },
]}
height={confirmDialogHeight(4 + viewPreferenceDiffLines.length)}
terminalHeight={terminalHeight}
terminalWidth={terminalWidth}
theme={theme}
title="Save view preferences?"
width={68}
onClose={closeSaveConfigPrompt}
>
<box style={{ width: "100%", height: 1 }}>
<text fg={theme.muted}>
You changed {changedViewPreferences.length} view{" "}
{changedViewPreferences.length === 1 ? "setting" : "settings"} during this session.
</text>
</box>
<box style={{ width: "100%", height: 1 }}>
<text fg={theme.muted}>
Save {changedViewPreferences.length === 1 ? "it" : "them"} to your config before quitting?
</text>
</box>
<box style={{ width: "100%", height: 1 }} />
<box style={{ width: "100%", height: 1 }}>
<text fg={theme.badgeNeutral}>{viewPreferencesConfigLabel}</text>
</box>
{viewPreferenceDiffLines.map((line) => (
<box key={line.text} style={{ width: "100%", height: 1 }}>
<text fg={line.removed ? theme.badgeRemoved : theme.badgeAdded}>{line.text}</text>
</box>
))}
</ConfirmDialog>
);
}
3 changes: 3 additions & 0 deletions packages/hunk/src/ui/history/runStaticHistory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 };
},
Expand Down
5 changes: 5 additions & 0 deletions packages/hunk/src/ui/history/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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(
Expand Down
28 changes: 7 additions & 21 deletions packages/hunk/src/ui/hooks/useAppKeyboardShortcuts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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";
};

Expand Down
8 changes: 4 additions & 4 deletions packages/hunk/src/ui/hooks/useViewPreferenceQuitController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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,
Expand Down
Loading
Loading