Skip to content

fix(app): scroll file links to their line in wrapped files - #58037

Open
opencode-agent[bot] wants to merge 1 commit into
devfrom
file-link-scroll
Open

opencode-agent[bot] wants to merge 1 commit into
devfrom
file-link-scroll

Conversation

@opencode-agent

Copy link
Copy Markdown
Contributor

Clicking a file link with a line number (e.g. rate-limit.ts:658-659) opened the file but left the line out of view.

Two causes:

  • Wrapped lines. The link stored a scroll estimate of (line - 4) * 24px. In a file with wrapped lines the real row sits much further down, so the view landed hundreds of lines early. The view now measures the selected row once the file renders and scrolls to it, leaving the same four lines of context. The estimate still covers the first paint. A link can mount the view, so the reveal it requests stays pending in shared.reveal until the view takes it.
  • Already open file. A link into an open tab forces a reread. The reread replaced the loaded value the code view was built from, so the Show fallback rebuilt the ScrollView and it remounted at the top. The view now reads the text through contents(), which updates the open view in place.

Added a review.spec.ts case covering a link into a new tab and a link into an open tab, using a file with wrapped lines. It fails on dev at the first step and passes with the fix.

Not covered: files over 500 KB render virtualized, and links into those still land in the wrong place, both before and after this change. Pierre has no API that maps a line to a scroll offset for virtualized files.

Requested by: @Brendonovich (Brendan via Slack)

The stored scroll for a file link was an estimate from a fixed 24px line
height, so wrapped lines left the target far below the view. Once the
file renders, measure the selected row and scroll to it.

Clicking a link into an already open file also remounted the code view
at the top: the reread replaced the loaded value the view was built
from. Read the text through contents() so the view updates in place.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks sound to me. I ran the new review.spec.ts case on the base, where it fails at its first step: the src/long.ts:658-659 link opens the file but the view sits around lines 262–275. With this change it passes, along with the rest of review.spec.ts (24 tests). Lines 658–659 come into view, including for the link into the already open tab. The gui-extensions typecheck, its src/file unit tests and oxlint on the changed files all pass.

The public API is unchanged. FileShared.reveal.take is internal to the file extension.

I have no blocking concerns.

Before

After

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant