diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0a16d8af..5bc8d1fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: run: bun run typecheck - name: Test suite - run: bun test + run: bun run test - name: PTY integration tests run: bun run test:integration diff --git a/.github/workflows/pr-ci.yml b/.github/workflows/pr-ci.yml index 3ad7332a..47c6fad7 100644 --- a/.github/workflows/pr-ci.yml +++ b/.github/workflows/pr-ci.yml @@ -43,7 +43,7 @@ jobs: run: bun run typecheck - name: Test suite - run: bun test + run: bun run test - name: PTY integration tests run: bun run test:integration diff --git a/package.json b/package.json index 383f6dd6..1a919e0d 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "lint": "oxlint . --deny-warnings", "lint:fix": "oxlint . --fix", "prepare": "simple-git-hooks", - "test": "\"${npm_execpath:-bun}\" test", + "test": "\"${npm_execpath:-bun}\" test ./src ./scripts ./test/cli ./test/session", "test:integration": "\"${npm_execpath:-bun}\" test ./test/pty", "test:tty-smoke": "HUNK_RUN_TTY_SMOKE=1 \"${npm_execpath:-bun}\" test ./test/smoke", "check:pack": "bun run ./scripts/check-pack.ts", diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 74f751ee..41c7c37f 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -122,7 +122,6 @@ export function App({ const filteredFiles = review.visibleFiles; const selectedFile = review.selectedFile; const selectedHunkIndex = review.selectedHunkIndex; - const clearFilter = review.clearFilter; const moveToAnnotatedFile = review.moveToAnnotatedFile; const moveToAnnotatedHunk = review.moveToAnnotatedHunk; @@ -534,13 +533,10 @@ export function App({ activeMenuId, activateCurrentMenuItem, canRefreshCurrentInput, - clearFilter, closeHelp, closeMenu, cycleTheme, - filter: review.filter, focusArea, - focusFiles, focusFilter, moveToAnnotatedHunk, moveToHunk: review.moveToHunk, diff --git a/src/ui/components/chrome/StatusBar.tsx b/src/ui/components/chrome/StatusBar.tsx index 4ade321f..a5bda04f 100644 --- a/src/ui/components/chrome/StatusBar.tsx +++ b/src/ui/components/chrome/StatusBar.tsx @@ -1,3 +1,4 @@ +import { isEscapeKey } from "../../lib/keyboard"; import type { AppTheme } from "../../themes"; /** Render the active file filter input or current filter summary. */ @@ -45,6 +46,21 @@ export function StatusBar({ focused={true} onInput={onFilterInput} onSubmit={onFilterSubmit} + onKeyDown={(key) => { + if (!isEscapeKey(key)) { + return; + } + + key.preventDefault(); + key.stopPropagation(); + + if (filter.length > 0) { + onFilterInput(""); + return; + } + + onFilterSubmit(); + }} /> ) : filter.length > 0 ? ( diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index 6f7545da..d82bead1 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -1,8 +1,10 @@ import type { KeyEvent } from "@opentui/core"; import { useKeyboard } from "@opentui/react"; +import { useRef } from "react"; import type { LayoutMode } from "../../core/types"; import type { MenuId } from "../components/chrome/menu"; import { + isEscapeKey, isHalfPageDownKey, isHalfPageUpKey, isPageDownKey, @@ -21,13 +23,10 @@ export interface UseAppKeyboardShortcutsOptions { activeMenuId: MenuId | null; activateCurrentMenuItem: () => void; canRefreshCurrentInput: boolean; - clearFilter: () => void; closeHelp: () => void; closeMenu: () => void; cycleTheme: () => void; - filter: string; focusArea: FocusArea; - focusFiles: () => void; focusFilter: () => void; moveToAnnotatedHunk: (delta: number) => void; moveToHunk: (delta: number) => void; @@ -55,13 +54,10 @@ export function useAppKeyboardShortcuts({ activeMenuId, activateCurrentMenuItem, canRefreshCurrentInput, - clearFilter, closeHelp, closeMenu, cycleTheme, - filter, focusArea, - focusFiles, focusFilter, moveToAnnotatedHunk, moveToHunk, @@ -83,6 +79,16 @@ export function useAppKeyboardShortcuts({ toggleSidebar, triggerRefreshCurrentInput, }: UseAppKeyboardShortcutsOptions) { + const activeMenuIdRef = useRef(activeMenuId); + const focusAreaRef = useRef(focusArea); + const pagerModeRef = useRef(pagerMode); + const showHelpRef = useRef(showHelp); + + activeMenuIdRef.current = activeMenuId; + focusAreaRef.current = focusArea; + pagerModeRef.current = pagerMode; + showHelpRef.current = showHelp; + const runAndCloseMenu = (action: () => void) => { action(); closeMenu(); @@ -93,11 +99,11 @@ export function useAppKeyboardShortcuts({ return false; } - if (pagerMode) { + if (pagerModeRef.current) { return true; } - if (activeMenuId) { + if (activeMenuIdRef.current) { closeMenu(); } else { openMenu("file"); @@ -107,7 +113,7 @@ export function useAppKeyboardShortcuts({ }; const handlePagerShortcut = (key: KeyEvent) => { - if (key.name === "q" || key.name === "escape") { + if (key.name === "q" || isEscapeKey(key)) { requestQuit(); return; } @@ -168,7 +174,7 @@ export function useAppKeyboardShortcuts({ }; const handleHelpShortcut = (key: KeyEvent) => { - if (!showHelp || key.name !== "escape") { + if (!showHelpRef.current || !isEscapeKey(key)) { return false; } @@ -177,11 +183,11 @@ export function useAppKeyboardShortcuts({ }; const handleMenuShortcut = (key: KeyEvent) => { - if (!activeMenuId) { + if (!activeMenuIdRef.current) { return false; } - if (key.name === "escape") { + if (isEscapeKey(key)) { closeMenu(); return true; } @@ -215,26 +221,16 @@ export function useAppKeyboardShortcuts({ }; const handleFilterShortcut = (key: KeyEvent) => { - if (focusArea !== "filter") { + if (focusAreaRef.current !== "filter") { return false; } - if (key.name === "escape") { - if (filter.length > 0) { - clearFilter(); - return true; - } - - focusFiles(); - return true; - } - if (key.name === "tab") { toggleFocusArea(); return true; } - // Let the input widget own typing while the filter is focused. + // Let the focused input own filter editing and escape handling. return true; }; @@ -244,13 +240,13 @@ export function useAppKeyboardShortcuts({ return; } - if (key.name === "?") { + if (key.name === "?" || key.sequence === "?") { toggleHelp(); closeMenu(); return; } - if (key.name === "escape") { + if (isEscapeKey(key)) { requestQuit(); return; } @@ -390,7 +386,7 @@ export function useAppKeyboardShortcuts({ return; } - if (pagerMode) { + if (pagerModeRef.current) { handlePagerShortcut(key); return; } diff --git a/src/ui/lib/keyboard.ts b/src/ui/lib/keyboard.ts index 4ac33dbc..8bb81e83 100644 --- a/src/ui/lib/keyboard.ts +++ b/src/ui/lib/keyboard.ts @@ -4,6 +4,11 @@ function isSpaceKey(key: KeyEvent) { return key.name === "space" || key.name === " " || key.sequence === " "; } +/** Normalize the escape key aliases emitted by different terminal input paths. */ +export function isEscapeKey(key: KeyEvent) { + return key.name === "escape" || key.name === "esc"; +} + /** Match any key alias that should scroll forward by a full viewport. */ export function isPageDownKey(key: KeyEvent) { return ( diff --git a/src/ui/lib/ui-lib.test.ts b/src/ui/lib/ui-lib.test.ts index a2954ed1..5f549924 100644 --- a/src/ui/lib/ui-lib.test.ts +++ b/src/ui/lib/ui-lib.test.ts @@ -12,6 +12,7 @@ import { import { buildAgentPopoverContent, resolveAgentPopoverPlacement, wrapText } from "./agentPopover"; import { buildAppMenus } from "./appMenus"; import { + isEscapeKey, isHalfPageDownKey, isHalfPageUpKey, isPageDownKey, @@ -171,6 +172,8 @@ describe("ui helpers", () => { }); test("keyboard alias helpers normalize the shared scroll shortcut keys", () => { + expect(isEscapeKey(createKeyEvent({ name: "escape" }))).toBe(true); + expect(isEscapeKey(createKeyEvent({ name: "esc" }))).toBe(true); expect(isPageDownKey(createKeyEvent({ name: "pagedown" }))).toBe(true); expect(isPageDownKey(createKeyEvent({ name: "space" }))).toBe(true); expect(isPageDownKey(createKeyEvent({ name: "f" }))).toBe(true); @@ -185,6 +188,7 @@ describe("ui helpers", () => { expect(isStepDownKey(createKeyEvent({ sequence: "j" }))).toBe(true); expect(isStepUpKey(createKeyEvent({ name: "up" }))).toBe(true); expect(isStepUpKey(createKeyEvent({ sequence: "k" }))).toBe(true); + expect(isEscapeKey(createKeyEvent({ name: "q" }))).toBe(false); expect(isPageDownKey(createKeyEvent({ name: "space", shift: true }))).toBe(false); expect(isPageDownKey(createKeyEvent({ name: "q" }))).toBe(false); expect(isShiftSpacePageUpKey(createKeyEvent({ name: "space", shift: false }))).toBe(false); diff --git a/test/pty/ui-integration.test.ts b/test/pty/ui-integration.test.ts index d32f1eba..b25fa964 100644 --- a/test/pty/ui-integration.test.ts +++ b/test/pty/ui-integration.test.ts @@ -393,6 +393,49 @@ describe("live UI integration", () => { } }); + test("slash focuses the filter and narrows the visible review stream", async () => { + const fixture = harness.createSidebarJumpRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 12, + }); + + try { + const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + expect(initial).toContain("alphaOnly = true"); + expect(initial).toContain("betaValue = 2"); + + await session.type("/"); + await harness.waitForSnapshot( + session, + (text) => text.includes("filter: type to filter files"), + 5_000, + ); + + await session.type("delta"); + const filtered = await harness.waitForSnapshot( + session, + (text) => + text.includes("filter: delta") && + text.includes("deltaOnly = true") && + !text.includes("alphaOnly = true"), + 5_000, + ); + + expect(filtered.toLowerCase()).toContain("filter"); + expect(filtered).toContain("delta"); + expect(filtered).toContain("deltaOnly = true"); + expect(filtered).not.toContain("alphaOnly = true"); + } finally { + session.close(); + } + }); + test("pager mode hides chrome and pages forward on space", async () => { const fixture = harness.createPagerPatchFixture(); const session = await harness.launchHunk({ @@ -422,6 +465,110 @@ describe("live UI integration", () => { } }); + test("pager mode handles half-page, page-up, and content-jump keyboard navigation", async () => { + const fixture = harness.createPagerPatchFixture(60); + const session = await harness.launchHunk({ + args: ["patch", fixture.patchFile, "--pager"], + cols: 120, + rows: 12, + }); + + try { + const initial = await session.waitForText(/scroll\.ts/, { timeout: 15_000 }); + + expect(initial).toContain("before_01"); + expect(initial).not.toContain("before_12"); + + await session.press("d"); + const halfPaged = await harness.waitForSnapshot( + session, + (text) => !text.includes("before_01"), + 5_000, + ); + + expect(halfPaged).not.toContain("before_01"); + + await session.press("u"); + const halfPageRestored = await harness.waitForSnapshot( + session, + (text) => text.includes("before_01"), + 5_000, + ); + + expect(halfPageRestored).toContain("before_01"); + + await session.press("space"); + const paged = await harness.waitForSnapshot( + session, + (text) => text.includes("before_18") || text.includes("after_02"), + 5_000, + ); + + expect(paged.includes("before_18") || paged.includes("after_02")).toBe(true); + + await session.press("b"); + const pageRestored = await harness.waitForSnapshot( + session, + (text) => text.includes("before_01") && !text.includes("after_02"), + 5_000, + ); + + expect(pageRestored).toContain("before_01"); + expect(pageRestored).not.toContain("after_02"); + + await session.press("end"); + const bottom = await harness.waitForSnapshot( + session, + (text) => text.includes("after_60"), + 5_000, + ); + + expect(bottom).toContain("after_60"); + + await session.press("home"); + const top = await harness.waitForSnapshot( + session, + (text) => text.includes("before_01") && !text.includes("after_60"), + 5_000, + ); + + expect(top).toContain("before_01"); + expect(top).not.toContain("after_60"); + } finally { + session.close(); + } + }); + + test("keyboard help can open with ? in a real PTY", async () => { + const fixture = harness.createTwoFileRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 24, + }); + + try { + await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + await session.press("?"); + const help = await harness.waitForSnapshot( + session, + (text) => + (text.includes("Keyboard help") || text.includes("Controls help")) && + text.includes("move line-by-line"), + 5_000, + ); + + expect(help.includes("Keyboard help") || help.includes("Controls help")).toBe(true); + expect(help).toContain("move line-by-line"); + } finally { + session.close(); + } + }); + test("mouse menu navigation can switch the diff layout", async () => { const fixture = harness.createTwoFileRepoFixture(); const session = await harness.launchHunk({ @@ -463,6 +610,104 @@ describe("live UI integration", () => { } }); + test("keyboard menu navigation can switch layouts in a real PTY", async () => { + const fixture = harness.createTwoFileRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "split"], + cwd: fixture.dir, + cols: 220, + rows: 24, + }); + + try { + const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + expect(initial).toMatch(/▌.*▌/); + + await session.press("f10"); + const fileMenu = await harness.waitForSnapshot( + session, + (text) => text.includes("Toggle files/filter focus") && text.includes("Quit"), + 5_000, + ); + + expect(fileMenu).toContain("Reload"); + + await session.press("right"); + const viewMenu = await harness.waitForSnapshot( + session, + (text) => text.includes("Split view") && text.includes("Stacked view"), + 5_000, + ); + + expect(viewMenu).toContain("Auto layout"); + + await session.press("down"); + await session.press("enter"); + const stacked = await harness.waitForSnapshot( + session, + (text) => !/▌.*▌/.test(text) && text.includes("1 - export const alpha = 1;"), + 5_000, + ); + + expect(stacked).not.toMatch(/▌.*▌/); + expect(stacked).toContain("1 - export const alpha = 1;"); + } finally { + session.close(); + } + }); + + test("direct layout hotkeys can switch between split, stack, and auto in a real PTY", async () => { + const fixture = harness.createTwoFileRepoFixture(); + const session = await harness.launchHunk({ + args: ["diff", "--mode", "stack"], + cwd: fixture.dir, + cols: 220, + rows: 24, + }); + + try { + const initial = await session.waitForText(/View\s+Navigate\s+Theme\s+Agent\s+Help/, { + timeout: 15_000, + }); + + expect(initial).not.toMatch(/▌.*▌/); + expect(initial).toContain("1 - export const alpha = 1;"); + + await session.press("1"); + const split = await harness.waitForSnapshot( + session, + (text) => /▌.*▌/.test(text) && harness.countMatches(text, /alpha\.ts/g) >= 2, + 5_000, + ); + + expect(split).toMatch(/▌.*▌/); + + await session.press("2"); + const stack = await harness.waitForSnapshot( + session, + (text) => !/▌.*▌/.test(text) && text.includes("1 - export const alpha = 1;"), + 5_000, + ); + + expect(stack).not.toMatch(/▌.*▌/); + expect(stack).toContain("1 - export const alpha = 1;"); + + await session.press("0"); + const auto = await harness.waitForSnapshot( + session, + (text) => /▌.*▌/.test(text) && harness.countMatches(text, /alpha\.ts/g) >= 2, + 5_000, + ); + + expect(auto).toMatch(/▌.*▌/); + } finally { + session.close(); + } + }); + test("mouse wheel scrolling moves the review pane", async () => { const fixture = harness.createScrollableFilePair(); const session = await harness.launchHunk({