feat(terminal): record OSC 8 hyperlinks on the cells they cover - #181
Draft
Ayman Bagabas (aymanbagabas) wants to merge 1 commit into
Draft
feat(terminal): record OSC 8 hyperlinks on the cells they cover#181Ayman Bagabas (aymanbagabas) wants to merge 1 commit into
Ayman Bagabas (aymanbagabas) wants to merge 1 commit into
Conversation
Nothing read a cell's hyperlink before this: `EmuCell` had no field for one,
so `OSC 8` was parsed by every backend and then dropped at the seam. A test
could see the text of a link but never where it pointed.
Add `Hyperlink { id, uri }` to the cell vocabulary, behind an `Arc` because a
link covers a run of cells rather than one, and storing it inline would put a
copy of the URI in every cell of the run. Surface it as `link` and `link_id`
on the cell model, and as a `link` key in snapshots. The key records the URI
rather than a boolean, because a snapshot that only said "this is a link"
would pass when the link started pointing somewhere else.
Only `id` and the URI survive parsing. OSC 8 allows arbitrary `key=value`
params before the URI, but alacritty, rio, ghostty and xterm.js all keep `id`
and discard the rest before a cell can be read back, so there is nothing left
to report.
alacritty and rio each invent an id for a link that arrived without one,
appending `_alacritty` or `_rio` to a process-wide counter. That is a
rendering aid rather than something the child sent, it varies with the order
links were parsed in, and no other backend produces it, so it is dropped and
such a link reports no id. ghostty reports no id either: its FFI exposes
`hyperlink_uri` and has no accessor for the parameter, which is declared as a
conformance divergence.
Two things fixed in the xterm.js shim, both found by the new tests:
`getCell` reuses a single `CellData` and overwrites `extended` only for a
cell that has extended attributes, so an unlinked cell read after a linked
one still carried the previous cell's `urlId`. Reading it unguarded made a
link run to the end of the row no matter where it was closed.
A cell that is both underlined and inside a link reported its underline as
dashed whatever SGR asked for, because `ExtendedAttrs.underlineStyle` returns
5 whenever a `urlId` is set. That is how xterm.js draws links, not something
the child sent, and the other three backends report what SGR said.
`Profile::hyperlinks`, default on, turns the whole thing off so a session
behaves like a terminal without OSC 8 support, which is what a program's
fallback text renders on. No backend offers a native switch for this, so each
is gated where the cell is read.
Signed-off-by: Ayman Bagabas <ayman.bagabas@gmail.com>
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
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.
Adds OSC 8 hyperlinks to the cell vocabulary, so a test can assert where a link points rather than only what it says.
Why
Nothing read a cell's hyperlink.
EmuCellhad no field for one, so every backend parsedOSC 8and then dropped it at the seam. A CLI that printsView logsas a link could be asserted on for its text and never for its URL, which is the part that breaks.The model
Behind an
Arcbecause a link covers a run of cells rather than one. Inline, a 40-character URL would be copied into every cell of the run and would more than doubleEmuCell; alacritty and rio already hand out reference-counted links, so this is also what they cost to clone.Surfaced as
linkandlink_idon the cell model, and as alinkkey in snapshots. The snapshot key records the URI rather than a boolean, on the same reasoning as the existingunderlinekey recording a style rather than a boolean: a snapshot that only said "this is a link" would pass when the link silently started pointing somewhere else.Only
idand the URIOSC 8 allows arbitrary
key=valueparams before the URI. All four backends keepidand discard the rest before a cell can be read back:Cell::hyperlink()id,uriExtras::hyperlinkid,uriGridRef::hyperlink_uri()oscLinkService.getLinkData(urlId)id,uriSo there is nothing left to report, and reporting the full param list would mean parsing OSC 8 a second time ourselves in parallel with the emulator.
Synthesized ids are dropped. alacritty and rio each invent an id for a link that arrived without one, appending
_alacrittyor_rioto a process-wide counter. That is a rendering aid rather than something the child sent, it varies with the order links were parsed in, and no other backend produces it, so letting it through would put a backend's private counter into snapshots. A link with noid=reportsNone.ghostty reports no id at all: its FFI exposes
ghostty_grid_ref_hyperlink_uriand has no accessor for the parameter, so it never crosses the boundary. Declared as ahyperlink_has_no_iddivergence rather than worked around.Two xterm.js bugs, both found by the new tests
A link ran to the end of the row.
getCellreuses a singleCellDatainstance and overwritesextendedonly for a cell that actually has extended attributes, so an unlinked cell read straight after a linked one still carried the previous cell'surlId. Now gated onhasExtendedAttrs().A link forced a dashed underline.
ExtendedAttrs.underlineStylereturns5whenever aurlIdis set:That is how xterm.js chooses to draw links, not something the child sent, and the other three backends report what SGR said. Only reachable for a cell that is both underlined and linked, since a link alone never sets the underline flag. The raw bits still hold the real style, so those are read instead.
Both were latent before this PR: xterm.js has been reporting dashed underlines on underlined links since the backend landed, with no test that could see it.
ghostty's viewport path
ghostty's render iterator exposes a cell's style and graphemes but not its hyperlink, so the URI has to come from a grid reference, which costs an FFI call per cell.
Row::has_hyperlinknarrows that to the rows that have one; it is documented as allowing false positives but not false negatives, so a row it skips genuinely has no link. A session with no links pays one flag check per row.Configuration
Profile::hyperlinks, default on, plumbed throughtui-test.toml, JS (hyperlinks) and Python (hyperlinks). Off makes a session behave like a terminal without OSC 8 support, which is what a program's fallback text renders on. No backend offers a native switch, so each is gated where the cell is read.Tests
Five conformance cases, so all four backends are held to the same contract:
conformance_hyperlinks_cover_the_cells_they_open_overconformance_hyperlink_ids_are_only_what_the_child_sentconformance_adjacent_hyperlinks_stay_separateconformance_a_hyperlink_does_not_change_the_underlineconformance_hyperlinks_can_be_turned_offcargo test --workspace --features tui-test-rs/ghostty,tui-test-rs/rio,tui-test-rs/xtermjsgives 435 passed, 0 failed.Independent of #180; both touch
Profileand the bindings, so they will need a trivial merge if both land.