diff --git a/CHANGELOG.md b/CHANGELOG.md index 06aa067..0ea1461 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and - `watch = true` in `.dunk/config.toml` (or `~/.config/dunk/config.toml`) now enables watch mode without passing `--watch` every time. The `--watch`/absence of it on the command line still overrides the config value. - Ctrl-Z suspends `dunk` as a normal shell job (the terminal is restored, the process group gets SIGTSTP, and `fg` resumes the renderer). Previously Ctrl-Z was swallowed in raw mode and `dunk` could not be suspended. No-op on Windows. +- `,` and `.` jump to the previous / next file in the review stream (clamped at the ends), so you can skip whole files without stepping through every hunk. Shown in `?` help. ### Removed diff --git a/src/ui/App.tsx b/src/ui/App.tsx index e3a3526..ba9193b 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -987,6 +987,7 @@ export function App({ focusArea, focusFilter, moveToAnnotatedHunk, + moveToFile: review.moveToFile, moveToHunk: moveToHunkWithDrift, openCommentEditor, openInEditor, diff --git a/src/ui/components/chrome/HelpDialog.tsx b/src/ui/components/chrome/HelpDialog.tsx index 026b075..42f3cb1 100644 --- a/src/ui/components/chrome/HelpDialog.tsx +++ b/src/ui/components/chrome/HelpDialog.tsx @@ -24,6 +24,7 @@ export function HelpDialog({ ["Space / b", "page down / up"], ["Ctrl+D / Ctrl+U", "half page down / up"], ["K / J", "previous / next hunk"], + [", / .", "previous / next file"], ["← / →", "scroll code left / right (Shift = faster)"], ["gg / G", "jump to first / last hunk"], ["Home / End", "jump to top / bottom"], diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index a937763..d783f20 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; openCommentEditor: () => void; openInEditor: () => void; @@ -61,6 +62,7 @@ export function useAppKeyboardShortcuts({ focusArea, focusFilter, moveToAnnotatedHunk, + moveToFile, moveToHunk, openCommentEditor, openInEditor, @@ -338,6 +340,16 @@ export function useAppKeyboardShortcuts({ return; } + if (key.name === "," || key.sequence === ",") { + moveToFile(-1); + return; + } + + if (key.name === "." || key.sequence === ".") { + moveToFile(1); + return; + } + if (key.sequence === "D") { deleteAllDriftedComments(); return; diff --git a/src/ui/hooks/useReviewController.test.tsx b/src/ui/hooks/useReviewController.test.tsx index 36df021..ba16928 100644 --- a/src/ui/hooks/useReviewController.test.tsx +++ b/src/ui/hooks/useReviewController.test.tsx @@ -270,4 +270,89 @@ describe("useReviewController", () => { }); } }); + + test("moveToFile steps through visible files clamped, aligning the new file header", async () => { + const controllerRef: { current: ReviewController | null } = { current: null }; + const setup = await testRender( + { + controllerRef.current = nextController; + }} + />, + { width: 80, height: 4 }, + ); + + try { + await flush(setup); + + // Move off hunk 0 so a file jump must reset the hunk index. + await act(async () => { + expectValue(controllerRef.current).selectHunk("alpha", 1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedHunkIndex).toBe(1); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(1); + }); + await flush(setup); + let controller = expectValue(controllerRef.current); + expect(controller.selectedFile?.path).toBe("beta.ts"); + expect(controller.selectedHunkIndex).toBe(0); + expect(controller.selectedFileTopAlignRequestId).toBe(1); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("gamma.ts"); + + // Forward at the last file is a no-op and must not bump the align request. + await act(async () => { + expectValue(controllerRef.current).moveToFile(1); + }); + await flush(setup); + controller = expectValue(controllerRef.current); + expect(controller.selectedFile?.path).toBe("gamma.ts"); + expect(controller.selectedFileTopAlignRequestId).toBe(2); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(-1); + }); + await flush(setup); + controller = expectValue(controllerRef.current); + expect(controller.selectedFile?.path).toBe("beta.ts"); + const alignAfterBeta = controller.selectedFileTopAlignRequestId; + + // Step to the first file, then a backward press at the first file is a + // no-op and must not bump the align request. + await act(async () => { + expectValue(controllerRef.current).moveToFile(-1); + }); + await flush(setup); + expect(expectValue(controllerRef.current).selectedFile?.path).toBe("alpha.ts"); + + await act(async () => { + expectValue(controllerRef.current).moveToFile(-1); + }); + await flush(setup); + controller = expectValue(controllerRef.current); + expect(controller.selectedFile?.path).toBe("alpha.ts"); + expect(controller.selectedFileTopAlignRequestId).toBe(alignAfterBeta + 1); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); }); diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index 1b892d5..de087c2 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -37,6 +37,7 @@ export interface ReviewController { filter: string; moveToAnnotatedFile: (delta: number) => void; moveToAnnotatedHunk: (delta: number) => void; + moveToFile: (delta: number) => void; moveToHunk: (delta: number) => void; scrollToNote: boolean; selectedFile: DiffFile | undefined; @@ -229,6 +230,25 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon ], ); + /** Step through the visible files one at a time, clamped to the ends. */ + const moveToFile = useCallback( + (delta: number) => { + if (visibleFiles.length === 0) { + return; + } + const currentIndex = visibleFiles.findIndex((file) => file.id === selectedFile?.id); + const baseIndex = currentIndex >= 0 ? currentIndex : 0; + const nextIndex = clamp(baseIndex + delta, 0, visibleFiles.length - 1); + const nextFile = visibleFiles[nextIndex]; + if (!nextFile || nextFile.id === selectedFile?.id) { + return; + } + // Start the new file at its header so a file jump always lands at the top. + selectFile(nextFile.id, 0, { alignFileHeaderTop: true }); + }, + [selectFile, selectedFile?.id, visibleFiles], + ); + /** Cycle through only the currently visible files that carry annotations. */ const moveToAnnotatedFile = useCallback( (delta: number) => { @@ -262,6 +282,7 @@ export function useReviewController({ files }: { files: DiffFile[] }): ReviewCon clearFilter, moveToAnnotatedFile, moveToAnnotatedHunk, + moveToFile, moveToHunk, selectFile, selectFirstHunk, diff --git a/test/pty/ui-integration.test.ts b/test/pty/ui-integration.test.ts index 5fd2151..39a7feb 100644 --- a/test/pty/ui-integration.test.ts +++ b/test/pty/ui-integration.test.ts @@ -86,6 +86,89 @@ ptyDescribe("live UI integration", () => { } }); + test("comma and period jump between files 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 { + const initial = await session.waitForText(/Press \? for help/, { + timeout: ptyTimeouts.waitForText, + }); + + expect(initial).toContain("first.ts"); + expect(initial).toContain("line01 = 101"); + + // `.` moves to the next file (second.ts) and pins its header to the top. + await session.press("."); + const onSecond = await harness.waitForSnapshot( + session, + (text) => + text.includes("second.ts") && + text.includes("line17 = 117") && + harness.countMatches(text, /first\.ts/g) === 1, + ptyTimeouts.waitForSnapshot, + ); + expect(onSecond).toContain("line17 = 117"); + + // `,` moves back to the previous file (first.ts). + await session.press(","); + const backOnFirst = await harness.waitForSnapshot( + session, + (text) => text.includes("first.ts") && text.includes("line01 = 101"), + ptyTimeouts.waitForSnapshot, + ); + expect(backOnFirst).toContain("line01 = 101"); + } finally { + session.close(); + } + }); + + test("comma and period are inert while the filter is focused", async () => { + const fixture = harness.createPinnedHeaderRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 10, + }); + + try { + const initial = await session.waitForText(/Press \? for help/, { + timeout: ptyTimeouts.waitForText, + }); + expect(initial).toContain("line01 = 101"); + + await session.type("/"); + await harness.waitForSnapshot( + session, + (text) => text.includes("filter: type to filter files"), + ptyTimeouts.waitForSnapshot, + ); + + // `.` is a printable key with a nav meaning; while the filter owns the + // keyboard it must land in the filter query, not jump files. "." is a + // substring of both file names so the stream stays visible. + await session.type("."); + const filtered = await harness.waitForSnapshot( + session, + (text) => + text.includes("filter: .") && + text.includes("line01 = 101") && + !text.includes("line17 = 117"), + ptyTimeouts.waitForSnapshot, + ); + expect(filtered).toContain("filter: ."); + expect(filtered).not.toContain("line17 = 117"); + } finally { + session.close(); + } + }); + test("backward cross-file hunk navigation reveals the target hunk in a real PTY", async () => { const fixture = harness.createCrossFileHunkNavigationRepoFixture(); const session = await harness.launchHunk({