Report the hovered OSC 8 URI so a host can show where a link goes - #7
Merged
Conversation
…ere a link goes The view paints an underline on every OSC 8 cell and swaps the cursor on hover, but the URI itself never leaves it -- so a host has no way to disclose a destination before opening it, and OSC 8 lets a link's text disagree with its target. Fires only on a real change, so a host may rebuild on it: moving within one link is silent. A controller swap reports for itself, deferred to after the frame because didUpdateWidget runs mid-build and a host rebuilding there would be setting state during build. dispose deliberately does not fire, for the same reason.
PR #6 put `_explicitHyperlinkAt` in the `onHover` guard so a link under a full-screen agent would paint as reachable. That call resolves a cell through `_positionForOffset`, which asked `controller.snapshot` whether the buffer was empty — and reading the snapshot settles the styled formatter: three full-buffer passes plus a re-parse of the styled output. Under `GhosttyTerminalRendererMode.renderState` the painter never reads that snapshot (`snapshotOf` is a lazy getter behind `_paintsTranscript`), so nothing else was paying for it and the hover paid the whole cost alone. Any output since the last read marks it stale, which in an agent session is every frame — so moving the mouse over a live terminal cost a full transcript rebuild per hover event. Measured at ~17ms per hover on a 1800-line scrollback, on the surface where the pointer spends all its time. Selection drags resolve cells the same way and paid it too. Ask the engine instead. `_scrollableLineCount` already prefers the native scrollback total and falls back to the formatter only where there is no engine geometry to ask — web, and before the terminal exists, which is where the formatter is what renders anyway. It was already being called a few lines below for `maxRow`, so this also drops a duplicate read. The guard is timed against the same loop with the pointer held still: both take the same output and repaint the same frames, so what separates them is the hover alone, and a ratio survives a machine an absolute bound would not. 862ms vs 179ms with the old guard; even under the bound with it gone.
bharathm03
added a commit
to antgrid-ai/antgrid
that referenced
this pull request
Aug 27, 2026
…transcript PR #6 put a cell resolution in the onHover guard, and that resolution asked the controller's snapshot whether the buffer was empty -- which settles the styled formatter. In renderState mode, which this app uses, the painter never reads that snapshot, so the hover paid a full transcript rebuild alone: ~17ms per hover event over a terminal that is still producing output, which is every hover in an agent session. antgrid-ai/dart_terminal#7 now asks the engine's own row total instead.
bharathm03
added a commit
to antgrid-ai/antgrid
that referenced
this pull request
Aug 27, 2026
…transcript PR #6 put a cell resolution in the onHover guard, and that resolution asked the controller's snapshot whether the buffer was empty -- which settles the styled formatter. In renderState mode, which this app uses, the painter never reads that snapshot, so the hover paid a full transcript rebuild alone: ~17ms per hover event over a terminal that is still producing output, which is every hover in an agent session. antgrid-ai/dart_terminal#7 now asks the engine's own row total instead.
bharathm03
added a commit
to antgrid-ai/antgrid
that referenced
this pull request
Aug 27, 2026
…misread (#28) * fix: confirm a terminal link whose URI is built to be misread, on desktop too Desktop skipped the confirm sheet on the grounds that the browser's address bar discloses the destination. It does -- but disclosure is not the same as being read, and OSC 8 payloads exist that are written specifically to be misread at a glance: a `github.com@` userinfo prefix parks a familiar name where the eye stops, and a punycoded or percent-encoded host spells a lookalike in characters that render as the real thing. Pull those three shapes back to the sheet, which names the host on its own line. Judged from the URI alone, which is all a terminal hyperlink hands over -- so a lookalike that is honestly its own host (`github.com.evil.example`) is not caught, and the test says so by name rather than leaving the gap to be mistaken for coverage. * feat: show where a terminal link goes before it is opened The terminal view paints an OSC 8 cell underlined and swaps the cursor to a pointer, and that is the whole affordance -- nothing anywhere shows the URI. Since the link's visible text is free to disagree with its target, the first place the destination appeared was the browser it had already been opened in. Wire the fork's new `onHyperlinkHover` to a readout that floats near the pointer: the host reads brightest, the prefix before it dimmest, so a userinfo impostor renders as the decoration it is. Parked below the pointer and flipped above near the bottom edge, because the bottom line of the panel is the prompt the user is typing into. The pointer position is sampled only at the instant the hovered URI changes, so a hover frame costs no rebuild and the card does not slide around under the cursor it belongs to. The fork pin moves to the hover-callback branch commit; it must be re-pinned to the merged master SHA before this lands. * fix: make the hover readout expose the shapes it exists to expose Review of the first cut found the readout confirming the lie in three ways. Locating the host with `indexOf` of the parsed value finds the FIRST occurrence, so `https://b.com.evil@b.com/x` painted the userinfo copy bright; a raw unicode host never occurs in the payload at all, because `Uri.host` percent-encodes it, so the homoglyph case silently lost its emphasis; and a single rich run elides its TAIL, which behind a padded userinfo IS the host — the card rendered as a clean GitHub URL. Locate the host positionally instead (scheme, authority, last `@`, bracketed IPv6 or port colon), and lay the readout out as three siblings with the host as the only non-flex child, so the elidable parts surrender their width first and the host is cut last. Also: spell out bidi and control characters the way the launcher receives them, mask the password half of a userinfo (a hover is not consent to display a secret), and cap each part so an unbounded payload is not shaped in full on the UI thread. The anchor now comes from an enclosing MouseRegion through a pending-URI handshake: the view reports the URI from a region BELOW ours and hit paths dispatch child-first, so reading the position at that instant was always one event stale — and Offset.zero on the first hover, which parked the card in the corner. That region's onExit is also what clears a card whose terminal was unmounted under the pointer. Hover state moved to a ValueNotifier so crossing a wall of links no longer rebuilds the whole panel. The preview tests were pumping without the palette extension, so both sides of every colour assertion resolved to the same fallback. * chore: bump the dart_terminal pin so hovering no longer rebuilds the transcript PR #6 put a cell resolution in the onHover guard, and that resolution asked the controller's snapshot whether the buffer was empty -- which settles the styled formatter. In renderState mode, which this app uses, the painter never reads that snapshot, so the hover paid a full transcript rebuild alone: ~17ms per hover event over a terminal that is still producing output, which is every hover in an agent session. antgrid-ai/dart_terminal#7 now asks the engine's own row total instead. * fix: skip the confirmation only when the app actually showed the destination The desktop branch tested `defaultTargetPlatform`, which answers a question about the OS, not about what the user was shown. A finger tap on a Windows or Linux touchscreen took that branch on the grounds that the hover readout had disclosed the destination -- and there is no hover on touch. Same hole for a Shift chord, and for a link that scrolled out from under a resting pointer between the hover and the click. The terminal knows the answer without asking anyone: the readout is up, for a specific URI, or it is not. Pass that in as `disclosed` and the platform test disappears -- along with the class of bug where a new input path silently inherits an exemption it was never measured for. Defaults to false, so a caller with no readout cannot inherit a claim by omission: "we showed it" belongs to the surface that showed it. * chore: re-pin dart_terminal to the merged master commit The hover callback and its perf fix are on antgrid-ai/dart_terminal master now, so the pin no longer has to name a PR branch that could be rewritten or deleted under us.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The view paints an underline on every OSC 8 cell (
_nativeTextDecoration) and swaps the cursor to a pointer on hover, but the URI itself never leaves the view --_hoveredHyperlinkdrives exactly one thing,SystemMouseCursors.click. So a host has no way to disclose a destination before opening it, and OSC 8 is precisely the case where the visible text may disagree with the target.Adds
onHyperlinkHover(ValueChanged<String?>).Contract
didUpdateWidgetruns mid-build, and a host that rebuilt on the callback there would be setting state during build. The pointer has not moved in that case, so without it the host is left showing a link from a terminal that is gone. Tested, and negative-checked by removing the notify.disposedeliberately does not fire, for the same phase reason. Documented on the field.Cost is not a concern:
hyperlinkUriAtmeasures 0.481 us on a link cell, 0.313 us off one, 0.189 us outside the grid (200k iterations, warmed). At 120 Hz pointer motion that is ~60 us/sec, so no per-cell memoization is needed.Tests
Three added: enter/leave reports the URI then null and stays silent across the link; leaving the view entirely reports null; a controller swap clears with the pointer stationary.
ghostty_vte_flutter:+211 -28, the 28 pre-existing onmaster.flutter analyzeclean,dart format --set-exit-if-changed .clean across 104 files.Second commit: a perf regression from #6, found while wiring this up
614e05dis not part of the hover callback — it fixes something #6 shipped, and it lands here because this is the branch the consumer pins.#6 added
_explicitHyperlinkAtto theonHoverguard so a link under a full-screen agent would paint as reachable. That call resolves a cell through_positionForOffset, which askedcontroller.snapshotwhether the buffer was empty — and reading the snapshot settles the styled formatter: three full-buffer passes plus a re-parse of the styled output.Under
GhosttyTerminalRendererMode.renderStatethe painter never reads that snapshot (snapshotOfis a lazy getter behind_paintsTranscript), so nothing else was paying for it. Any output since the last read marks it stale — which in an agent session is every frame — so moving the mouse over a live terminal cost a full transcript rebuild per hover event. Measured at ~17ms per hover on an 1800-line scrollback. Selection drags resolve cells the same way and paid it too.The fix asks the engine instead:
_scrollableLineCountalready prefers the native scrollback total and falls back to the formatter only where there is no engine geometry (web, and before the terminal exists) — where the formatter is what renders anyway. It was already being called a few lines below formaxRow, so this drops a duplicate read as well.The guard in
terminal_view_hover_cost_test.darttimes the same loop with the pointer moving against the pointer held still: both take the same output and repaint the same frames, so what separates them is the hover alone, and a ratio survives a machine that an absolute millisecond bound would not. 862ms vs 179ms with the old guard; comfortably under the bound with it gone.Package suite:
+212 -28— the 28 are this tree's pre-existing failures, unchanged.