diff --git a/CHANGELOG.md b/CHANGELOG.md index 89505edf..d82cba4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable user-visible changes to Hunk are documented in this file. ### Added +- Added `,` and `.` shortcuts for previous and next file navigation in the review stream. + ### Changed ### Fixed diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 87dba698..872630a2 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -478,6 +478,7 @@ export function App({ layoutMode, moveToAnnotatedFile, moveToAnnotatedHunk, + moveToFile: review.moveToFile, moveToHunk: review.moveToHunk, refreshCurrentInput: triggerRefreshCurrentInput, requestQuit, @@ -504,6 +505,7 @@ export function App({ layoutMode, moveToAnnotatedFile, moveToAnnotatedHunk, + review.moveToFile, requestQuit, review.moveToHunk, selectLayoutMode, @@ -550,6 +552,7 @@ export function App({ focusArea, focusFilter, moveToAnnotatedHunk, + moveToFile: review.moveToFile, moveToHunk: review.moveToHunk, moveMenuItem, openMenu, diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 82dc9e45..44c1a241 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -2033,6 +2033,49 @@ describe("App interactions", () => { } }); + test("file navigation shortcuts jump between file headers", async () => { + const setup = await testRender(, { + width: 220, + height: 10, + }); + + try { + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText("."); + }); + await flush(setup); + + let frame = await waitForFrame( + setup, + (nextFrame) => + nextFrame.includes("second.ts") && (nextFrame.match(/first\.ts/g) ?? []).length === 1, + 24, + ); + expect(frame).toContain("second.ts"); + expect((frame.match(/first\.ts/g) ?? []).length).toBe(1); + + await act(async () => { + await setup.mockInput.typeText(","); + }); + await flush(setup); + + frame = await waitForFrame( + setup, + (nextFrame) => + nextFrame.includes("first.ts") && (nextFrame.match(/second\.ts/g) ?? []).length === 1, + 24, + ); + expect(frame).toContain("first.ts"); + expect((frame.match(/second\.ts/g) ?? []).length).toBe(1); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("forward cross-file hunk navigation keeps the destination file owning the review pane", async () => { const setup = await testRender( , diff --git a/src/ui/components/chrome/HelpDialog.tsx b/src/ui/components/chrome/HelpDialog.tsx index 1ed1f595..24291758 100644 --- a/src/ui/components/chrome/HelpDialog.tsx +++ b/src/ui/components/chrome/HelpDialog.tsx @@ -26,6 +26,7 @@ export function HelpDialog({ ["Shift+Space", "page up (alt)"], ["d / u", "half page down / up"], ["[ / ]", "previous / next hunk"], + [", / .", "previous / next file"], ["{ / }", "previous / next comment"], ["← / →", "scroll code left / right (Shift = faster)"], ["Home / End", "jump to top / bottom"], diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 668a3f15..40fdb0b2 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -1605,6 +1605,7 @@ describe("UI components", () => { "Shift+Space page up (alt)", "d / u half page down / up", "[ / ] previous / next hunk", + ", / . previous / next file", "{ / } previous / next comment", "← / → scroll code left / right (Shift = faster)", "Home / End jump to top / bottom", diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index f2349d9e..4f118de8 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -29,6 +29,7 @@ export interface UseAppKeyboardShortcutsOptions { focusArea: FocusArea; focusFilter: () => void; moveToAnnotatedHunk: (delta: number) => void; + moveToFile: (delta: number) => void; moveToHunk: (delta: number) => void; moveMenuItem: (delta: number) => void; openMenu: (menuId: MenuId) => void; @@ -60,6 +61,7 @@ export function useAppKeyboardShortcuts({ focusArea, focusFilter, moveToAnnotatedHunk, + moveToFile, moveToHunk, moveMenuItem, openMenu, @@ -376,6 +378,16 @@ export function useAppKeyboardShortcuts({ return; } + if (key.name === "," || key.sequence === ",") { + runAndCloseMenu(() => moveToFile(-1)); + return; + } + + if (key.name === "." || key.sequence === ".") { + runAndCloseMenu(() => moveToFile(1)); + return; + } + if (key.sequence === "{") { runAndCloseMenu(() => moveToAnnotatedHunk(-1)); return; diff --git a/src/ui/hooks/useReviewController.test.tsx b/src/ui/hooks/useReviewController.test.tsx index d0885f59..6ea69570 100644 --- a/src/ui/hooks/useReviewController.test.tsx +++ b/src/ui/hooks/useReviewController.test.tsx @@ -198,6 +198,72 @@ describe("useReviewController", () => { } }); + test("moves through files using the current visible review order", async () => { + const controllerRef: { current: ReviewController | null } = { current: null }; + const setup = await testRender( + { + controllerRef.current = nextController; + }} + />, + { width: 80, height: 4 }, + ); + + try { + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("alpha.ts"); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("beta.ts"); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("gamma.ts"); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(-1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("beta.ts"); + + await act(async () => { + expectValue(controllerRef.current).setFilter("gamma"); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("gamma.ts"); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(-1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("gamma.ts"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("live comment mutations update annotated navigation without remounting the app", async () => { const controllerRef: { current: ReviewController | null } = { current: null }; const setup = await testRender( diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index cd2508dc..32dbce98 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -32,6 +32,7 @@ import type { RemovedCommentResult, SessionLiveCommentSummary, } from "../../hunk-session/types"; +import { findNextFile } from "../lib/files"; import { findNextHunkCursor } from "../lib/hunks"; import { buildReviewState, @@ -60,6 +61,7 @@ export interface ReviewController { liveCommentsByFileId: Record; moveToAnnotatedFile: (delta: number) => void; moveToAnnotatedHunk: (delta: number) => void; + moveToFile: (delta: number) => void; moveToHunk: (delta: number) => void; scrollToNote: boolean; selectedFile: DiffFile | undefined; @@ -247,6 +249,19 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon [selectFile, selectedFile?.id, visibleFiles], ); + /** Move through the currently visible files in review stream order. */ + const moveToFile = useCallback( + (delta: number) => { + const nextFile = findNextFile(visibleFiles, selectedFile?.id, delta); + if (!nextFile) { + return; + } + + selectFile(nextFile.id, 0, { alignFileHeaderTop: true }); + }, + [selectFile, selectedFile?.id, visibleFiles], + ); + /** Clear the active file filter without touching the current selection. */ const clearFilter = useCallback(() => { setFilter(""); @@ -504,6 +519,7 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon clearLiveComments, moveToAnnotatedFile, moveToAnnotatedHunk, + moveToFile, moveToHunk, navigateToLocation, removeLiveComment, diff --git a/src/ui/lib/appMenus.ts b/src/ui/lib/appMenus.ts index 058bb33c..3b5200e9 100644 --- a/src/ui/lib/appMenus.ts +++ b/src/ui/lib/appMenus.ts @@ -9,6 +9,7 @@ export interface BuildAppMenusOptions { layoutMode: LayoutMode; moveToAnnotatedFile: (delta: number) => void; moveToAnnotatedHunk: (delta: number) => void; + moveToFile: (delta: number) => void; moveToHunk: (delta: number) => void; refreshCurrentInput: () => void; requestQuit: () => void; @@ -37,6 +38,7 @@ export function buildAppMenus({ layoutMode, moveToAnnotatedFile, moveToAnnotatedHunk, + moveToFile, moveToHunk, refreshCurrentInput, requestQuit, @@ -173,6 +175,19 @@ export function buildAppMenus({ action: () => moveToHunk(1), }, { kind: "separator" }, + { + kind: "item", + label: "Previous file", + hint: ",", + action: () => moveToFile(-1), + }, + { + kind: "item", + label: "Next file", + hint: ".", + action: () => moveToFile(1), + }, + { kind: "separator" }, { kind: "item", label: "Previous comment", diff --git a/src/ui/lib/files.test.ts b/src/ui/lib/files.test.ts index 4e2b653f..a5de11f7 100644 --- a/src/ui/lib/files.test.ts +++ b/src/ui/lib/files.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { createTestDiffFile, lines } from "../../../test/helpers/diff-helpers"; -import { buildSidebarEntries, fileLabelParts } from "./files"; +import { buildSidebarEntries, fileLabelParts, findNextFile } from "./files"; describe("files helpers", () => { test("buildSidebarEntries hides zero-value sidebar stats", () => { @@ -134,4 +134,35 @@ describe("files helpers", () => { stateLabel: null, }); }); + + test("findNextFile follows visible file order and clamps at stream edges", () => { + const files = [ + createTestDiffFile({ + id: "alpha", + path: "alpha.ts", + before: lines("export const alpha = 1;"), + after: lines("export const alpha = 2;"), + }), + createTestDiffFile({ + id: "beta", + path: "beta.ts", + before: lines("export const beta = 1;"), + after: lines("export const beta = 2;"), + }), + createTestDiffFile({ + id: "gamma", + path: "gamma.ts", + before: lines("export const gamma = 1;"), + after: lines("export const gamma = 2;"), + }), + ]; + + expect(findNextFile(files, "alpha", 1)?.id).toBe("beta"); + expect(findNextFile(files, "beta", -1)?.id).toBe("alpha"); + expect(findNextFile(files, "alpha", -1)?.id).toBe("alpha"); + expect(findNextFile(files, "gamma", 1)?.id).toBe("gamma"); + expect(findNextFile(files, undefined, 1)?.id).toBe("alpha"); + expect(findNextFile(files, undefined, -1)?.id).toBe("gamma"); + expect(findNextFile([], "alpha", 1)).toBeNull(); + }); }); diff --git a/src/ui/lib/files.ts b/src/ui/lib/files.ts index 562860fc..85354b4b 100644 --- a/src/ui/lib/files.ts +++ b/src/ui/lib/files.ts @@ -116,6 +116,23 @@ export function filterReviewFiles(files: DiffFile[], query: string): DiffFile[] }); } +/** Move forward or backward through the visible file order, clamping at stream edges. */ +export function findNextFile(files: DiffFile[], currentFileId: string | undefined, delta: number) { + if (files.length === 0) { + return null; + } + + const currentIndex = files.findIndex((file) => file.id === currentFileId); + const nextIndex = + currentIndex >= 0 + ? Math.min(Math.max(currentIndex + delta, 0), files.length - 1) + : delta >= 0 + ? 0 + : files.length - 1; + + return files[nextIndex] ?? null; +} + /** Build the grouped sidebar entries while preserving the review stream order. */ export function buildSidebarEntries(files: DiffFile[]): SidebarEntry[] { const entries: SidebarEntry[] = []; diff --git a/src/ui/lib/ui-lib.test.ts b/src/ui/lib/ui-lib.test.ts index 14136c59..e4d6c834 100644 --- a/src/ui/lib/ui-lib.test.ts +++ b/src/ui/lib/ui-lib.test.ts @@ -127,6 +127,7 @@ describe("ui helpers", () => { layoutMode: "stack", moveToAnnotatedFile: () => {}, moveToAnnotatedHunk: () => {}, + moveToFile: () => {}, moveToHunk: () => {}, refreshCurrentInput: () => {}, requestQuit: () => {}, @@ -170,6 +171,19 @@ describe("ui helpers", () => { .filter((entry): entry is Extract => entry.kind === "item") .map((entry) => entry.label), ).toEqual(["Graphite", "Midnight", "Paper", "Ember"]); + expect( + menus.navigate + .filter((entry): entry is Extract => entry.kind === "item") + .map((entry) => [entry.label, entry.hint]), + ).toEqual([ + ["Previous hunk", "["], + ["Next hunk", "]"], + ["Previous file", ","], + ["Next file", "."], + ["Previous comment", "{"], + ["Next comment", "}"], + ["Focus filter", "/"], + ]); expect( menus.theme.some( (entry) => entry.kind === "item" && entry.label === "Graphite" && entry.checked, diff --git a/test/pty/ui-integration.test.ts b/test/pty/ui-integration.test.ts index 1696b963..90a1e4a5 100644 --- a/test/pty/ui-integration.test.ts +++ b/test/pty/ui-integration.test.ts @@ -353,6 +353,52 @@ describe("live UI integration", () => { } }); + test("file navigation shortcuts jump between file headers in a real PTY", async () => { + const fixture = harness.createPinnedHeaderRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 10, + }); + + try { + await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + await session.press("."); + const nextFile = await harness.waitForSnapshot( + session, + (text) => + text.includes("second.ts") && + text.includes("line17 = 117") && + harness.countMatches(text, /first\.ts/g) === 1, + 5_000, + ); + + expect(nextFile).toContain("second.ts"); + expect(nextFile).toContain("line17 = 117"); + expect(harness.countMatches(nextFile, /first\.ts/g)).toBe(1); + + await session.press(","); + const previousFile = await harness.waitForSnapshot( + session, + (text) => + text.includes("first.ts") && + text.includes("line01 = 101") && + harness.countMatches(text, /second\.ts/g) === 1, + 5_000, + ); + + expect(previousFile).toContain("first.ts"); + expect(previousFile).toContain("line01 = 101"); + expect(harness.countMatches(previousFile, /second\.ts/g)).toBe(1); + } finally { + session.close(); + } + }); + test("mouse wheel scrolling preserves the divider and header handoff in a real PTY", async () => { const fixture = harness.createPinnedHeaderRepoFixture(); const session = await harness.launchHunk({