diff --git a/.changeset/goto-line-prompt.md b/.changeset/goto-line-prompt.md new file mode 100644 index 000000000..47bc09683 --- /dev/null +++ b/.changeset/goto-line-prompt.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Go to a line by typing its number: `:` opens a line prompt in the status bar, and Enter jumps the current line to it in the selected file, so `c` can annotate that line directly. Numbers use the current file's numbering (the note labels' `R` side); prefix with `l` (e.g. `:l42`) to target the source file's numbering (the `L` side). diff --git a/docs/keybindings.md b/docs/keybindings.md index 992e697bf..09545bffc 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -51,6 +51,7 @@ The built-in commands and the keys they ship with: | `hunk.review.alignCurrentLineTop` | Align current line to viewport top | _(none)_ | | `hunk.review.editSelectedFile` | Open the selected file in your editor | `e` | | `hunk.review.focusFilter` | Focus the file filter | `/` | +| `hunk.review.gotoLine` | Go to line | `:` | | `hunk.review.halfPageDown` | Scroll down half a page | `d` | | `hunk.review.halfPageUp` | Scroll up half a page | `u` | | `hunk.review.jumpToBottom` | Jump to end | `G`, `end` | diff --git a/examples/extensions/vim-navigation/README.md b/examples/extensions/vim-navigation/README.md index 074c0bc4d..1c9046fa9 100644 --- a/examples/extensions/vim-navigation/README.md +++ b/examples/extensions/vim-navigation/README.md @@ -29,13 +29,13 @@ cp -R examples/extensions/vim-navigation ~/.config/hunk/extensions/ | `zt` / `zz` / `zb` | Align the current line at the top/center/bottom | | `Ctrl-D` / `Ctrl-U` | Move down/up by half pages | | positive digits | Prefix the next relative motion, for example `5j` or `3]` | -| `:` | Open the host-rendered Vim command line | +| `;` | Open the host-rendered Vim command line | | `Esc` | Exit the mode (host-owned; the extension never receives it) | | everything else | Pass through to normal Hunk routing | Counts are parsed by the extension and capped at 10,000. Once a normal-mode sequence resolves, the extension calls `ctx.commands.execute(id, { count })` exactly once, so Hunk applies movement atomically. A bare `0` passes to Hunk's normal layout shortcut; `0` can extend a count that already began with `1`–`9`. -Pressing `:` passes the key to the example's registered command, which opens `ctx.dialogs.input()`. That focused host dialog captures typed keys ahead of the still-active session mode until Enter submits or Escape cancels. The deliberately small Ex-style command set is: +Pressing `;` passes the key to the example's registered command, which opens `ctx.dialogs.input()`. That focused host dialog captures typed keys ahead of the still-active session mode until Enter submits or Escape cancels. The deliberately small Ex-style command set is: | Command | Action | | --------- | ------------------------------- | diff --git a/examples/extensions/vim-navigation/index.ts b/examples/extensions/vim-navigation/index.ts index 7f84a7a2b..a28aace63 100644 --- a/examples/extensions/vim-navigation/index.ts +++ b/examples/extensions/vim-navigation/index.ts @@ -21,7 +21,7 @@ export default function (hunk: HunkExtensionAPI) { }); hunk.registerCommand( - { id: "command-line", title: "Open Vim command line", key: ":" }, + { id: "command-line", title: "Open Vim command line", key: ";" }, async (ctx) => { if (!ctx.keyboardModes.isActive("normal")) { ctx.notify("Enter Vim navigation before opening its command line", "info"); @@ -29,7 +29,7 @@ export default function (hunk: HunkExtensionAPI) { } const input = await ctx.dialogs.input({ - title: "Vim command (:)", + title: "Vim command (;)", placeholder: "top or bottom", }); if (input === null || !ctx.keyboardModes.isActive("normal")) return; diff --git a/examples/extensions/vim-navigation/state.ts b/examples/extensions/vim-navigation/state.ts index ac3b9114a..5852d9ba8 100644 --- a/examples/extensions/vim-navigation/state.ts +++ b/examples/extensions/vim-navigation/state.ts @@ -126,8 +126,8 @@ export function createVimNavigationState(commands: VimNavigationCommands) { return "handled"; } - // The registered `:` command opens a host dialog after this mode passes the key onward. - if (text === ":") { + // The registered `;` command opens a host dialog after this mode passes the key onward. + if (text === ";") { reset(); return "pass"; } diff --git a/scripts/vim-navigation-extension.test.ts b/scripts/vim-navigation-extension.test.ts index ae85f0e33..2b35ee7d4 100644 --- a/scripts/vim-navigation-extension.test.ts +++ b/scripts/vim-navigation-extension.test.ts @@ -72,11 +72,11 @@ describe("vim navigation example state", () => { expect(calls).toEqual([{ id: "hunk.review.stepDown", options: { count: 10_000 } }]); }); - test("passes colon to the registered command line and ignores unsupported modifiers", () => { + test("passes semicolon and unsupported modifiers through, resetting pending counts", () => { const { calls, state } = recordingState(); expect(state.handleKey({ sequence: "4" })).toBe("handled"); - expect(state.handleKey({ sequence: ":" })).toBe("pass"); + expect(state.handleKey({ sequence: ";" })).toBe("pass"); expect(state.handleKey({ meta: true, name: "j" })).toBe("pass"); expect(state.handleKey({ option: true, name: "k" })).toBe("pass"); expect(state.handleKey({ ctrl: true, option: true, name: "d" })).toBe("pass"); diff --git a/src/core/run/commandCatalog.ts b/src/core/run/commandCatalog.ts index 0011247a4..7a600c136 100644 --- a/src/core/run/commandCatalog.ts +++ b/src/core/run/commandCatalog.ts @@ -149,6 +149,17 @@ const BUILTIN_COMMANDS = [ locus: "client-local", publicToExtensions: true, }, + { + id: "hunk.review.gotoLine", + title: "Go to line", + category: "review", + defaultKeys: [":"], + // The command only opens this client's line-number prompt; submitting it + // resolves through the shared reveal path like any line navigation. + locus: "client-local", + publicToExtensions: true, + closesMenu: true, + }, { id: "hunk.review.startNote", title: "Add a review note", diff --git a/src/ui/App.tsx b/src/ui/App.tsx index 811aae0b4..bbd826932 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -3,7 +3,7 @@ import { type MouseEvent as TuiMouseEvent, type ScrollBoxRenderable, } from "@opentui/core"; -import { useRenderer, useTerminalDimensions } from "@opentui/react"; +import { flushSync, useRenderer, useTerminalDimensions } from "@opentui/react"; import { writeFile } from "node:fs/promises"; import { Suspense, @@ -140,7 +140,7 @@ import { resolveResponsiveLayout } from "./lib/responsive"; import { resizeSidebarWidth } from "./lib/sidebar"; import { availableThemes, resolveTheme, withTransparentSurfaces } from "./themes"; -type FocusArea = "files" | "filter" | "note"; +type FocusArea = "files" | "filter" | "goto" | "note"; type ActiveAddNoteTarget = ActiveAddNoteAffordance & { fileId: string }; type ThemeSelectorState = { open: boolean; @@ -331,6 +331,8 @@ export function App({ const [showAgentSkill, setShowAgentSkill] = useState(false); const [saveConfigPromptOpen, setSaveConfigPromptOpen] = useState(false); const [focusArea, setFocusArea] = useState("files"); + const [gotoLineText, setGotoLineText] = useState(""); + const gotoLineTextRef = useRef(""); const [activeAddNoteTarget, setActiveAddNoteTarget] = useState(null); const [paneSizes, setPaneSizes] = useState>({}); const [paneResize, setPaneResize] = useState<{ @@ -1170,6 +1172,7 @@ export function App({ }, [extensions, layoutMode, resolvedLayout]); const statusBarVisible = focusArea === "filter" || + focusArea === "goto" || Boolean(review.filter) || Boolean( sessionNoticeText ?? @@ -1404,7 +1407,9 @@ export function App({ /** Step one line: move the current line, or scroll the viewport when there is no marker. */ const stepDiffLine = (delta: number) => { - if (!activeLineCursor) { + // Cursor stepping needs the stop list; with `cursor_line = "off"` or a file view + // replacing the diff there is none, so fall back to plain scrolling. + if (!activeLineCursor || lineCursors.length === 0) { scrollDiff(delta, "step"); return; } @@ -1904,6 +1909,63 @@ export function App({ setFocusArea("filter"); }, []); + /** Focus the goto-line input in the status bar. */ + const focusGotoLine = useCallback(() => { + gotoLineTextRef.current = ""; + setGotoLineText(""); + // Keys of the same input chunk as the opener must already route to the + // prompt: without a synchronous flush the next character would still see + // the file-list focus and fire a command binding instead. + flushSync(() => setFocusArea("goto")); + }, []); + + /** Close the goto-line input without jumping. */ + const cancelGotoLine = useCallback(() => { + gotoLineTextRef.current = ""; + setGotoLineText(""); + setFocusArea("files"); + }, []); + + /** Keep the goto-line input to digits plus one leading side letter (l/r). */ + const handleGotoLineInput = useCallback((value: string) => { + const cleaned = value.replace(/[^0-9lr]/g, ""); + const next = /^[lr]/.test(cleaned) + ? cleaned[0]! + cleaned.slice(1).replace(/[lr]/g, "") + : cleaned.replace(/[lr]/g, ""); + gotoLineTextRef.current = next; + setGotoLineText(next); + }, []); + + /** Jump the current line to the typed line of the selected file; "l" targets the old side. */ + const submitGotoLine = useCallback(() => { + const text = gotoLineTextRef.current; + const side = text.startsWith("l") ? "old" : "new"; + const line = Number.parseInt(text.replace(/^[lr]/, ""), 10); + gotoLineTextRef.current = ""; + setGotoLineText(""); + setFocusArea("files"); + if (!Number.isInteger(line) || line <= 0) { + return; + } + + const fileId = selectedFileId ?? filteredFiles[0]?.id; + if (!fileId) { + showSessionNotice("No file selected"); + return; + } + + const result = review.revealLine(fileId, side, line); + if (result === "none") { + showSessionNotice( + `Line ${line}${side === "old" ? " (old side)" : ""} is not part of the diff`, + ); + } else if (result === "hunk") { + // The line cursor list is empty with `cursor_line = "off"`, so a line can + // degrade to its hunk — say so instead of presenting it as a precise jump. + showSessionNotice(`Line ${line} is not visible; jumped to its hunk`); + } + }, [review.revealLine, selectedFileId, filteredFiles, showSessionNotice]); + // Command-handler navigation lands here each render: the same focus and jump // semantics the sidebar's onSelect handlers use, so a command's navigation is // indistinguishable from a sidebar row click. Read through a ref because the @@ -1933,7 +1995,10 @@ export function App({ const startUserNote = useCallback( (fileId?: string, hunkIndex?: number, target?: UserNoteLineTarget) => { const hoverTarget = fileId === undefined ? activeAddNoteTarget : null; - const keyboardTarget = hoverTarget ?? (fileId === undefined ? activeLineCursor : null); + // Read the review cursor directly, not the display-gated active one: with + // `cursor_line = "off"` the marker stays hidden, but a line a goto reveal + // placed is still the note's intended target. + const keyboardTarget = hoverTarget ?? (fileId === undefined ? review.lineCursor : null); const draft = review.startUserNote( fileId ?? keyboardTarget?.fileId, hunkIndex ?? keyboardTarget?.hunkIndex, @@ -1945,7 +2010,7 @@ export function App({ setFocusArea("note"); } }, - [activeAddNoteTarget, activeLineCursor, review.startUserNote], + [activeAddNoteTarget, review.lineCursor, review.startUserNote], ); /** Mark the inline draft note textarea as the active keyboard input. */ @@ -2029,6 +2094,7 @@ export function App({ alignCurrentLine, applyFilePresentationToAllMatching, focusFilter, + focusGotoLine, moveSelection: review.moveSelection, openAgentSkill, openThemeSelector, @@ -2436,7 +2502,27 @@ export function App({ {statusBarVisible ? ( (review.filter.length > 0 ? review.setFilter("") : focusFiles()), + } + : focusArea === "goto" + ? { + label: "goto line:", + value: gotoLineText, + placeholder: "42 or l42 (old side)", + onInput: handleGotoLineInput, + onSubmit: submitGotoLine, + onEscape: cancelGotoLine, + } + : null + } modeText={keyboardModeHint ?? undefined} noticeText={ sessionNoticeText ?? transientNoticeText ?? noticeText ?? fileViewModeHint ?? undefined @@ -2444,8 +2530,6 @@ export function App({ terminalWidth={terminal.width} theme={activeTheme} onCloseMenu={closeMenu} - onFilterInput={review.setFilter} - onFilterSubmit={focusFiles} onExitMode={exitKeyboardMode} /> ) : null} diff --git a/src/ui/AppHost.interactions.test.tsx b/src/ui/AppHost.interactions.test.tsx index 3b62c08fb..e241cac78 100644 --- a/src/ui/AppHost.interactions.test.tsx +++ b/src/ui/AppHost.interactions.test.tsx @@ -2923,6 +2923,301 @@ describe("App interactions", () => { } }); + test("goto line works when the opener and digits arrive in one input chunk", async () => { + const setup = await testRender(, { + width: 160, + height: 40, + }); + + try { + await flush(setup); + + // Fast typing and terminal coalescing can deliver ":2" synchronously; the + // digit must land in the prompt rather than fire a file-list binding. + await act(async () => { + await setup.mockInput.typeText(":2"); + setup.mockInput.pressEnter(); + }); + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("Draft note - alpha.ts R2"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("goto line reports lines that are not part of the diff", async () => { + const setup = await testRender(, { + width: 160, + height: 40, + }); + + try { + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText(":"); + }); + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText("99"); + setup.mockInput.pressEnter(); + }); + await flush(setup); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("Line 99 is not part of the diff"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("goto line input strips non-digits and Escape cancels without jumping", async () => { + const setup = await testRender(, { + width: 160, + height: 40, + }); + + try { + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText(":"); + }); + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText("a1b"); + }); + await flush(setup); + + let frame = setup.captureCharFrame(); + expect(frame).toContain("goto line: 1"); + + await act(async () => { + setup.mockInput.pressEscape(); + }); + // A standalone Escape only parses into a key event once the input + // timeout fires, and the cancel commits a render cycle later. + let cancelled = false; + for (let attempt = 0; attempt < 10 && !cancelled; attempt++) { + await act(async () => { + await Bun.sleep(200); + await setup.renderOnce(); + }); + cancelled = !setup.captureCharFrame().includes("goto line:"); + } + expect(cancelled).toBe(true); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("goto line targets the new side by default and the old side with the l prefix", async () => { + const setup = await testRender(, { + width: 160, + height: 40, + }); + + const gotoAndAnnotate = async (input: string) => { + await act(async () => { + await setup.mockInput.typeText(`:${input}`); + setup.mockInput.pressEnter(); + }); + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + + const frame = setup.captureCharFrame(); + await act(async () => { + setup.mockInput.pressEscape(); + }); + // A standalone Escape needs the input timeout plus a render cycle. + for (let attempt = 0; attempt < 10; attempt++) { + await act(async () => { + await Bun.sleep(200); + await setup.renderOnce(); + }); + if (!setup.captureCharFrame().includes("Draft note")) { + break; + } + } + return frame; + }; + + try { + await flush(setup); + + // Bare numbers jump by the current file's numbering. + expect(await gotoAndAnnotate("1")).toContain("Draft note - alpha.ts R1"); + + // The l prefix jumps by the source file's numbering instead. + expect(await gotoAndAnnotate("l1")).toContain("Draft note - alpha.ts L1"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("goto line closes an open menu before taking input", async () => { + const setup = await testRender(, { + width: 160, + height: 40, + }); + + try { + await flush(setup); + + await act(async () => { + setup.mockInput.pressKey("F10"); + }); + await flush(setup); + expect(setup.captureCharFrame()).toContain("Reload"); + + await act(async () => { + await setup.mockInput.typeText(":"); + }); + await flush(setup); + + let frame = setup.captureCharFrame(); + expect(frame).toContain("goto line:"); + expect(frame).not.toContain("Reload"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("goto line jumps and annotates precisely when the line cursor is off", async () => { + const setup = await testRender( + , + { width: 160, height: 40 }, + ); + + try { + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText(":2"); + setup.mockInput.pressEnter(); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("Draft note - alpha.ts R2"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("goto line jumps inside the selected file, not the first visible one", async () => { + const setup = await testRender(, { + width: 160, + height: 40, + }); + + try { + await flush(setup); + + // Move the selection off the default first file before jumping. + await act(async () => { + await setup.mockInput.typeText("."); + }); + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText(":1"); + setup.mockInput.pressEnter(); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("c"); + }); + await flush(setup); + + const frame = setup.captureCharFrame(); + expect(frame).toContain("Draft note - beta.ts R1"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("filter prompt Escape clears the text first and closes second", async () => { + const setup = await testRender(, { + width: 160, + height: 40, + }); + + const pressEscapeAndWait = async (settled: (frame: string) => boolean) => { + await act(async () => { + setup.mockInput.pressEscape(); + }); + // A standalone Escape only parses into a key event once the input + // timeout fires, and the cancel commits a render cycle later. + for (let attempt = 0; attempt < 10; attempt++) { + await act(async () => { + await Bun.sleep(200); + await setup.renderOnce(); + }); + if (settled(setup.captureCharFrame())) { + return; + } + } + expect(settled(setup.captureCharFrame())).toBe(true); + }; + + try { + await flush(setup); + + await act(async () => { + await setup.mockInput.typeText("/"); + }); + await flush(setup); + await act(async () => { + await setup.mockInput.typeText("alpha"); + }); + await flush(setup); + expect(setup.captureCharFrame()).toContain("alpha"); + + // First Escape clears the filter text but keeps the prompt focused, + // leaving the placeholder where the text was. + await pressEscapeAndWait((frame) => frame.includes("type to filter files")); + + // Second Escape on the empty prompt closes it. + await pressEscapeAndWait((frame) => !frame.includes("filter:")); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("draft note saves Ctrl-S when tmux sends CSI-u input", async () => { const setup = await testRender(, { width: 240, diff --git a/src/ui/components/chrome/StatusBar.tsx b/src/ui/components/chrome/StatusBar.tsx index f05fa32dd..7e0bf0e21 100644 --- a/src/ui/components/chrome/StatusBar.tsx +++ b/src/ui/components/chrome/StatusBar.tsx @@ -3,28 +3,34 @@ import stringWidth from "string-width"; import { isEscapeKey } from "../../lib/keyboard"; import type { AppTheme } from "../../themes"; -/** Render the active file filter, transient notice, and persistent keyboard-mode badge. */ +/** One focused prompt input rendered inline in the status bar (file filter, goto line). */ +export interface StatusBarPromptInput { + label: string; + value: string; + placeholder?: string; + onInput: (value: string) => void; + onSubmit: () => void; + onEscape: () => void; +} + +/** Render the active prompt input, active file filter, transient notice, and mode badge. */ export function StatusBar({ filter, - filterFocused, + promptInput, modeText, noticeText, terminalWidth, theme, onCloseMenu, - onFilterInput, - onFilterSubmit, onExitMode, }: { filter: string; - filterFocused: boolean; + promptInput?: StatusBarPromptInput | null; modeText?: string; noticeText?: string; terminalWidth: number; theme: AppTheme; onCloseMenu: () => void; - onFilterInput: (value: string) => void; - onFilterSubmit: () => void; onExitMode?: () => void; }) { const modeWidth = modeText @@ -52,19 +58,19 @@ export function StatusBar({ flexDirection: "row", }} > - {filterFocused ? ( + {promptInput ? ( <> - filter: + {promptInput.label} { if (!isEscapeKey(key)) { return; @@ -72,20 +78,16 @@ export function StatusBar({ key.preventDefault(); key.stopPropagation(); - - if (filter.length > 0) { - onFilterInput(""); - return; - } - - onFilterSubmit(); + promptInput.onEscape(); }} /> + ) : noticeText ? ( + {noticeText} ) : filter.length > 0 ? ( {`filter=${filter}`} ) : ( - {noticeText ?? ""} + {""} )} {modeText ? ( diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 75bead05f..5d68aa273 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -2994,12 +2994,16 @@ describe("UI components", () => { const frame = await captureFrame( {}, + onSubmit: () => {}, + onEscape: () => {}, + }} terminalWidth={60} theme={theme} onCloseMenu={() => {}} - onFilterInput={() => {}} - onFilterSubmit={() => {}} />, 60, 3, @@ -3014,13 +3018,10 @@ describe("UI components", () => { const frame = await captureFrame( {}} - onFilterInput={() => {}} - onFilterSubmit={() => {}} />, 60, 3, @@ -3034,14 +3035,11 @@ describe("UI components", () => { const noticeFrame = await captureFrame( {}} - onFilterInput={() => {}} - onFilterSubmit={() => {}} onExitMode={() => {}} />, 80, @@ -3050,13 +3048,17 @@ describe("UI components", () => { const filterFrame = await captureFrame( {}, + onSubmit: () => {}, + onEscape: () => {}, + }} modeText="Vim navigation — ext vim:normal — Esc exits" terminalWidth={80} theme={theme} onCloseMenu={() => {}} - onFilterInput={() => {}} - onFilterSubmit={() => {}} onExitMode={() => {}} />, 80, @@ -3076,13 +3078,10 @@ describe("UI components", () => { let stopped = 0; const element = StatusBar({ filter: "", - filterFocused: false, modeText: "Vim navigation", terminalWidth: 80, theme, onCloseMenu: () => {}, - onFilterInput: () => {}, - onFilterSubmit: () => {}, onExitMode: () => { exits += 1; }, @@ -3106,13 +3105,17 @@ describe("UI components", () => { const frame = await captureFrame( {}, + onSubmit: () => {}, + onEscape: () => {}, + }} noticeText="Update available: 9.9.9 • npm i -g hunkdiff" terminalWidth={60} theme={theme} onCloseMenu={() => {}} - onFilterInput={() => {}} - onFilterSubmit={() => {}} />, 60, 3, @@ -3123,25 +3126,22 @@ describe("UI components", () => { expect(frame).not.toContain("Update available:"); }); - test("StatusBar keeps filter summary precedence over a notice", async () => { + test("StatusBar shows a transient notice over the filter summary", async () => { const theme = resolveTheme("github-dark-default", null); const frame = await captureFrame( {}} - onFilterInput={() => {}} - onFilterSubmit={() => {}} />, 60, 3, ); - expect(frame).toContain("filter=beta"); - expect(frame).not.toContain("Update available:"); + expect(frame).toContain("Update available: 9.9.9"); + expect(frame).not.toContain("filter=beta"); }); test("HelpDialog renders every documented control row without overlap", async () => { diff --git a/src/ui/hooks/useAppKeyboardShortcuts.ts b/src/ui/hooks/useAppKeyboardShortcuts.ts index ac288555c..a7bf42746 100644 --- a/src/ui/hooks/useAppKeyboardShortcuts.ts +++ b/src/ui/hooks/useAppKeyboardShortcuts.ts @@ -18,7 +18,7 @@ import { toExtensionKeyEvent } from "../lib/extensionKeyEvent"; import { isEscapeKey, isSaveDraftNoteKey } from "../lib/keyboard"; import { routeKeyOwnership, type KeyOwner } from "../lib/keyRouting"; -type FocusArea = "files" | "filter" | "note"; +type FocusArea = "files" | "filter" | "goto" | "note"; export interface UseAppKeyboardShortcutsOptions { activeMenuId: MenuId | null; @@ -490,6 +490,12 @@ export function useAppKeyboardShortcuts({ return "focused"; } + if (focusAreaRef.current === "goto") { + // The goto-line input owns every key: digits arrive through the renderable + // path, and its own Escape/Enter handling cancels or submits. + return "focused"; + } + if (focusAreaRef.current !== "note") { return "notMine"; } diff --git a/src/ui/hooks/useTerminalReview.test.tsx b/src/ui/hooks/useTerminalReview.test.tsx index e806af6fa..9c6c24719 100644 --- a/src/ui/hooks/useTerminalReview.test.tsx +++ b/src/ui/hooks/useTerminalReview.test.tsx @@ -1931,9 +1931,9 @@ describe("useTerminalReview", () => { } }); - test("falls back to the containing hunk when nothing measured a row for the line", async () => { - // With the current-line marker off the pane publishes no stops at all, so there is no - // measured row to scroll to; the hunk covering the line is the closest honest landing spot. + test("jumps precisely even when the pane publishes no line stops", async () => { + // With the current-line marker off the pane publishes no stops, but the target row is + // still measured: the reveal synthesizes the line's cursor and lands exactly on it. const { controllerRef, setup } = await renderTerminalReview([createThreeHunkFile()], { publishLineCursors: false, }); @@ -1948,10 +1948,14 @@ describe("useTerminalReview", () => { }); await flush(setup); - expect(outcome).toBe("hunk"); + expect(outcome).toBe("line"); + expect(expectValue(controllerRef.current).lineCursor).toMatchObject({ + fileId: "alpha", + hunkIndex: 1, + target: { side: "new", line: 15 }, + }); expect(expectValue(controllerRef.current).selectedHunkIndex).toBe(1); - // A hunk selection reveal, not a line reveal: nothing measured the requested row. - expect(expectValue(controllerRef.current).lineCursorRevealRequest.id).toBe(before.id); + expect(expectValue(controllerRef.current).lineCursorRevealRequest.id).toBe(before.id + 1); } finally { await act(async () => { setup.renderer.destroy(); @@ -2324,7 +2328,7 @@ describe("useTerminalReview", () => { } }); - test("navigate line targets fall back to the hunk when no row is measured", async () => { + test("navigate line targets land precisely when the pane publishes no stops", async () => { const { controllerRef, setup } = await renderTerminalReview([createTwoHunkFile()], { publishLineCursors: false, }); @@ -2342,7 +2346,11 @@ describe("useTerminalReview", () => { }); await flush(setup); - expect(result).toMatchObject({ hunkIndex: 1, revealed: "hunk" }); + expect(result).toMatchObject({ hunkIndex: 1, revealed: "line" }); + expect(expectValue(controllerRef.current).lineCursor).toMatchObject({ + fileId: "alpha", + target: { side: "new", line: 12 }, + }); expect(expectValue(controllerRef.current).selectedHunkIndex).toBe(1); } finally { await act(async () => { diff --git a/src/ui/hooks/useTerminalReview.ts b/src/ui/hooks/useTerminalReview.ts index 41dc9e7ea..07252899a 100644 --- a/src/ui/hooks/useTerminalReview.ts +++ b/src/ui/hooks/useTerminalReview.ts @@ -604,6 +604,20 @@ export function useTerminalReview({ ); const reconcileLineCursor = useCallback(() => { + // A hidden line-level navigation (`cursor_line = "off"`) keeps no stop list, so there + // is nothing to resolve against: keep a cursor a direct reveal placed on the selected + // hunk, and drop it once the selection moves somewhere else. + if (lineCursors.length === 0) { + const current = lineCursorRef.current; + if ( + current !== null && + (current.fileId !== selectedFileId || current.hunkIndex !== selectedHunkIndex) + ) { + applyLineCursor(null); + } + return; + } + // Expansion remeasures before its source text loads, so a toggle records what it wants and // this waits for the list that actually carries the revealed rows. Each request survives until // it resolves or the next toggle replaces it. @@ -711,9 +725,10 @@ export function useTerminalReview({ */ const revealLine = useCallback( (fileId: string, side: "old" | "new", line: number): RevealedLineResult => { - const cursor = findLineCursorAt(lineCursorsForRevealRef.current, fileId, side, line); - if (cursor) { - revealLineCursor(cursor, "reveal"); + const cursors = lineCursorsForRevealRef.current; + const stopped = findLineCursorAt(cursors, fileId, side, line); + if (stopped) { + revealLineCursor(stopped, "reveal"); return "line"; } @@ -723,6 +738,13 @@ export function useTerminalReview({ return "none"; } + // An empty list means line-level navigation is hidden (`cursor_line = "off"`), not + // that the target row is missing: synthesize the line's cursor and reveal it exactly. + if (cursors.length === 0) { + revealLineCursor(lineCursorAt(cursors, fileId, hunkIndex, { side, line }), "reveal"); + return "line"; + } + selectHunk(fileId, hunkIndex); return "hunk"; }, diff --git a/src/ui/lib/appCommands.test.ts b/src/ui/lib/appCommands.test.ts index 188538cfa..dab7eca61 100644 --- a/src/ui/lib/appCommands.test.ts +++ b/src/ui/lib/appCommands.test.ts @@ -49,6 +49,7 @@ function createTestCommands(resolvedKeys?: ResolvedCommandKeys) { alignCurrentLine: record("alignCurrentLine"), applyFilePresentationToAllMatching: record("applyFilePresentationToAllMatching"), focusFilter: record("focusFilter"), + focusGotoLine: record("focusGotoLine"), moveSelection: record("moveSelection"), openAgentSkill: record("openAgentSkill"), openThemeSelector: record("openThemeSelector"), diff --git a/src/ui/lib/appCommands.ts b/src/ui/lib/appCommands.ts index 87fed7b70..b49569649 100644 --- a/src/ui/lib/appCommands.ts +++ b/src/ui/lib/appCommands.ts @@ -117,6 +117,7 @@ export interface BuildAppCommandsOptions { alignCurrentLine: (alignment: "top" | "center" | "bottom") => void; applyFilePresentationToAllMatching: () => void; focusFilter: () => void; + focusGotoLine: () => void; /** Step the review selection through one scope, as the catalog entry declares it. */ moveSelection: (scope: ReviewSelectionScope, delta: number) => void; openAgentSkill: () => void; @@ -181,6 +182,7 @@ function builtinCommandHandlers( "hunk.app.openAgentSkill": { run: () => options.openAgentSkill() }, "hunk.app.toggleFocusArea": { run: () => options.toggleFocusArea() }, "hunk.review.focusFilter": { run: () => options.focusFilter() }, + "hunk.review.gotoLine": { run: () => options.focusGotoLine() }, "hunk.review.startNote": { run: () => options.startUserNote() }, "hunk.review.pageDown": { run: (_key, count) => options.scrollDiff(count, "viewport") }, "hunk.review.pageUp": { run: (_key, count) => options.scrollDiff(-count, "viewport") }, @@ -317,6 +319,7 @@ const NOOP_COMMAND_OPTIONS: BuildAppCommandsOptions = (() => { alignCurrentLine: noop, applyFilePresentationToAllMatching: noop, focusFilter: noop, + focusGotoLine: noop, moveSelection: noop, openAgentSkill: noop, openThemeSelector: noop, diff --git a/src/ui/lib/appMenus.test.ts b/src/ui/lib/appMenus.test.ts index a738816da..fc4148f01 100644 --- a/src/ui/lib/appMenus.test.ts +++ b/src/ui/lib/appMenus.test.ts @@ -43,6 +43,7 @@ function createTestCommands(overrides: Partial = {}) { alignCurrentLine: record("alignCurrentLine"), applyFilePresentationToAllMatching: record("applyFilePresentationToAllMatching"), focusFilter: noop, + focusGotoLine: noop, moveSelection: record("moveSelection"), openAgentSkill: record("openAgentSkill"), openThemeSelector: noop, @@ -147,7 +148,7 @@ describe("buildAppMenus", () => { "Next annotated file", "Previous annotated file", ]); - expect(items(menus.navigate).map((item) => item.hint)).toEqual(["[", "]", "{", "}", "/"]); + expect(items(menus.navigate).map((item) => item.hint)).toEqual(["[", "]", ":", "/", "{", "}"]); }); test("every item carries the id of the command it runs", () => { diff --git a/src/ui/lib/appMenus.ts b/src/ui/lib/appMenus.ts index 0614b6c00..3976bdc51 100644 --- a/src/ui/lib/appMenus.ts +++ b/src/ui/lib/appMenus.ts @@ -193,10 +193,11 @@ export function buildAppMenus({ { commandId: "hunk.review.previousHunk" }, { commandId: "hunk.review.nextHunk" }, SEPARATOR, + { commandId: "hunk.review.gotoLine", label: "Go to line…" }, + { commandId: "hunk.review.focusFilter", label: "Focus filter" }, + SEPARATOR, { commandId: "hunk.review.previousAnnotatedHunk", label: "Previous comment" }, { commandId: "hunk.review.nextAnnotatedHunk", label: "Next comment" }, - SEPARATOR, - { commandId: "hunk.review.focusFilter", label: "Focus filter" }, ], agent: [ { commandId: "hunk.view.toggleAgentNotes", label: "Agent notes", checked: showAgentNotes }, diff --git a/test/pty/cursor-line.test.ts b/test/pty/cursor-line.test.ts index 0248316b6..bc61a9d68 100644 --- a/test/pty/cursor-line.test.ts +++ b/test/pty/cursor-line.test.ts @@ -305,4 +305,31 @@ describe("PTY current line", () => { session.close(); } }); + + test("goto line jumps to a typed line and anchors a note there", async () => { + const fixture = harness.createMultiHunkFilePair(); + const session = await harness.launchHunk({ + args: ["diff", fixture.before, fixture.after, "--mode", "split"], + cols: 120, + rows: 24, + }); + + try { + await session.waitForText(/View\s+Navigate\s+Agent\s+Help/, { + timeout: 15_000, + }); + + await session.press(":"); + await session.waitForText(/goto line:/, { timeout: 5_000 }); + + await session.type("62"); + await session.type("\r"); + + await session.press("c"); + const draft = await session.waitForText(/Draft note/, { timeout: 5_000 }); + expect(draft).toMatch(/Draft note - .*after\.ts R62/); + } finally { + session.close(); + } + }); }); diff --git a/test/pty/extensions-integration.test.ts b/test/pty/extensions-integration.test.ts index 05a4d6240..4e96d8e0e 100644 --- a/test/pty/extensions-integration.test.ts +++ b/test/pty/extensions-integration.test.ts @@ -734,20 +734,20 @@ describe("PTY extensions", () => { const centered = await session.text({ immediate: true }); expect(lineIndexOf(centered, "export const line11 = 11;")).toBeGreaterThan(topAlignedRow); - // `:` passes into the registered command, whose focused host dialog owns even mode keys. - await session.press(":"); - await session.waitForText(/Vim command \(:\)/, { timeout: 20_000 }); + // `;` passes into the registered command, whose focused host dialog owns even mode keys. + await session.press(";"); + await session.waitForText(/Vim command \(;\)/, { timeout: 20_000 }); await session.type("j-owned"); await session.waitForText(/j-owned/, { timeout: 20_000 }); await session.press("escape"); await harness.waitForSnapshot( session, - (text) => !text.includes("Vim command (:)") && /Vim navigation.*Esc exits/.test(text), + (text) => !text.includes("Vim command (;)") && /Vim navigation.*Esc exits/.test(text), 20_000, ); - await session.press(":"); - await session.waitForText(/Vim command \(:\)/, { timeout: 20_000 }); + await session.press(";"); + await session.waitForText(/Vim command \(;\)/, { timeout: 20_000 }); await session.type("bottom"); await session.press("enter"); const commandBottom = await harness.waitForSnapshot( @@ -757,8 +757,8 @@ describe("PTY extensions", () => { ); expect(commandBottom).toContain("second.ts"); - await session.press(":"); - await session.waitForText(/Vim command \(:\)/, { timeout: 20_000 }); + await session.press(";"); + await session.waitForText(/Vim command \(;\)/, { timeout: 20_000 }); await session.type("top"); await session.press("enter"); const commandTop = await harness.waitForSnapshot(