diff --git a/.changeset/reveal-line-navigation.md b/.changeset/reveal-line-navigation.md new file mode 100644 index 000000000..559ea3c51 --- /dev/null +++ b/.changeset/reveal-line-navigation.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": minor +--- + +Extensions can jump the review to one exact source line with `ctx.navigation.revealLine(fileId, side, line)` (API v5), so a target deep inside a tall hunk lands near the top of the viewport instead of pages below its anchor. diff --git a/docs/extension-architecture.md b/docs/extension-architecture.md index cda3c794b..1c35a7169 100644 --- a/docs/extension-architecture.md +++ b/docs/extension-architecture.md @@ -168,6 +168,17 @@ chord at a time and detected by probing matchers with a synthesized event `src/ui/lib/extensionSelection.ts`, derived from the same frozen file views the panes render. App reads it through a ref so the dispatch table stays stable. +`src/ui/lib/extensionNavigation.ts` mints the guarded navigation behind both +`ctx.navigation` and a pane's `actions`, so a jump from either surface is +validated, attributed, and reported the same way. It owns argument policy only +— visible-file validation, hunk clamping, `revealLine`'s side and line-number +checks — and delegates the move itself to the review controller. Where a jump +puts a line on screen stays host policy: `useReviewController` tags each +current-line reveal with a placement, and `DiffPane` reads it to choose between +stepping's minimum-distance scroll and the top-padded position hunk, note, and +`revealLine` reveals share. Extensions name a target; they never name a scroll +position. + `ctx.dialogs` is the one place extension code can interrupt the user, so its ordering and settlement live outside React in `src/ui/lib/extensionDialogs.ts` — one FIFO queue per App instance, minting a diff --git a/docs/extensions.md b/docs/extensions.md index 8bc8d72d6..7018406bb 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -279,8 +279,9 @@ new instances and run that shutdown/startup pair around the replacement. ### `hunk.apiVersion` The API generation this Hunk speaks (currently `5`). Version 5 adds line -highlighters; version 4 added keyboard modes and docked panes, with API-v3 -sidebar names remaining as deprecated aliases. +highlighters and line-granular navigation (`revealLine`); version 4 added +keyboard modes and docked panes, with API-v3 sidebar names remaining as +deprecated aliases. ### `hunk.registerTheme(theme)` @@ -648,13 +649,16 @@ The component receives fresh props as the app changes: API-v3 sidebar names remain as deprecated aliases: use `registerPane`, `ExtensionPane*`, `ctx.panes`, and `replaces: "hunk:files"` in new code. -`actions.selectFile(fileId)` and `actions.selectHunk(fileId, hunkIndex)` route -through the same review controller as the built-in files pane and the keyboard -shortcuts, so the review stream scrolls, selection updates, and the -`selection_changed` event fires exactly as if the user had clicked a built-in -row. `actions.notify(message, type?)` shows a toast attributed to your -extension. An action given a file id that is not currently visible is refused -with a warning rather than corrupting the selection. +`actions.selectFile(fileId)`, `actions.selectHunk(fileId, hunkIndex)`, and +`actions.revealLine(fileId, side, line)` route through the same review +controller as the built-in files pane and the keyboard shortcuts, so the review +stream scrolls, selection updates, and the `selection_changed` event fires +exactly as if the user had clicked a built-in row. `actions.notify(message, +type?)` shows a toast attributed to your extension. An action given a file id +that is not currently visible is refused with a warning rather than corrupting +the selection. A pane's `actions` carry the same navigation methods a command +handler's [`ctx.navigation`](#navigating-the-review) does, with the same +guarantees. The three hunk surfaces line up by design: each file's `hunks` lists public `ExtensionDiffHunk` summaries (`index`, the `@@` header, inclusive old/new @@ -1358,15 +1362,49 @@ session keyboard modes. See [Session keyboard modes](#session-keyboard-modes). `{ fileId }`-scoped. See [`hunk.registerLineHighlighter`](#hunkregisterlinehighlighterhighlighter). -`ctx.navigation` moves the review stream: `selectFile(fileId)` and -`selectHunk(fileId, hunkIndex)`, the same guarded navigation a pane's -`actions` carry, routed through the same review controller — the stream -scrolls, selection updates, and `selection_changed` fires exactly as if the -user had clicked a pane row. Unlike `selection` it is live, not a snapshot: -a call acts on the review as it is at that moment, so a handler that awaits a -dialog and then navigates still works. A file id the stream cannot currently -show is refused with a warning rather than corrupting the selection, and a -hunk index is clamped into the file's real range. +#### Navigating the review + +`ctx.navigation` moves the review stream: `selectFile(fileId)`, +`selectHunk(fileId, hunkIndex)`, and `revealLine(fileId, side, line)`, the same +guarded navigation a pane's `actions` carry, routed through the same review +controller — the stream scrolls, selection updates, and `selection_changed` +fires exactly as if the user had clicked a pane row. Unlike `selection` it is +live, not a snapshot: a call acts on the review as it is at that moment, so a +handler that awaits a dialog and then navigates still works. A file id the +stream cannot currently show is refused with a warning rather than corrupting +the selection, and a hunk index is clamped into the file's real range. + +`revealLine` is the finest target there is, and the one to reach for when your +extension knows exactly which line it means — a search hit, a lint finding, the +line a mark from [`registerLineHighlighter`](#hunkregisterlinehighlighterhighlighter) +sits on. A hunk hundreds of lines tall has one anchor, so `selectHunk` can leave +the line you meant pages below the viewport; `revealLine` scrolls to the line +itself, lands it a little below the viewport top like every other Hunk reveal, +and makes it the current line so the reverse-video marker sits on it. + +`line` is 1-based on `side` as the patch numbers it, so a context line answers +to either side's number. Two things soften the target rather than failing it: +when no rendered row carries that line — it is inside a collapsed gap, absent +from a partial patch, or the reviewer turned the current-line marker off +(`view.cursor_line = "off"`) — the jump lands on the hunk containing the line +instead. Only a line no hunk of the file covers is refused, with a warning +naming your extension, and so are a side outside `"old"`/`"new"` and a line +number that is not a positive whole number. + +```ts +hunk.registerCommand({ id: "first-todo", title: "Jump to the first TODO" }, async (ctx) => { + const file = ctx.selection.file; + if (!file) { + return; + } + + const document = await ctx.workspace.readDocument(file.id, "new"); + const index = (document ?? "").split("\n").findIndex((line) => line.includes("TODO")); + if (index >= 0) { + ctx.navigation.revealLine(file.id, "new", index + 1); + } +}); +``` A handler may be async; a failure (sync or rejected) becomes a warning naming your extension. diff --git a/docs/line-highlights-proposal.md b/docs/line-highlights-proposal.md deleted file mode 100644 index 380e5a68e..000000000 --- a/docs/line-highlights-proposal.md +++ /dev/null @@ -1,257 +0,0 @@ -# Proposal: extension-contributed line highlights - -Status: implemented. This document records the design, what a critical review -of the draft changed, and the resolution of its open questions. The authoring -contract lives in `docs/extensions.md`; ownership is mapped in -`docs/extension-architecture.md`. - -An extension can add panes, file views, commands, keyboard modes, dialogs, and -whole VCS backends — but it could not mark **a range of characters inside a -diff line**. This is the smallest API that closes that gap, and it is smaller -than it looks because Hunk already renders exactly this kind of mark for its -own word-diff emphasis. - -## The gap - -Concretely: a search extension finds `readConfig` on line 214 and can jump the -review to that hunk, but cannot show the user _which characters_ matched. The -best it could do was quote the line on its own pane and highlight it there — -one row of indirection away from the code it is talking about, which defeats -the point. - -`registerFileView` is the only nearby lever and it is the wrong one: presenting -a file as extension-supplied rows replaces Pierre's diff rendering wholesale, -trading syntax highlighting, word diff, and layout fidelity for the ability to -color a substring. - -## What already existed - -Hunk paints intra-line background ranges today. Pierre marks word-diff emphasis -spans with a `data-diff-span` attribute, and `flattenHighlightedLine` -(`src/ui/diff/pierre.ts`) turns them into `RenderSpan { text, fg?, bg? }` runs -carrying an emphasis background. - -More importantly, Hunk already solves the hard part of this problem — **a -background is invisible unless it is resolved against the background it sits -on**. `MIN_WORD_DIFF_BG_DISTANCE` / `strengthenWordDiffBg` / -`resolveWordDiffHighlightBg` in `pierre.ts` blend an anchor color into the -line's own background until it clears a minimum perceptual distance, per line -kind and per theme. An added line already has a green background; a naive -highlight background is indistinguishable on it, which is precisely what a -first attempt at this produced. That machinery is the reason this API exists at -the tone level rather than the color level. The extension-facing version is -`lineHighlightToneBg` in `src/ui/diff/rowStyle.ts`, verified against every -built-in theme and line kind by tests. - -## The shape - -```ts -hunk.registerLineHighlighter({ - id: "matches", - highlight: ({ file, signal, readDocument }) => - ExtensionLineHighlight[] | null | Promise<...>, -}); -``` - -```ts -interface ExtensionLineHighlight { - /** Which side the line belongs to; a context line may be addressed by either. */ - side: "old" | "new"; - /** 1-based source line number on that side. */ - line: number; - /** [start, end) UTF-16 code-unit offsets into the line's raw source text. */ - range: readonly [number, number]; - /** What the mark means. The host decides what that looks like. */ - tone?: "match" | "current" | "info" | "warning" | "error"; -} -``` - -Addressing by `(side, line, range)` rather than by rendered row is deliberate: -source coordinates survive split vs stack, wrapping, horizontal scroll, -collapsed context, and note insertion. Extensions never learn Hunk's row model, -which is the same boundary `registerFileView` and `DiffPane` already hold. - -Invalidation is pull-based, identical in spirit to `ctx.fileViews.refresh`: -`highlight` is a pure derivation of the file plus an invalidation epoch, so a -search that moves to the next match bumps the epoch -(`ctx.highlights.refresh("matches")`, optionally `{ fileId }`-scoped) rather -than pushing new state into the host. There is no host-held mark state to go -stale across a reload. The epoch policy is the same one file views use, -extracted to `src/ui/lib/scopedEpochs.ts` and shared. - -## Tones, not colors - -The API does not accept a color. Three reasons, in order of importance: - -1. **Visibility is not the extension's to get right.** The contrast problem is - real, theme-dependent, and per line kind — `#334455` is legible on a context - line and invisible on a green one. `tone` lets the host apply the word-diff - minimum-distance guarantee to extension marks too. -2. **Themes stay coherent.** Hunk's theme guidance keeps official palette - tokens separate from the semantic `AppTheme` mapping; a raw color from an - extension punches straight through that. -3. **It is a smaller contract.** A tone can be re-mapped later; a color cannot. - -Tinted tone anchors map to existing semantic theme tokens (`accent`, -`badgeNeutral`, `fileModified`, `removedSignColor`) with a distance floor well -above word diff's, backing off only where the code on top would stop being -readable. `current` renders as reverse video — theme text as the block, theme -background as the glyphs — the `less`/vim convention for the active hit, which -sidesteps the tint-versus-readability tug-of-war entirely. A transparent cell -has no color to blend against, so resolution falls back to the theme's -background and then to the appearance's extreme: the mark stays visible, but -its distance floor is measured against an assumed surface rather than the one -the terminal actually shows. Only a theme whose own colors cannot take a blend -(non-hex) declines the mark — the same degradation word diff uses. - -## Where it plugs in - -The instinct is to apply highlights where spans are built — `makeSplitCell` / -`makeStackCell` in `pierre.ts`. **That is the wrong layer.** -`buildDiffSectionRowPlan` is the shared plan consumed by _both_ rendering and -geometry measurement; feeding highlights into it would put them in the cache -key of the expensive, geometry-bearing artifact, so pressing `n` during a -search would re-plan whole files to change some colors. - -Highlights are applied at **paint time** instead. The review confirmed the -placement argument holds, with two wiring caveats the draft missed: - -- **Memoized rows repaint only when a prop changes.** `DiffRowView` compares - props by reference, so the paint-time index is a real prop - (`lineHighlights`), built per file in `PierreDiffView` and threaded through - `DiffPane`/`DiffSection`. Per-file mark arrays keep stable identities across - unrelated preparation runs, so an epoch bump repaints exactly the files whose - marks changed. -- **Cell spans are shared cached arrays** (`flattenHighlightedLine` memoizes - per HAST node; `aliasHighlightedContextLines` shares one array across context - sides). Application therefore copies: spans are split at column boundaries - with backgrounds overridden, before `sliceSpansWindow` (horizontal scroll) - and `wrapSpans` (wrapping), which both preserve per-span colors for free. - -The split of responsibilities: - -| Layer | File | Work | -| ------------ | -------------------------------------------------------- | --------------------------------------------------- | -| Contract | `src/extension-api/types.ts` | types (import-free), API v5 | -| Registry | `src/extensions/types.ts`, `runExtension.ts`, `apply.ts` | collect + resolve registrations | -| Preparation | `src/ui/highlights/` | bounded async, validation, caps, epochs | -| Column model | `src/ui/diff/lineHighlightPaint.ts` | offsets → terminal columns; span repaint transform | -| Tone → color | `src/ui/diff/rowStyle.ts` | `lineHighlightToneBg`, word-diff distance guarantee | -| Paint | `src/ui/diff/renderRows.tsx` | apply per rendered cell, memo-aware | - -That placement buys three properties by construction: geometry neutrality -(colors change, text never does, so measurement and wrapping cannot move), -windowed cost (only mounted rows consult the index), and no cache invalidation -(a highlight change is a repaint, never a re-plan). - -## What the review surfaced - -- **The offset mapping is derivable but not free.** Cell spans are already - sanitized and tab-expanded, so raw code-unit offsets map through one owned - conversion: sanitize-aware (control characters can shift offsets), tab-aware - (a prefix expands to the same columns the full line's expansion gives it), - and snapped outward to grapheme-cluster boundaries so a mid-surrogate or - mid-cluster offset widens to the whole glyph instead of tearing it. -- **The static pager needs nothing.** `staticDiffPager.ts` never runs extension - code, so highlights are interactive-only by construction; the docs say so - explicitly rather than leaving it to be discovered. -- **Expanded collapsed-context rows are covered.** Gap rows render source - slices, and the gap's old↔new correspondence resolves marks addressed to - either side onto the loaded source line, keyed under both line numbers. - Without loaded source those marks are silently invisible — matching the rows - themselves. -- **Marks on invisible lines are not errors.** A mark inside a collapsed gap or - absent from a partial patch is valid; the review just is not showing that - line. Only structural garbage warns. -- **Extension file views are out of scope.** A file view already owns its rows' - spans and tones; highlights apply to Hunk's own diff rendering only. -- **Theme changes re-resolve for free.** Tone resolution happens at paint time - keyed by the theme object; nothing is cached against a stale theme. - -## Validation and containment - -- Out-of-range lines: silently invisible (see above). Inverted, empty, or - non-integer ranges, bad sides, unknown tones: dropped, one warning per - extension per file. -- Caps: 2,000 ranges per file, 100 per line; beyond either the file's - highlights from that highlighter are dropped whole rather than truncated - silently. -- Overlaps resolve deterministically: ranges sort by start column and the later - range wins where they overlap. -- A throwing, rejecting, or timed-out `highlight` costs that file's marks and - nothing else, bounded by the same timeout and concurrency discipline as file - views. -- Precedence: extension marks override word-diff emphasis where they overlap - (the more specific statement); cursor-line and copy-selection blends compose - on top, since they are `bg => blend(bg)` functions over whatever background - is present. - -## Resolved questions - -1. **Offset units** — UTF-16 code units into the raw line text. That is what - `indexOf`/`RegExp.exec` return against `file.patch` lines, so it is the only - contract extensions can satisfy without reimplementing Hunk's measurement - stack. The host clamps to cluster boundaries, widening outward. -2. **Context lines** — one mark, mirrored to both halves in split view, found - through either side's line number in stack view. A context line is one - occurrence that split view happens to draw twice; changed lines are separate - addresses with separate marks, which is what makes `n` step from an old-side - hit across to a new-side hit. -3. **`current` as a tone** — kept as a tone, rendered as reverse video. A - separate concept would be more API for the same pixels, and inversion is - both unmistakable and readable by construction where a stronger tint kept - trading against the text on top of it. -4. **Interaction with word diff** — the extension mark wins where they overlap. - It is the more specific statement, and it is stated in the docs. -5. **`matches` pre-filter** — dropped. File views need `matches` because view - availability is user-visible (the per-file View menu); highlights have no - visible selection state, so a cheap `highlight` returning `null` makes the - pre-filter pure optimization with no observable role. - -## What else this unlocks - -Search is the weakest justification on this list. - -- **Diagnostics inline.** `tsc`, `eslint`, `clippy`, `ruff` output mapped onto - the exact columns of the changed lines — "this PR introduces this error, - here" during review rather than in CI later. -- **Secret scanning.** Mark the exact token that tripped a rule, instead of - naming the line in a pane. -- **Homoglyph marking.** Marking the confusable identifier characters that read - as ASCII but are not. (Trojan-source defense more broadly does _not_ work - yet, and listing it here was a mistake: a bidi control or zero-width - character occupies no terminal column, so a range covering only those - characters resolves to zero columns and paints nothing. Making invisible - characters visible needs a separate mechanism — a gutter or margin marker, or - rendering a visible stand-in glyph — that this API does not have.) -- **Coverage.** Uncovered added ranges, straight from an lcov file. -- **Provenance.** Which ranges an agent wrote versus a human — squarely in - Hunk's stated purpose of understanding coding-agent changesets. -- **Occurrence highlighting.** Every use of the identifier under the cursor, - the way an editor does. -- **Repo conventions.** A `.hunk/extensions/` extension marking banned APIs, - `any`, stray `console.log`, or missing license headers for every reviewer on - the team. -- **Anchored agent notes.** Notes attach to lines today; a range would tie a - note to the exact expression it discusses. -- **Agent-driven marks.** With the session daemon already brokering agent - commands into live sessions, an agent answering "where does this change - behavior?" could light up the ranges in the user's terminal as it explains. - Nothing else in the ecosystem can do that. - -## Follow-ups - -- **Zero-width ranges are silently invisible.** A mark covering only characters - that occupy no terminal column (bidi controls, ZWSP, ZWJ) resolves to an - empty column range and paints nothing. It is the one case where a valid mark - on a rendered line still shows nothing, and it wants its own mechanism rather - than a background tint. - -## Companion gap (separate PR) - -`ctx.navigation` reaches files and hunks; the finest target is `selectHunk`. A -highlight tells you which characters matched, but the review still lands on the -hunk. `revealLine(fileId, side, line)` would complete it, and the internals -already have the pieces — `firstLineCursorInHunk` and `revealLineCursor` in -`useReviewController.ts` do exactly this for the host's own line cursor. -Deliberately kept out of the highlight change to keep both reviewable. diff --git a/scripts/check-pack.ts b/scripts/check-pack.ts index c23b3a145..60b71da9f 100644 --- a/scripts/check-pack.ts +++ b/scripts/check-pack.ts @@ -195,6 +195,16 @@ export default function (hunk: HunkExtensionAPI) { modeControls.exitMode(); ctx.panes.toggle("bottom"); if (ctx.sidebars.isOpen("legacy")) ctx.sidebars.close("legacy"); + + const targetFile = ctx.selection.file; + if (targetFile) { + ctx.navigation.selectFile(targetFile.id); + ctx.navigation.selectHunk(targetFile.id, 0); + ctx.navigation.revealLine(targetFile.id, "new", 211); + ctx.navigation.revealLine(targetFile.id, "old", 1); + // @ts-expect-error Only the two diff sides address a line. + ctx.navigation.revealLine(targetFile.id, "both", 1); + } }); hunk.registerCommand({ id: "rewrite", title: "Rewrite the selection" }, async (ctx) => { diff --git a/skills/hunk-extensions/SKILL.md b/skills/hunk-extensions/SKILL.md index 958f44e79..d6a89f17f 100644 --- a/skills/hunk-extensions/SKILL.md +++ b/skills/hunk-extensions/SKILL.md @@ -123,7 +123,8 @@ transform — gets `ctx.cwd` and `ctx.notify(message, type?)`. A file view's - **Command handlers** get `ctx.panes`, `ctx.fileViews` (select/toggle/isActive/ refresh/enterMode/exitMode), `ctx.highlights` (refresh prepared line marks, whole or `{ fileId }`-scoped), `ctx.selection` (a snapshot of file + hunk index), - `ctx.navigation` (live, guarded `selectFile`/`selectHunk`), `ctx.commands` + `ctx.navigation` (live, guarded `selectFile`/`selectHunk`/`revealLine`, the + last landing one exact `(side, line)` near the viewport top), `ctx.commands` (`isEnabled`/`execute` for public semantic `hunk.*` commands), `ctx.keyboardModes` (enter/exit/probe this extension's session modes), `ctx.dialogs` (`confirm`/`select`/`input`, queued and attributed), and `ctx.workspace` diff --git a/src/extension-api/types.ts b/src/extension-api/types.ts index ac1375a6b..e11ea29ae 100644 --- a/src/extension-api/types.ts +++ b/src/extension-api/types.ts @@ -1012,6 +1012,22 @@ export interface ExtensionReviewNavigation { selectFile(fileId: string): void; /** Jump the review stream to one hunk of one file. */ selectHunk(fileId: string, hunkIndex: number): void; + /** + * Jump the review stream to one source line, addressed by side and number. + * + * The finest navigation target there is: a hunk hundreds of lines tall no + * longer lands the viewport pages away from the line you meant. `line` is a + * 1-based number on `side` as the patch numbers it, so a context line + * answers to either side's number. The revealed line lands where every other + * Hunk reveal lands — a little below the viewport top — and becomes the + * current line, so it pairs with a mark from `registerLineHighlighter`. + * + * When the review cannot render that line (it sits inside a collapsed gap, + * or the patch never numbered it) the jump falls back to the hunk containing + * it; a line no hunk contains is refused with a warning naming the + * extension. + */ + revealLine(fileId: string, side: "old" | "new", line: number): void; } /** diff --git a/src/ui/App.tsx b/src/ui/App.tsx index e198f57f7..b3e6d845f 100644 --- a/src/ui/App.tsx +++ b/src/ui/App.tsx @@ -84,7 +84,11 @@ import { useExtensionDialogController } from "./hooks/useExtensionDialogControll import { useExtensionNotifications } from "./hooks/useExtensionNotifications"; import { useHunkSessionBridge } from "./hooks/useHunkSessionBridge"; import { useMenuController } from "./hooks/useMenuController"; -import { useReviewController, type AgentNoteGeometrySnapshot } from "./hooks/useReviewController"; +import { + useReviewController, + type AgentNoteGeometrySnapshot, + type RevealedLineResult, +} from "./hooks/useReviewController"; import { useWatchedInput, type WatchedInputRuntime } from "./hooks/useWatchedInput"; import { agentNoteMarkupWidth } from "./lib/agentNoteGeometry"; import { @@ -491,6 +495,8 @@ export function App({ const extensionCommandNavigationRef = useRef({ onSelectFile: (_fileId: string) => {}, onSelectHunk: (_fileId: string, _hunkIndex: number) => {}, + onRevealLine: (_fileId: string, _side: "old" | "new", _line: number): RevealedLineResult => + "none", }); // A hard session reload (`resetApp`) remounts App under an in-flight async // command handler, whose `ctx.navigation` closes over *this* instance's @@ -878,6 +884,8 @@ export function App({ onSelectFile: (fileId) => extensionCommandNavigationRef.current.onSelectFile(fileId), onSelectHunk: (fileId, hunkIndex) => extensionCommandNavigationRef.current.onSelectHunk(fileId, hunkIndex), + onRevealLine: (fileId, side, line) => + extensionCommandNavigationRef.current.onRevealLine(fileId, side, line), }), }; @@ -1752,6 +1760,10 @@ export function App({ focusFiles(); review.selectHunk(fileId, hunkIndex); }, + onRevealLine: (fileId, side, line) => { + focusFiles(); + return review.revealLine(fileId, side, line); + }, }; /** Toggle keyboard focus between the file list and the file filter. */ @@ -2081,6 +2093,10 @@ export function App({ focusFiles(); review.selectHunk(fileId, hunkIndex); }} + onRevealLine={(fileId, side, line) => { + focusFiles(); + return review.revealLine(fileId, side, line); + }} onRenderFailure={ pane.key === HUNK_FILES_PANE_KEY ? undefined @@ -2215,7 +2231,7 @@ export function App({ selectedHunkRevealRequestId={review.selectedHunkRevealRequestId} cursorLine={cursorLine} lineCursor={review.lineCursor} - lineCursorRevealRequestId={review.lineCursorRevealRequestId} + lineCursorRevealRequest={review.lineCursorRevealRequest} lineCursorAlignmentRequest={lineCursorAlignmentRequest} theme={activeTheme} width={diffPaneWidth} diff --git a/src/ui/components/panes/DiffPane.tsx b/src/ui/components/panes/DiffPane.tsx index 233be93fe..428e17080 100644 --- a/src/ui/components/panes/DiffPane.tsx +++ b/src/ui/components/panes/DiffPane.tsx @@ -36,6 +36,7 @@ import { computeLineAlignmentScrollTop, computeLineRevealScrollTop, type CurrentLineAlignment, + type LineRevealPlacement, } from "../../lib/hunkScroll"; import { inlineNoteStableKey } from "../../diff/reviewRenderPlan"; import { @@ -218,7 +219,7 @@ export function DiffPane({ selectedHunkIndex, cursorLine = "off", lineCursor = null, - lineCursorRevealRequestId = 0, + lineCursorRevealRequest = { id: 0, placement: "nearest" }, lineCursorAlignmentRequest = { id: 0, alignment: "center" }, scrollToNote = false, draftNote = null, @@ -280,7 +281,7 @@ export function DiffPane({ selectedHunkIndex: number; cursorLine?: CursorLine; lineCursor?: LineCursor | null; - lineCursorRevealRequestId?: number; + lineCursorRevealRequest?: { id: number; placement: LineRevealPlacement }; lineCursorAlignmentRequest?: { id: number; alignment: CurrentLineAlignment }; scrollToNote?: boolean; draftNote?: DraftReviewNote | null; @@ -2110,13 +2111,13 @@ export function DiffPane({ suppressViewportSelectionSync, ]); - const previousLineCursorRevealRequestIdRef = useRef(lineCursorRevealRequestId); + const previousLineCursorRevealRequestIdRef = useRef(lineCursorRevealRequest.id); useLayoutEffect(() => { - if (previousLineCursorRevealRequestIdRef.current === lineCursorRevealRequestId) { + if (previousLineCursorRevealRequestIdRef.current === lineCursorRevealRequest.id) { return; } - previousLineCursorRevealRequestIdRef.current = lineCursorRevealRequestId; + previousLineCursorRevealRequestIdRef.current = lineCursorRevealRequest.id; const scrollBox = scrollRef.current; if (!scrollBox || !lineCursor) { @@ -2129,12 +2130,30 @@ export function DiffPane({ } const viewportHeight = scrollBox.viewport.height || scrollViewport.height; - const revealScrollTop = computeLineRevealScrollTop({ - lineTop: bounds.top, - lineHeight: bounds.height, - scrollTop: scrollBox.scrollTop, - viewportHeight, - }); + // A jump lands the line where hunk and note reveals land theirs; stepping only closes the + // gap to the viewport edge, so a held key does not drag the whole stream past the marker. + const revealScrollTop = + lineCursorRevealRequest.placement === "reveal" + ? computeHunkRevealScrollTop({ + hunkTop: bounds.top, + hunkHeight: bounds.height, + preferredTopPadding: Math.max(2, Math.floor(viewportHeight * 0.25)), + viewportHeight, + }) + : computeLineRevealScrollTop({ + lineTop: bounds.top, + lineHeight: bounds.height, + scrollTop: scrollBox.scrollTop, + viewportHeight, + }); + // A named line is the final scroll policy for this request, exactly as an + // explicit alignment is: a cross-file reveal changes the selection, and the + // selection reveal it schedules would otherwise run its zero-delay retry + // after this layout effect and drag the viewport back to the hunk anchor. + // Superseding here is what keeps a reveal into another file line-exact. + supersedePendingSelectionReveal(); + clearPendingFileTopAlign(); + if (revealScrollTop === scrollBox.scrollTop) { return; } @@ -2143,11 +2162,13 @@ export function DiffPane({ scrollBox.scrollTo(clampReviewScrollTop(revealScrollTop, viewportHeight)); }, [ clampReviewScrollTop, + clearPendingFileTopAlign, lineCursor, lineCursorBoundsOf, - lineCursorRevealRequestId, + lineCursorRevealRequest, scrollRef, scrollViewport.height, + supersedePendingSelectionReveal, suppressViewportSelectionSync, ]); diff --git a/src/ui/components/panes/ExtensionPane.test.tsx b/src/ui/components/panes/ExtensionPane.test.tsx index ededa92e0..135ddb4c3 100644 --- a/src/ui/components/panes/ExtensionPane.test.tsx +++ b/src/ui/components/panes/ExtensionPane.test.tsx @@ -89,6 +89,7 @@ describe("ExtensionPaneHost actions", () => { notify={(message) => notifications.push(message)} onSelectFile={() => {}} onSelectHunk={(fileId, hunkIndex) => hunkSelections.push([fileId, hunkIndex])} + onRevealLine={() => "line"} />, async () => { if (!actions) { @@ -156,6 +157,7 @@ describe("ExtensionPaneHost failure recovery", () => { notify={(message) => notifications.push(message)} onSelectFile={() => {}} onSelectHunk={() => {}} + onRevealLine={() => "line"} />, async (setup) => { expect(setup.captureCharFrame()).toContain("Files pane unavailable"); @@ -199,6 +201,7 @@ describe("ExtensionPaneHost failure recovery", () => { notify={(message) => notifications.push(message)} onSelectFile={() => {}} onSelectHunk={() => {}} + onRevealLine={() => "line"} /> ); } diff --git a/src/ui/components/panes/ExtensionPane.tsx b/src/ui/components/panes/ExtensionPane.tsx index 21f35b48f..3689fd34c 100644 --- a/src/ui/components/panes/ExtensionPane.tsx +++ b/src/ui/components/panes/ExtensionPane.tsx @@ -66,6 +66,7 @@ export interface ExtensionPaneHostProps { notify: ExtensionNotifySink; onSelectFile: (fileId: string) => void; onSelectHunk: (fileId: string, hunkIndex: number) => void; + onRevealLine: (fileId: string, side: "old" | "new", line: number) => "line" | "hunk" | "none"; onRenderFailure?: () => void; } @@ -86,6 +87,7 @@ function ExtensionPaneHostView({ notify, onSelectFile, onSelectHunk, + onRevealLine, onRenderFailure, }: ExtensionPaneHostProps) { const { extensionId } = registered; @@ -99,12 +101,13 @@ function ExtensionPaneHostView({ notify, onSelectFile, onSelectHunk, + onRevealLine, }), notify(message: string, type: ExtensionNotifyType = "info") { notify(`${extensionId}: ${message}`, type); }, }), - [extensionId, files, notify, onSelectFile, onSelectHunk], + [extensionId, files, notify, onRevealLine, onSelectFile, onSelectHunk], ); const View = registered.pane.component as (props: ExtensionPaneProps) => ReactNode; const viewProps: ExtensionPaneProps = { diff --git a/src/ui/components/ui-components.test.tsx b/src/ui/components/ui-components.test.tsx index 88ac91af2..453d4b04a 100644 --- a/src/ui/components/ui-components.test.tsx +++ b/src/ui/components/ui-components.test.tsx @@ -17,7 +17,7 @@ import { measureDiffSectionGeometry } from "../diff/diffSectionGeometry"; import { buildFileSectionLayouts, buildInStreamFileHeaderHeights } from "../lib/fileSectionLayout"; import { builtinCommandKeyDefaults, builtinCommandMatchProbes } from "../lib/appCommands"; import { resolveCommandKeys } from "../lib/keymap"; -import type { CurrentLineAlignment } from "../lib/hunkScroll"; +import type { CurrentLineAlignment, LineRevealPlacement } from "../lib/hunkScroll"; import type { LineCursor } from "../lib/lineCursors"; const { AppHost } = await import("../AppHost"); @@ -461,7 +461,12 @@ describe("UI components", () => { theme={theme} width={30} keybindings={{ matches: () => false, getKeys: () => [] }} - actions={{ selectFile: () => {}, selectHunk: () => {}, notify: () => {} }} + actions={{ + selectFile: () => {}, + selectHunk: () => {}, + revealLine: () => {}, + notify: () => {}, + }} />, 36, 12, @@ -1363,6 +1368,103 @@ describe("UI components", () => { } }); + test("DiffPane keeps a cross-file line reveal against the selection reveal retry", async () => { + const theme = resolveTheme("github-dark-default", null); + // Two files, so revealing a line in the second one also changes the + // selected file — the case that schedules selection-reveal retries. The + // second file is one tall hunk, so a line near its end sits far from the + // hunk anchor the retry would scroll to, making the two outcomes + // unmistakably different rather than a row apart. + const files = [ + createWideTwoHunkDiffFile("first", "first.ts", 1), + createTallDiffFile("second", "second.ts", 60), + ]; + const scrollRef = createRef(); + let revealDeepLineInSecondFile = () => {}; + let cursorReady = false; + + function CrossFileRevealHarness() { + const [selection, setSelection] = useState({ + fileId: files[0]!.id, + hunkIndex: 0, + }); + const [selectedHunkRevealRequestId, setSelectedHunkRevealRequestId] = useState(0); + const [lineCursor, setLineCursor] = useState(null); + const [revealRequest, setRevealRequest] = useState<{ + id: number; + placement: LineRevealPlacement; + }>({ id: 0, placement: "nearest" }); + const deepCursorRef = useRef(null); + + revealDeepLineInSecondFile = () => { + const cursor = deepCursorRef.current; + if (!cursor) return; + // Exactly what a `revealLine` into another file produces: a new + // selection, a hunk reveal request, and an explicit line reveal. + setSelection({ fileId: cursor.fileId, hunkIndex: cursor.hunkIndex }); + setSelectedHunkRevealRequestId((current) => current + 1); + setLineCursor(cursor); + setRevealRequest((current) => ({ id: current.id + 1, placement: "reveal" })); + }; + + return ( + { + const deep = cursors.filter((cursor) => cursor.fileId === files[1]!.id).at(-1); + if (!deep) return; + deepCursorRef.current = deep; + cursorReady = true; + }} + /> + ); + } + + const setup = await testRender(, { width: 104, height: 14 }); + + try { + for (let attempt = 0; attempt < 10 && !cursorReady; attempt += 1) { + await settleDiffPane(setup); + } + expect(cursorReady).toBe(true); + + await act(async () => { + revealDeepLineInSecondFile(); + await setup.renderOnce(); + await setup.renderOnce(); + }); + const revealedScrollTop = scrollRef.current?.scrollTop ?? 0; + + // The selection reveal schedules a zero-delay retry plus a 120ms + // pinned-header settle window. Both must leave the exact line where the + // reveal put it; without superseding they scroll back to the hunk anchor. + await act(async () => { + await Bun.sleep(150); + await setup.renderOnce(); + }); + + expect(scrollRef.current?.scrollTop ?? 0).toBe(revealedScrollTop); + // Guard the guard: if the fixture ever put the revealed line at the hunk + // anchor, this test would pass without proving anything. + expect(revealedScrollTop).toBeGreaterThan(20); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + test("DiffPane viewport-follow selection does not move the scroll position", async () => { const theme = resolveTheme("github-dark-default", null); const files = [ diff --git a/src/ui/diff/reviewRenderPlan.ts b/src/ui/diff/reviewRenderPlan.ts index 13c0e230f..b6888966c 100644 --- a/src/ui/diff/reviewRenderPlan.ts +++ b/src/ui/diff/reviewRenderPlan.ts @@ -96,6 +96,7 @@ function contextLineStableKey(hunkIndex: number, oldLineNumber?: number, newLine const SIDED_LINE_STABLE_KEY = /^line:(\d+):(old|new):(\d+)$/; const CONTEXT_LINE_STABLE_KEY = /^line:(\d+):context:\d+:(\d+)$/; +const CONTEXT_LINE_STABLE_KEY_SIDES = /^line:(\d+):context:(\d+):(\d+)$/; /** Recover the source line one single-sided stable anchor names. */ export function lineStableKeyTarget( @@ -125,6 +126,27 @@ export function contextLineStableKeyTarget( return { hunkIndex: Number(match[1]), side: "new", line: Number(match[2]) }; } +/** + * Recover both source lines one shared context anchor names. + * + * A context row shows the same text on both sides under two different numbers, so a caller + * addressing it by line number has to be able to match either one. + */ +export function contextLineStableKeySides( + stableKey: string, +): { hunkIndex: number; oldLine: number; newLine: number } | null { + const match = CONTEXT_LINE_STABLE_KEY_SIDES.exec(stableKey); + if (!match) { + return null; + } + + return { + hunkIndex: Number(match[1]), + oldLine: Number(match[2]), + newLine: Number(match[3]), + }; +} + /** Resolve the stable anchor keys for one rendered diff row across split and stack layouts. */ function diffRowStableKeys(row: DiffRow) { if (row.type === "collapsed") { diff --git a/src/ui/hooks/useReviewController.test.tsx b/src/ui/hooks/useReviewController.test.tsx index 42152197c..eb6880d46 100644 --- a/src/ui/hooks/useReviewController.test.tsx +++ b/src/ui/hooks/useReviewController.test.tsx @@ -1,6 +1,6 @@ import { describe, expect, spyOn, test } from "bun:test"; import { testRender } from "@opentui/react/test-utils"; -import { act, StrictMode, useEffect, useState } from "react"; +import { act, StrictMode, useEffect, useRef, useState } from "react"; import { SourceTextTooLargeError } from "../../core/fileSource"; import type { DiffFile } from "../../core/types"; import { @@ -49,7 +49,7 @@ function createTwoHunkFile() { } /** Build one file with three separated hunks for counted navigation coverage. */ -function createThreeHunkFile() { +function createThreeHunkFile(id = "alpha", path = "alpha.ts") { const beforeLines = Array.from( { length: 30 }, (_, index) => `export const line${index + 1} = ${index + 1};`, @@ -59,7 +59,7 @@ function createThreeHunkFile() { afterLines[14] = "export const line15 = 1500;"; afterLines[29] = "export const line30 = 3000;"; - return createDiffFile("alpha", "alpha.ts", lines(...beforeLines), lines(...afterLines)); + return createDiffFile(id, path, lines(...beforeLines), lines(...afterLines)); } /** Build the same file id with only one hunk so stale hunk indices must clamp. */ @@ -155,23 +155,40 @@ function expectValue(value: T): NonNullable { function ReviewControllerHarness({ initialFiles, noteGeometry, + publishLineCursors = true, stmlEnabled, onController, + onFirstController, onSetFiles, }: { initialFiles: DiffFile[]; noteGeometry?: Parameters[0]["noteGeometry"]; + /** Publish measured stops, as the diff pane does unless the current-line marker is off. */ + publishLineCursors?: boolean; stmlEnabled?: boolean; onController: (controller: ReviewController) => void; + /** Receive the first render's controller, before any cursors were published. */ + onFirstController?: (controller: ReviewController) => void; onSetFiles?: (setFiles: (nextFiles: DiffFile[]) => void) => void; }) { const [files, setFiles] = useState(initialFiles); const [lineCursors, setLineCursors] = useState([]); const controller = useReviewController({ files, lineCursors, noteGeometry, stmlEnabled }); + // Capture during render, as a memoized consumer's closure would: the effects + // below have not yet published measured cursors on the first pass. + const firstControllerRef = useRef(null); + if (firstControllerRef.current === null) { + firstControllerRef.current = controller; + onFirstController?.(controller); + } const visibleFiles = controller.visibleFiles; const { expandedGapsByFileId, sourceStatusByFileId } = controller; useEffect(() => { + if (!publishLineCursors) { + return; + } + setLineCursors( buildLineCursors( visibleFiles, @@ -191,7 +208,7 @@ function ReviewControllerHarness({ ), ), ); - }, [expandedGapsByFileId, sourceStatusByFileId, visibleFiles]); + }, [expandedGapsByFileId, publishLineCursors, sourceStatusByFileId, visibleFiles]); useEffect(() => { onController(controller); @@ -210,11 +227,15 @@ async function renderReviewController( { strictMode = false, noteGeometry, + publishLineCursors, stmlEnabled, + onFirstController, }: { strictMode?: boolean; noteGeometry?: Parameters[0]["noteGeometry"]; + publishLineCursors?: boolean; stmlEnabled?: boolean; + onFirstController?: (controller: ReviewController) => void; } = {}, ) { const controllerRef: { current: ReviewController | null } = { current: null }; @@ -223,7 +244,9 @@ async function renderReviewController( { controllerRef.current = nextController; }} @@ -1578,16 +1601,17 @@ describe("useReviewController", () => { try { await flush(setup); - const initialRequestId = expectValue(controllerRef.current).lineCursorRevealRequestId; + const initialRequestId = expectValue(controllerRef.current).lineCursorRevealRequest.id; await act(async () => { expectValue(controllerRef.current).moveLineCursor(1); }); await flush(setup); - expect(expectValue(controllerRef.current).lineCursorRevealRequestId).toBe( - initialRequestId + 1, - ); + expect(expectValue(controllerRef.current).lineCursorRevealRequest).toEqual({ + id: initialRequestId + 1, + placement: "nearest", + }); } finally { await act(async () => { setup.renderer.destroy(); @@ -1621,7 +1645,7 @@ describe("useReviewController", () => { (cursor) => cursor.stableKey === initial.stableKey && cursor.fileId === initial.fileId, ); const expected = expectValue(expectedCursors[initialIndex + 4]); - const initialRequestId = expectValue(controllerRef.current).lineCursorRevealRequestId; + const initialRequestId = expectValue(controllerRef.current).lineCursorRevealRequest.id; await act(async () => { expectValue(controllerRef.current).moveLineCursor(4); @@ -1629,9 +1653,10 @@ describe("useReviewController", () => { await flush(setup); expect(expectValue(controllerRef.current).lineCursor).toEqual(expected); - expect(expectValue(controllerRef.current).lineCursorRevealRequestId).toBe( - initialRequestId + 1, - ); + expect(expectValue(controllerRef.current).lineCursorRevealRequest).toEqual({ + id: initialRequestId + 1, + placement: "nearest", + }); } finally { await act(async () => { setup.renderer.destroy(); @@ -1797,4 +1822,133 @@ describe("useReviewController", () => { }); } }); + + test("reveals one line of another file and asks for the reveal placement", async () => { + // Cursors are measured for every visible file, so a cross-file jump resolves in the same + // pass as a local one — no pending second reveal once the target file renders. + const { controllerRef, setup } = await renderReviewController([ + createDiffFile("alpha", "alpha.ts", "export const alpha = 1;\n", "export const alpha = 2;\n"), + createThreeHunkFile("beta", "beta.ts"), + ]); + + try { + await flush(setup); + const before = expectValue(controllerRef.current).lineCursorRevealRequest; + expect(expectValue(expectValue(controllerRef.current).lineCursor).fileId).toBe("alpha"); + + let outcome: string | undefined; + await act(async () => { + outcome = expectValue(controllerRef.current).revealLine("beta", "new", 30); + }); + await flush(setup); + + expect(outcome).toBe("line"); + expect(expectValue(controllerRef.current).lineCursor).toMatchObject({ + fileId: "beta", + hunkIndex: 2, + target: { side: "new", line: 30 }, + }); + // Selection follows the revealed line so notes and hunk actions stay on the same target. + expect(expectValue(controllerRef.current).selectedFileId).toBe("beta"); + expect(expectValue(controllerRef.current).selectedHunkIndex).toBe(2); + expect(expectValue(controllerRef.current).lineCursorRevealRequest).toEqual({ + id: before.id + 1, + placement: "reveal", + }); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + 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. + const { controllerRef, setup } = await renderReviewController([createThreeHunkFile()], { + publishLineCursors: false, + }); + + try { + await flush(setup); + const before = expectValue(controllerRef.current).lineCursorRevealRequest; + + let outcome: string | undefined; + await act(async () => { + outcome = expectValue(controllerRef.current).revealLine("alpha", "new", 15); + }); + await flush(setup); + + expect(outcome).toBe("hunk"); + 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); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("a revealLine reference held from before cursors were measured still lands the line", async () => { + // A memoized extension pane keeps its mount-time `actions`, whose + // `revealLine` was minted on the first render — before the diff pane's + // effect published any measured cursors. That held reference must read the + // stops live: the session's first deferred search jump used to silently + // degrade to the hunk fallback because its closure still saw the empty + // pre-measurement list. + const firstControllerRef: { current: ReviewController | null } = { current: null }; + const { controllerRef, setup } = await renderReviewController([createThreeHunkFile()], { + onFirstController: (controller) => { + firstControllerRef.current = controller; + }, + }); + + try { + await flush(setup); + // The captured instance predates the cursor publication… + const first = expectValue(firstControllerRef.current); + // …while the current instance has long since seen the measured stops. + expect(expectValue(controllerRef.current).lineCursor).not.toBeNull(); + + let outcome: string | undefined; + await act(async () => { + outcome = first.revealLine("alpha", "new", 30); + }); + await flush(setup); + + expect(outcome).toBe("line"); + expect(expectValue(controllerRef.current).lineCursor).toMatchObject({ + fileId: "alpha", + hunkIndex: 2, + target: { side: "new", line: 30 }, + }); + expect(expectValue(controllerRef.current).lineCursorRevealRequest.placement).toBe("reveal"); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); + + test("reports a line no hunk of the file covers instead of moving the review", async () => { + const { controllerRef, setup } = await renderReviewController([createTwoHunkFile()]); + + try { + await flush(setup); + + let outcome: string | undefined; + await act(async () => { + outcome = expectValue(controllerRef.current).revealLine("alpha", "new", 9001); + }); + await flush(setup); + + expect(outcome).toBe("none"); + expect(expectValue(expectValue(controllerRef.current).lineCursor).hunkIndex).toBe(0); + } finally { + await act(async () => { + setup.renderer.destroy(); + }); + } + }); }); diff --git a/src/ui/hooks/useReviewController.ts b/src/ui/hooks/useReviewController.ts index db838847e..9652ee7f3 100644 --- a/src/ui/hooks/useReviewController.ts +++ b/src/ui/hooks/useReviewController.ts @@ -33,6 +33,7 @@ import { } from "../../core/review/intents"; import { projectReviewDocument } from "../../core/review/document"; import { reviewExpansionSide } from "../../core/review/expansion"; +import { reviewHunkIndexForLine } from "../../core/review/geometry"; import type { ReviewSelectionScope } from "../../core/review/navigation"; import { reviewFileKeysWithRetiredContent, @@ -59,8 +60,10 @@ import type { } from "../../session/types"; import type { FileSourceStatus } from "../diff/expandCollapsedRows"; +import type { LineRevealPlacement } from "../lib/hunkScroll"; import { EMPTY_LINE_CURSORS, + findLineCursorAt, findNextLineCursor, firstLineCursorInHunk, hasLineCursor, @@ -138,6 +141,25 @@ interface SourceLoadRequest { side: "old" | "new"; } +/** + * One request to scroll the current line into view, and how far it may move the viewport. + * + * Carried as one object because the id and the placement have to change together: a consumer + * that read a bumped id against a stale placement would scroll by the previous policy. + */ +export interface LineCursorRevealRequest { + id: number; + placement: LineRevealPlacement; +} + +/** + * What a `revealLine` call could actually reach. + * + * `"line"` landed on the requested line, `"hunk"` fell back to the hunk containing it because + * the review stream draws no row for that line, and `"none"` means no hunk covers it either. + */ +export type RevealedLineResult = "line" | "hunk" | "none"; + export interface ReviewSelectionOptions { alignFileHeaderTop?: boolean; scrollToNote?: boolean; @@ -179,13 +201,14 @@ export interface ReviewController { showAgentNotes: boolean; userNotesByFileId: Record; lineCursor: LineCursor | null; - lineCursorRevealRequestId: number; + lineCursorRevealRequest: LineCursorRevealRequest; anchorLineCursor: (cursor: LineCursor) => void; /** Adopt the hunk a viewport settled on, without asking any viewport to move. */ anchorSelection: (fileId: string, hunkIndex: number) => void; moveLineCursor: (delta: number) => void; /** Step the selection through one navigable scope; the scope owns wrap and reveal. */ moveSelection: (scope: ReviewSelectionScope, delta: number) => void; + revealLine: (fileId: string, side: "old" | "new", line: number) => RevealedLineResult; scrollToNote: boolean; selectedFile: DiffFile | undefined; selectedFileId: string; @@ -315,7 +338,10 @@ export function useReviewController({ // A held key drains as one stdin chunk, so every press in the burst would otherwise read the // same pre-batch state and the cursor would advance a single row. const lineCursorRef = useRef(null); - const [lineCursorRevealRequestId, setLineCursorRevealRequestId] = useState(0); + const [lineCursorRevealRequest, setLineCursorRevealRequest] = useState({ + id: 0, + placement: "nearest", + }); const previousLineCursorsRef = useRef(lineCursors); const pendingLineCursorRef = useRef< | { kind: "reveal"; fileId: string; gapKey: string } @@ -488,9 +514,9 @@ export function useReviewController({ /** Move the current line to a row the reviewer just asked to see, and scroll to it. */ const revealLineCursor = useCallback( - (cursor: LineCursor) => { + (cursor: LineCursor, placement: LineRevealPlacement = "nearest") => { applyLineCursor(cursor); - setLineCursorRevealRequestId((current) => current + 1); + setLineCursorRevealRequest((current) => ({ id: current.id + 1, placement })); // The line cursor carries its own reveal request; the selection only follows it. anchorSelection(cursor.fileId, cursor.hunkIndex); }, @@ -570,6 +596,18 @@ export function useReviewController({ [lineCursors, revealLineCursor], ); + // `revealLine` is handed to surfaces that hold it across commits — a memoized + // extension pane keeps its mount-time `actions`, and an async command handler + // may navigate long after its dispatch render. Reading these through the + // callback's own closure silently downgraded such a call to the hunk + // fallback: at mount the pane captured a `revealLine` whose cursor list was + // still the pre-measurement empty state, so the first deferred jump of a + // session landed on the hunk anchor. Refs keep any held reference live. + const lineCursorsForRevealRef = useRef(lineCursors); + lineCursorsForRevealRef.current = lineCursors; + const visibleFilesRef = useRef(visibleFiles); + visibleFilesRef.current = visibleFiles; + /** * Step the selection through one navigable scope. * @@ -583,6 +621,34 @@ export function useReviewController({ [annotations, runIntent], ); + /** + * Jump to one file's source line, addressed the way a patch numbers it. + * + * Cursors are measured for every visible file, not just the selected one, so a cross-file + * jump resolves in the same pass as a local one. A line the stream draws no row for — hidden + * inside a collapsed gap, or never numbered by a partial patch — has no row to scroll to, so + * the jump degrades to the hunk containing it and reports which target it reached. + */ + const revealLine = useCallback( + (fileId: string, side: "old" | "new", line: number): RevealedLineResult => { + const cursor = findLineCursorAt(lineCursorsForRevealRef.current, fileId, side, line); + if (cursor) { + revealLineCursor(cursor, "reveal"); + return "line"; + } + + const file = visibleFilesRef.current.find((candidate) => candidate.id === fileId); + const hunkIndex = file ? reviewHunkIndexForLine(file.metadata.hunks, side, line) : -1; + if (hunkIndex < 0) { + return "none"; + } + + selectHunk(fileId, hunkIndex); + return "hunk"; + }, + [revealLineCursor, selectHunk], + ); + /** Set the shared file filter. */ const setFilter = useCallback( (value: string) => { @@ -1148,7 +1214,7 @@ export function useReviewController({ liveCommentSummaries, liveCommentsByFileId, lineCursor, - lineCursorRevealRequestId, + lineCursorRevealRequest, reviewNoteCount: reviewNoteSummaries.length, reviewNoteSummaries, showAgentNotes: state.showAgentNotes, @@ -1176,6 +1242,7 @@ export function useReviewController({ navigateToLocation, removeLiveComment, removeUserNote, + revealLine, saveDraftNote, selectFile, selectHunk, diff --git a/src/ui/lib/extensionNavigation.test.ts b/src/ui/lib/extensionNavigation.test.ts index 9f50fcc47..7321fc238 100644 --- a/src/ui/lib/extensionNavigation.test.ts +++ b/src/ui/lib/extensionNavigation.test.ts @@ -10,10 +10,12 @@ function createTestNavigation(options?: { files?: ReturnType[]; onSelectFile?: (fileId: string) => void; onSelectHunk?: (fileId: string, hunkIndex: number) => void; + revealResult?: "line" | "hunk" | "none"; }) { const warnings: string[] = []; const selectedFiles: string[] = []; const selectedHunks: Array<[string, number]> = []; + const revealedLines: Array<[string, string, number]> = []; let files = options?.files ?? [createTestNavigableFile("a", 3)]; const navigation = createGuardedReviewNavigation({ @@ -27,6 +29,10 @@ function createTestNavigation(options?: { onSelectFile: options?.onSelectFile ?? ((fileId) => selectedFiles.push(fileId)), onSelectHunk: options?.onSelectHunk ?? ((fileId, hunkIndex) => selectedHunks.push([fileId, hunkIndex])), + onRevealLine: (fileId, side, line) => { + revealedLines.push([fileId, side, line]); + return options?.revealResult ?? "line"; + }, }); return { @@ -34,6 +40,7 @@ function createTestNavigation(options?: { warnings, selectedFiles, selectedHunks, + revealedLines, setFiles(next: ReturnType[]) { files = next; }, @@ -116,20 +123,88 @@ describe("createGuardedReviewNavigation", () => { }, onSelectFile: (fileId) => selectedFiles.push(fileId), onSelectHunk: () => {}, + onRevealLine: () => "line", }); navigation.selectFile("a"); alive = false; navigation.selectFile("a"); navigation.selectHunk("a", 0); + navigation.revealLine("a", "new", 1); expect(selectedFiles).toEqual(["a"]); expect(warnings).toEqual([ "Extension triage selectFile ignored — the review session was reloaded", "Extension triage selectHunk ignored — the review session was reloaded", + "Extension triage revealLine ignored — the review session was reloaded", ]); }); + test("routes a line reveal on a visible file through to the host callback", () => { + const { navigation, revealedLines, warnings } = createTestNavigation(); + + navigation.revealLine("a", "old", 211); + + expect(revealedLines).toEqual([["a", "old", 211]]); + expect(warnings).toEqual([]); + }); + + test("refuses a line reveal on a file the review stream cannot show", () => { + const { navigation, revealedLines, warnings } = createTestNavigation(); + + navigation.revealLine("hidden", "new", 4); + + expect(revealedLines).toEqual([]); + expect(warnings).toEqual(['Extension triage revealLine targeted unknown file id "hidden"']); + }); + + test("refuses a side outside the two diff sides", () => { + const { navigation, revealedLines, warnings } = createTestNavigation(); + + navigation.revealLine("a", "both" as unknown as "new", 4); + + expect(revealedLines).toEqual([]); + expect(warnings).toEqual(['Extension triage revealLine received an invalid side for "a"']); + }); + + test("refuses a line number that is not a 1-based whole line", () => { + // Patches number lines from 1 upward; a fraction, a zero, or a string is a caller bug, + // not a line the review merely failed to find. + const { navigation, revealedLines, warnings } = createTestNavigation(); + + navigation.revealLine("a", "new", 0); + navigation.revealLine("a", "new", -3); + navigation.revealLine("a", "new", 2.5); + navigation.revealLine("a", "new", Number.NaN); + navigation.revealLine("a", "new", "4" as unknown as number); + + expect(revealedLines).toEqual([]); + expect(warnings).toEqual( + Array.from( + { length: 5 }, + () => 'Extension triage revealLine received an invalid line number for "a"', + ), + ); + }); + + test("stays quiet when the host falls back to the hunk containing the line", () => { + // A line inside a collapsed gap has no row to scroll to; landing on its hunk is the + // honest best effort, not a failure worth a toast. + const { navigation, warnings } = createTestNavigation({ revealResult: "hunk" }); + + navigation.revealLine("a", "new", 42); + + expect(warnings).toEqual([]); + }); + + test("warns when no hunk of the file covers the requested line", () => { + const { navigation, warnings } = createTestNavigation({ revealResult: "none" }); + + navigation.revealLine("a", "new", 9001); + + expect(warnings).toEqual(['Extension triage revealLine found no new line 9001 in "a"']); + }); + test("validates against the files visible at call time, not at creation", () => { // A command handler may await a dialog while a reload or filter changes // the review; navigation must judge the target against the current list. diff --git a/src/ui/lib/extensionNavigation.ts b/src/ui/lib/extensionNavigation.ts index 468ccc151..3a3232571 100644 --- a/src/ui/lib/extensionNavigation.ts +++ b/src/ui/lib/extensionNavigation.ts @@ -46,6 +46,13 @@ export interface CreateGuardedReviewNavigationOptions { notify: ExtensionNotifySink; onSelectFile: (fileId: string) => void; onSelectHunk: (fileId: string, hunkIndex: number) => void; + /** + * Jump to one source line, reporting which target the review could reach. + * + * `"line"` and `"hunk"` both moved the review; `"none"` means the line is nowhere in the + * file, which only this guard can report with the extension's name on it. + */ + onRevealLine: (fileId: string, side: "old" | "new", line: number) => "line" | "hunk" | "none"; } /** @@ -65,6 +72,7 @@ export function createGuardedReviewNavigation({ notify, onSelectFile, onSelectHunk, + onRevealLine, }: CreateGuardedReviewNavigationOptions): ExtensionReviewNavigation { /** Resolve a navigation target, or report one the review stream cannot show. */ const resolveVisibleFile = (method: string, fileId: string) => { @@ -124,5 +132,37 @@ export function createGuardedReviewNavigation({ onSelectHunk(fileId, Math.min(Math.max(0, Math.floor(hunkIndex)), maxHunkIndex)); }); }, + revealLine(fileId: string, side: "old" | "new", line: number) { + guard("revealLine", () => { + if (!resolveVisibleFile("revealLine", fileId)) { + return; + } + + // Line numbers are 1-based whole numbers as a patch writes them; anything else is a + // bug in the caller rather than a line the review failed to find. + if (side !== "old" && side !== "new") { + notify( + `Extension ${extensionId} revealLine received an invalid side for "${fileId}"`, + "warning", + ); + return; + } + + if (typeof line !== "number" || !Number.isInteger(line) || line < 1) { + notify( + `Extension ${extensionId} revealLine received an invalid line number for "${fileId}"`, + "warning", + ); + return; + } + + if (onRevealLine(fileId, side, line) === "none") { + notify( + `Extension ${extensionId} revealLine found no ${side} line ${line} in "${fileId}"`, + "warning", + ); + } + }); + }, }); } diff --git a/src/ui/lib/hunkScroll.ts b/src/ui/lib/hunkScroll.ts index 60fb30e90..44f7252c9 100644 --- a/src/ui/lib/hunkScroll.ts +++ b/src/ui/lib/hunkScroll.ts @@ -57,6 +57,16 @@ export function computeLineAlignmentScrollTop({ return Math.max(0, top - Math.floor((viewport - height) / 2)); } +/** + * How far a current-line reveal may move the viewport. + * + * `"nearest"` is stepping: move only as far as it takes to bring the line on screen. + * `"reveal"` is a jump to somewhere the reviewer was not looking, so it lands the line where + * hunk and note reveals land it, a little below the viewport top, even when the line already + * happened to be visible. + */ +export type LineRevealPlacement = "nearest" | "reveal"; + /** * Pick a scroll target that brings the current line just into view. * diff --git a/src/ui/lib/lineCursors.test.ts b/src/ui/lib/lineCursors.test.ts index bc4d430ec..6a44b858e 100644 --- a/src/ui/lib/lineCursors.test.ts +++ b/src/ui/lib/lineCursors.test.ts @@ -11,6 +11,7 @@ import { resolveTheme } from "../themes"; import { buildLineCursors, clampLineCursorToViewport, + findLineCursorAt, findNextLineCursor, firstLineCursorInHunk, resolveLineCursor, @@ -194,6 +195,60 @@ describe("buildLineCursors", () => { }); }); +describe("findLineCursorAt", () => { + /** Build a file whose inserted line pushes the trailing context onto different side numbers. */ + function createShiftedContextFile() { + return createTestDiffFile({ + id: "alpha", + path: "alpha.ts", + before: lines("one", "two", "three"), + after: lines("one", "inserted", "two", "three"), + context: 3, + }); + } + + test("finds a changed line by the side the patch numbers it on", () => { + const cursors = cursorsFor([createTwoHunkFile("alpha", "alpha.ts")], "stack"); + + expect(findLineCursorAt(cursors, "alpha", "new", 10)?.target).toEqual({ + side: "new", + line: 10, + }); + expect(findLineCursorAt(cursors, "alpha", "old", 10)?.target).toEqual({ + side: "old", + line: 10, + }); + }); + + test("answers a context row to either side's number, even once they diverge", () => { + // "three" is old line 3 and new line 4 after the insertion; both address the same row. + const cursors = cursorsFor([createShiftedContextFile()], "stack"); + const byNew = findLineCursorAt(cursors, "alpha", "new", 4); + const byOld = findLineCursorAt(cursors, "alpha", "old", 3); + + expect(byNew?.stableKey).toBe("line:0:context:3:4"); + expect(byOld).toEqual(byNew); + }); + + test("stays inside the requested file when two files number the same line", () => { + const cursors = cursorsFor( + [createTwoHunkFile("alpha", "alpha.ts"), createTwoHunkFile("beta", "beta.ts")], + "stack", + ); + + expect(findLineCursorAt(cursors, "beta", "new", 1)?.fileId).toBe("beta"); + }); + + test("finds no cursor for a line the stream draws no row for", () => { + // Line 3 is inside the collapsed gap above the only hunk, so nothing measures it. + const cursors = cursorsFor([createCollapsedGapFile()], "stack"); + + expect(findLineCursorAt(cursors, "alpha", "new", 3)).toBeNull(); + expect(findLineCursorAt(cursors, "alpha", "new", 900)).toBeNull(); + expect(findLineCursorAt(cursors, "missing", "new", 6)).toBeNull(); + }); +}); + describe("findNextLineCursor", () => { const cursors = cursorsFor( [createTwoHunkFile("alpha", "alpha.ts"), createTwoHunkFile("beta", "beta.ts")], diff --git a/src/ui/lib/lineCursors.ts b/src/ui/lib/lineCursors.ts index 17c9af2f3..9329dce0a 100644 --- a/src/ui/lib/lineCursors.ts +++ b/src/ui/lib/lineCursors.ts @@ -8,6 +8,7 @@ import type { DiffFile, UserNoteLineTarget } from "../../core/types"; import type { DiffSectionGeometry, DiffSectionRowBounds } from "../diff/diffSectionGeometry"; import { + contextLineStableKeySides, contextLineStableKeyTarget, lineStableKey, lineStableKeyTarget, @@ -195,6 +196,39 @@ export function lineCursorAt( ); } +/** + * Check whether one cursor stands on the source line a caller named. + * + * A context row carries a single new-side target but renders one line under both sides' + * numbers, so its old-side number addresses the same stop. + */ +function lineCursorAddresses(cursor: LineCursor, side: "old" | "new", line: number) { + if (cursor.target.side === side && cursor.target.line === line) { + return true; + } + + const context = contextLineStableKeySides(cursor.stableKey); + return context !== null && (side === "old" ? context.oldLine : context.newLine) === line; +} + +/** + * Find the rendered stop one file's source line sits on, addressed by side and number. + * + * Answers only for lines the review stream actually draws: a line hidden inside a collapsed + * gap, or absent from a partial patch, has no cursor and no measured row to scroll to. + */ +export function findLineCursorAt( + cursors: LineCursor[], + fileId: string, + side: "old" | "new", + line: number, +): LineCursor | null { + return ( + cursors.find((cursor) => cursor.fileId === fileId && lineCursorAddresses(cursor, side, line)) ?? + null + ); +} + /** Read one cursor's measured extent in whole-stream rows. */ export type LineCursorBoundsLookup = (cursor: LineCursor) => VerticalBounds | undefined; diff --git a/test/pty/extensions-integration.test.ts b/test/pty/extensions-integration.test.ts index b4638ffb0..f55935596 100644 --- a/test/pty/extensions-integration.test.ts +++ b/test/pty/extensions-integration.test.ts @@ -139,6 +139,79 @@ const LINE_HIGHLIGHT_EXTENSION_SOURCE = `export default function (hunk) { } `; +/** + * A single hunk tall enough that its anchor and its last lines cannot share a + * viewport, with one unmistakable token near the bottom. + * + * Every line differs, so git emits one hunk spanning the whole file — the shape + * `selectHunk` cannot navigate usefully. + */ +const REVEAL_LINE_TARGET = 111; +const REVEAL_LINE_TOKEN = "REVEALLINETOKEN"; +const TALL_HUNK_FILE = { + path: "tall.ts", + before: `${Array.from( + { length: 130 }, + (_, index) => `export const line${String(index + 1).padStart(3, "0")} = ${index + 1};`, + ).join("\n")}\n`, + after: `${Array.from({ length: 130 }, (_, index) => + index + 1 === REVEAL_LINE_TARGET + ? `export const needle = "${REVEAL_LINE_TOKEN}";` + : `export const line${String(index + 1).padStart(3, "0")} = ${index + 1001};`, + ).join("\n")}\n`, +}; + +/** + * An extension that jumps to one exact line of the reviewed file. + * + * The second command asks for a line no hunk covers, so the same session shows + * both halves of the contract: a reachable line scrolls, an unreachable one + * comes back as a warning naming the extension. + */ +const REVEAL_LINE_EXTENSION_SOURCE = `export default function (hunk) { + hunk.registerCommand({ id: "jump", title: "Jump to the needle", key: "f7" }, (ctx) => { + const file = ctx.selection.file; + if (file) ctx.navigation.revealLine(file.id, "new", ${REVEAL_LINE_TARGET}); + }); + hunk.registerCommand({ id: "jump-nowhere", title: "Jump past the file", key: "f8" }, (ctx) => { + const file = ctx.selection.file; + if (file) ctx.navigation.revealLine(file.id, "new", 9001); + }); +} +`; + +/** + * An extension that jumps through pane actions captured at mount. + * + * This is the shape the less-search example uses: a keyboard mode cannot + * navigate, so it leaves the jump for the mounted pane, whose \`actions\` were + * minted on the pane's first render — before the diff pane had published any + * measured line cursors — and are documented to stay valid while the pane is + * mounted. A \`revealLine\` that reads its own stale closure instead of the + * live cursor list silently degrades this exact call to the hunk fallback. + */ +const REVEAL_LINE_MOUNT_ACTIONS_EXTENSION_SOURCE = `import { createElement } from "react"; +let capturedActions = null; +export default function (hunk) { + hunk.registerPane({ + id: "capture", + placement: "bottom", + defaultOpen: true, + height: { preferred: 1, min: 1, max: 1 }, + component: (props) => { + if (capturedActions === null) capturedActions = props.actions; + return createElement("text", { content: "CAPTURE PANE", style: { fg: props.theme.text } }); + }, + }); + hunk.registerCommand({ id: "jump-held", title: "Jump via held actions", key: "f7" }, (ctx) => { + const file = ctx.selection.file; + if (file && capturedActions) { + capturedActions.revealLine(file.id, "new", ${REVEAL_LINE_TARGET}); + } + }); +} +`; + const DIALOG_EXTENSION_SOURCE = `export default function (hunk) { hunk.registerCommand({ id: "ask", title: "Ask", key: "y" }, async (ctx) => { const proceed = await ctx.dialogs.confirm({ @@ -690,6 +763,101 @@ describe("PTY extensions", () => { } }); + test("revealLine lands a line deep inside one tall hunk near the viewport top", async () => { + const configHome = harness.createIsolatedConfigHome(); + const fixture = harness.createRepoExtensionFixture(REVEAL_LINE_EXTENSION_SOURCE, "fixture.ts", [ + TALL_HUNK_FILE, + ]); + const session = await harness.launchHunk({ + args: [ + "diff", + "--mode", + "stack", + "--extension", + join(fixture.dir, ".hunk", "extensions", "fixture.ts"), + ], + cwd: fixture.dir, + cols: 140, + rows: 24, + env: { XDG_CONFIG_HOME: configHome }, + }); + + try { + const review = await harness.waitForSnapshot( + session, + (text) => text.includes("tall.ts"), + 20_000, + ); + // This is the bug: the hunk anchor is on screen, the marked line is pages below it. + expect(review).not.toContain(REVEAL_LINE_TOKEN); + await harness.ensureKeyboardIsLive(session); + + await session.press("f7"); + const revealed = await harness.waitForSnapshot( + session, + (text) => text.includes(REVEAL_LINE_TOKEN), + 20_000, + ); + // Near the top of a 24-row terminal, where every other Hunk reveal lands: + // a little below the viewport edge, not scrolled just barely into view. + const row = lineIndexOf(revealed, REVEAL_LINE_TOKEN); + expect(row).toBeGreaterThan(0); + expect(row).toBeLessThan(12); + + await session.press("f8"); + const warned = await session.waitForText(/revealLine found no/, { timeout: 20_000 }); + expect(warned).toContain("Extension fixture revealLine found no new line 9001"); + } finally { + session.close(); + } + }); + + test("revealLine through pane actions held since mount still lands the line", async () => { + // The first deferred jump of a session runs against actions minted before + // any cursors were measured; it must land on the line, not the hunk anchor. + const configHome = harness.createIsolatedConfigHome(); + const fixture = harness.createRepoExtensionFixture( + REVEAL_LINE_MOUNT_ACTIONS_EXTENSION_SOURCE, + "fixture.ts", + [TALL_HUNK_FILE], + ); + const session = await harness.launchHunk({ + args: [ + "diff", + "--mode", + "stack", + "--extension", + join(fixture.dir, ".hunk", "extensions", "fixture.ts"), + ], + cwd: fixture.dir, + cols: 140, + rows: 24, + env: { XDG_CONFIG_HOME: configHome }, + }); + + try { + const review = await harness.waitForSnapshot( + session, + (text) => text.includes("tall.ts") && text.includes("CAPTURE PANE"), + 20_000, + ); + expect(review).not.toContain(REVEAL_LINE_TOKEN); + await harness.ensureKeyboardIsLive(session); + + await session.press("f7"); + const revealed = await harness.waitForSnapshot( + session, + (text) => text.includes(REVEAL_LINE_TOKEN), + 20_000, + ); + const row = lineIndexOf(revealed, REVEAL_LINE_TOKEN); + expect(row).toBeGreaterThan(0); + expect(row).toBeLessThan(12); + } finally { + session.close(); + } + }); + test("a startup handler's notify renders as a toast and clears itself", async () => { const configHome = harness.createIsolatedConfigHome(); const fixture = harness.createRepoExtensionFixture(NOTIFY_EXTENSION_SOURCE); diff --git a/test/pty/harness.ts b/test/pty/harness.ts index d95f314e3..cb4fcece7 100644 --- a/test/pty/harness.ts +++ b/test/pty/harness.ts @@ -516,20 +516,37 @@ export function createPtyHarness() { * test shows only the two changed source files, keeping snapshot assertions * about the extension's effect unambiguous. */ - function createRepoExtensionFixture(source: string, entryName = "fixture.ts") { + function createRepoExtensionFixture( + source: string, + entryName = "fixture.ts", + changedFiles: ChangedFileSpec[] = [ + { + path: "alpha.ts", + before: "export const alpha = 1;\n", + after: "export const alphaValue = 2;\n", + }, + { + path: "beta.ts", + before: "export const beta = 1;\n", + after: "export const betaValue = 2;\n", + }, + ], + ) { const dir = makeTempDir("hunk-tuistory-extension-"); runGit(["init"], dir); runGit(["config", "user.name", "Pi"], dir); runGit(["config", "user.email", "pi@example.com"], dir); - writeText(join(dir, "alpha.ts"), "export const alpha = 1;\n"); - writeText(join(dir, "beta.ts"), "export const beta = 1;\n"); + for (const file of changedFiles) { + writeText(join(dir, file.path), file.before); + } writeText(join(dir, ".hunk", "extensions", entryName), source); runGit(["add", "."], dir); runGit(["commit", "-m", "initial"], dir); - writeText(join(dir, "alpha.ts"), "export const alphaValue = 2;\n"); - writeText(join(dir, "beta.ts"), "export const betaValue = 2;\n"); + for (const file of changedFiles) { + writeText(join(dir, file.path), file.after); + } return { dir }; } diff --git a/website/src/content/docs/docs/extend/extension-api.md b/website/src/content/docs/docs/extend/extension-api.md index e55405ed2..e54b0970e 100644 --- a/website/src/content/docs/docs/extend/extension-api.md +++ b/website/src/content/docs/docs/extend/extension-api.md @@ -7,7 +7,7 @@ The extension factory receives one API object. Registration calls are only valid ## `hunk.apiVersion` -The API generation this Hunk speaks (currently `5`). Version 5 adds line highlighters; version 4 added keyboard modes and docked panes, with API-v3 sidebar names remaining as deprecated aliases. +The API generation this Hunk speaks (currently `5`). Version 5 adds line highlighters and line-granular navigation (`revealLine`); version 4 added keyboard modes and docked panes, with API-v3 sidebar names remaining as deprecated aliases. ## `hunk.registerTheme(theme)` @@ -168,7 +168,9 @@ hunk.registerCommand( `selection.file` is a frozen view, identical to a pane's `files` entries; it is `null` only when no files are visible. `selection.hunkIndex` is `null` whenever `file` is, or when the file has no hunks. The values are captured when the command fires, so an async handler keeps the selection it started from. -`ctx.navigation.selectFile(fileId)` and `selectHunk(fileId, hunkIndex)` route through the same guarded review controller as a pane's `actions` — the stream scrolls, selection updates, `selection_changed` fires. Unlike `selection` it is live: a handler that awaits a dialog and then navigates still works. +`ctx.navigation.selectFile(fileId)`, `selectHunk(fileId, hunkIndex)`, and `revealLine(fileId, side, line)` route through the same guarded review controller as a pane's `actions` — the stream scrolls, selection updates, `selection_changed` fires. Unlike `selection` it is live: a handler that awaits a dialog and then navigates still works. + +`revealLine` is the finest target: a hunk hundreds of lines tall has one anchor, so `selectHunk` can leave the line you meant pages below the viewport. `line` is 1-based on `side` as the patch numbers it, so a context line answers to either side's number. The revealed line lands a little below the viewport top — where every other Hunk reveal lands — and becomes the current line, pairing with a mark from `registerLineHighlighter`. A line no rendered row carries (inside a collapsed gap, absent from a partial patch, or with the current-line marker off) falls back to the hunk containing it; a line no hunk covers, a side outside `"old"`/`"new"`, and a line number that is not a positive whole number are refused with a warning naming the extension. All built-ins listed in the [keybindings reference](https://github.com/modem-dev/hunk/blob/main/docs/keybindings.md) are public to command handlers. This includes the unbound `hunk.review.alignCurrentLineTop`, `hunk.review.alignCurrentLineCenter`, and `hunk.review.alignCurrentLineBottom` commands. `count` defaults to `1`, is capped at `10,000`, and scales relative row, viewport, horizontal, file, hunk, and annotated navigation in one host transition. Absolute and one-shot commands run once. Unknown, disabled, non-public, extension-owned, or stale commands return `false`. `isEnabled` also returns `false` for a malformed id; malformed `execute` ids, options, and counts throw into normal extension failure containment.