-
Notifications
You must be signed in to change notification settings - Fork 53
feat: enable OSC 8 hyperlink clicking with Cmd/Ctrl modifier #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@codex review |
|
@slang25 seems the updates to the patch do not cleanly apply would you mind fixing that up? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05bc928b79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lib/ghostty.ts
Outdated
| const bufSize = 2048; // URLs can be long | ||
| const bufPtr = this.exports.ghostty_wasm_alloc_u8_array(bufSize); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retry OSC8 URI fetch when buffer is too small
The URI lookup uses a fixed 2048‑byte buffer and returns null on any non‑positive result. The new WASM API returns -1 when the buffer is too small, so OSC 8 links with long URIs (e.g., signed URLs or large query strings) will never be detected/clickable because the call always fails. Consider a retry path with a larger buffer (or a length query) so long hyperlinks still resolve.
Useful? React with 👍 / 👎.
| const scrollbackLength = this.terminal.wasmTerm.getScrollbackLength(); | ||
| const viewportRow = y - scrollbackLength; | ||
|
|
||
| // Find the full extent of this link (may span multiple lines) | ||
| const range = this.findLinkRange(hyperlinkId, y, x); | ||
| // Skip if this row is in scrollback (not in active viewport) | ||
| if (viewportRow < 0) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t skip OSC8 links that are in scrollback
When the terminal is scrolled up, rows mapped into the scrollback region are explicitly skipped, so OSC 8 links in visible scrollback lines will never be returned to the LinkDetector and Cmd/Ctrl‑click won’t open them. This is user‑visible when you scroll back to click a link in output history. If scrollback should be interactive like regex URLs, the provider needs a scrollback‑capable URI lookup (or a fallback) instead of early‑returning.
Useful? React with 👍 / 👎.
30ecf0f to
9c94f68
Compare
Add support for clicking OSC 8 hyperlinks in the terminal. This involves: 1. Add ghostty_terminal_get_hyperlink_uri() to the WASM API to retrieve the actual URI for cells marked with hyperlinks. The hyperlink_id field is just a boolean indicator; the real URI is stored in Ghostty's internal hyperlink set and must be looked up via this new function. 2. Update OSC8LinkProvider to use the new WASM API, with proper coordinate conversion from buffer rows to viewport rows (accounting for scrollback). 3. Fix LinkDetector to cache links by position range rather than hyperlink_id, since all hyperlinks incorrectly shared the same ID value (1), causing multiple links on one line to all open the same URL. Now Cmd+clicking (Mac) or Ctrl+clicking (Windows/Linux) an OSC 8 hyperlink correctly opens that specific link's URI. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
9c94f68 to
e593548
Compare
Summary
getHyperlinkUriAPI to WASM to retrieve the actual URI for hyperlinked cellsProblem
OSC 8 hyperlinks (created with escape sequences like
\e]8;;http://example.com\e\\Link text\e]8;;\e\\) were visually detected and underlined on hover, but Cmd+clicking them did nothing. Additionally, when multiple hyperlinks existed on the same line, all links would incorrectly open the same URL.Root Causes
Missing WASM API: The WASM didn't expose a function to retrieve the hyperlink URI. The
hyperlink_idfield in cells was just a boolean (0 or 1) indicating presence of a hyperlink, not a unique identifier.Incorrect link caching: The
LinkDetectorcached links byh${hyperlinkId}, but since all hyperlinks havehyperlink_id=1, all links on a line would share the same cache key, causing only the last discovered link to be returned for any hyperlink cell.Solution
Added
ghostty_terminal_get_hyperlink_uri(row, col)to the WASM API that looks up the actual URI from Ghostty's internal hyperlink storage.Updated
OSC8LinkProviderto use the new WASM API to fetch URIs, with proper coordinate conversion from buffer rows to viewport rows.Fixed
LinkDetectorto cache links by position range (r${row}:${startX}-${endX}) instead of hyperlink_id, ensuring each link is cached uniquely.Test Plan
printf '\e]8;;http://example.com\e\\Link text\e]8;;\e\\\n'printf '\e]8;;http://google.com\e\\Google\e]8;;\e\\ and \e]8;;http://github.com\e\\GitHub\e]8;;\e\\\n'🤖 Generated with Claude Code