Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions packages/app/e2e/regression/review.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,46 @@ test("rereads a file each time an artifact link opens it", async ({ page }) => {
await expect(missingTab).toHaveCount(0)
})

test("a file link scrolls its line into view in a new or open file tab", async ({ page }) => {
// Every other line wraps, so a line's offset cannot be computed from the line height.
const source = Array.from(
{ length: 900 },
(_, index) => `export const value${index + 1} = "${"wrapped ".repeat(index % 2 === 0 ? 24 : 1)}"`,
).join("\n")

await openSession(page, {
name: "LineLink",
fileList: () => [],
findFiles: () => [fileNode("C:/OpenCode/LineLink", "src/long.ts")],
fileContent: () => source,
pageMessages: () => ({
items: [
{ id: "msg_prompt", type: "user", text: "Where is it?", time: { created: 1 } },
{
id: "msg_reply",
type: "assistant",
agent: "build",
model: { id: "test", providerID: "opencode" },
content: [{ type: "text", text: "See `src/long.ts:658-659`, then `src/long.ts:120`." }],
time: { created: 2, completed: 3 },
},
],
}),
})

const panel = page.locator("#review-panel")

for (const link of [
{ name: "src/long.ts:658-659", line: 658 },
{ name: "src/long.ts:120", line: 120 },
]) {
await page.getByText(link.name, { exact: true }).click()
const row = panel.locator(`[data-line="${link.line}"]`)
await expect(row).toHaveAttribute("data-selected-line", /.+/)
await expect(row).toBeInViewport()
}
})

/** What the image case's frame probe records on the window. */
type ImageProbe = { problems: string[]; frames: number; running: boolean }

Expand Down
4 changes: 3 additions & 1 deletion packages/gui-extensions/src/file/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ export interface FileShared {
get(session: string, path: string): LineRange | null | undefined
set(session: string, files: Record<string, LineRange | null>): void
}
/** Reveals a selected line range on a mounted file view when a link action targets it. */
/** Reveals a selected line range on a file view when a link action targets it. */
readonly reveal: {
register(session: string, path: string, run: () => void): () => void
run(session: string, path: string): void
/** Whether a reveal ran since the last take; the view scrolls to the selection once its row renders. */
take(session: string, path: string): boolean
}
/** The tab is the session's selected side tab. */
active(session: MountedSession, id: string): boolean
Expand Down
5 changes: 5 additions & 0 deletions packages/gui-extensions/src/file/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const setup: Setup<typeof File> = (ctx) => {
const [filters, setFilters] = createStore<Record<string, { text: string; browsing: boolean } | undefined>>({})

const revealListeners = new Map<string, Set<() => void>>()
const pendingReveals = new Set<string>()
let clickController: AbortController | undefined

// Tab objects per session screen, which stays while it routes another session, reused so neither strip updates nor a
Expand Down Expand Up @@ -216,8 +217,12 @@ const setup: Setup<typeof File> = (ctx) => {
}
},
run(session, path) {
pendingReveals.add(`${session}\n${path}`)
revealListeners.get(`${session}\n${path}`)?.forEach((fn) => fn())
},
take(session, path) {
return pendingReveals.delete(`${session}\n${path}`)
},
},
active,
open,
Expand Down
63 changes: 52 additions & 11 deletions packages/gui-extensions/src/file/view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,14 @@ type NoteState = { openedComment: string | null; commenting: LineRange | null; s

type FindState = { find: FileSearchHandle | null }

function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos: ScrollPos) => void }) {
// Four unwrapped lines above a revealed line: the context the estimate in `renderer.tsx` leaves.
const REVEAL_MARGIN = 4 * 24

function createScrollSync(input: {
get: () => ScrollPos | undefined
set: (pos: ScrollPos) => void
restored: () => void
}) {
const state: ScrollSyncState = {}
const [code, setCode] = createSignal<HTMLElement[]>([])

Expand Down Expand Up @@ -133,7 +140,7 @@ function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos:

const pos = input.get()

if (!pos) return
if (!pos) return input.restored()

sync()

Expand All @@ -145,9 +152,21 @@ function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos:

if (el.scrollTop !== pos.y) el.scrollTop = pos.y

if (code().length > 0) return
if (code().length === 0 && el.scrollLeft !== pos.x) el.scrollLeft = pos.x

input.restored()
}

// Scrolls a rendered line to the top of the view, with a few lines of context above it.
const scrollToLine = (line: number) => {
const el = state.scroll
const row = el?.querySelector("diffs-container")?.shadowRoot?.querySelector(`[data-line="${line}"]`)

if (!el || !(row instanceof HTMLElement)) return false

el.scrollTop += row.getBoundingClientRect().top - el.getBoundingClientRect().top - REVEAL_MARGIN

if (el.scrollLeft !== pos.x) el.scrollLeft = pos.x
return true
}

const queueRestore = () => {
Expand Down Expand Up @@ -185,6 +204,7 @@ function createScrollSync(input: { get: () => ScrollPos | undefined; set: (pos:
return {
handleScroll,
queueRestore,
scrollToLine,
setViewport,
}
}
Expand Down Expand Up @@ -239,9 +259,27 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio
return shared.handoff.get(props.session.key, p) ?? null
})

// A link's stored scroll is an estimate from unwrapped line heights; wrapped lines and annotations push the real row
// further down. The link's reveal is taken when the stored scroll restores, so a view the link mounts sees it too, and
// stays pending until the row is measured or the rendered file has no row for it.
const reveal = { pending: false, rendered: false }

const scrollSync = createScrollSync({
get: () => layout.scroll.get(props.session, key()),
set: (pos) => layout.scroll.set(props.session, key(), pos),
restored: () => {
const p = path()

if (p && shared.reveal.take(props.session.key, p)) reveal.pending = true

if (!reveal.pending) return

const range = selectedLines()

if ((range && scrollSync.scrollToLine(Math.min(range.start, range.end))) || reveal.rendered) {
reveal.pending = false
}
},
})

const selectionPreview = (source: string, selection: FileSelection) => {
Expand Down Expand Up @@ -448,21 +486,22 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio
},
)

const renderFile = (source: string) => (
const renderFile = () => (
<div class="relative overflow-hidden pb-40">
<Dynamic
component={fileComponent}
mode="text"
file={{
name: path() ?? "",
contents: source,
contents: contents(),
cacheKey: cacheKey(),
}}
enableLineSelection
enableGutterUtility
selectedLines={activeSelection()}
commentedLines={commentedLines()}
onRendered={() => {
reveal.rendered = true
scrollSync.queueRestore()
}}
annotations={commentsUi.annotations()}
Expand Down Expand Up @@ -492,10 +531,12 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio
</div>
)

// The code view scrolls inside ScrollView so line state and scroll position persist per tab.
const codeView = (source: string) => (
// The code view scrolls inside ScrollView so line state and scroll position persist per tab. It reads the text through
// `contents()`, not the loaded value, so a reread (every link click forces one) updates it in place instead of
// remounting it scrolled to the top.
const codeView = () => (
<ScrollView class="min-h-0 flex-1" viewportRef={scrollSync.setViewport} onScroll={scrollSync.handleScroll}>
{renderFile(source)}
{renderFile()}
</ScrollView>
)

Expand All @@ -504,13 +545,13 @@ export function SessionFileView(props: { session: MountedSession; screen: Sessio
<Switch>
<Match when={current()?.loaded ? current()?.content : undefined}>
{(value) => (
<Show when={artifact()} fallback={codeView(value().content)}>
<Show when={artifact()} fallback={codeView()}>
<ArtifactView
session={props.session}
path={path() ?? ""}
content={value()}
cacheKey={cacheKey()}
source={codeView(value().content)}
source={codeView()}
/>
</Show>
)}
Expand Down
Loading