fix: cursor shape (DECSCUSR), Ctrl+V forwarding, and mouse scroll in alt screen#13
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements dynamic cursor styles and blinking support by fetching state from the WASM backend. It also adds SGR mouse scroll tracking for the alternate screen and updates the input handler to forward Ctrl+V/Cmd+V signals to the PTY while maintaining native paste functionality. Feedback includes recommendations to clamp mouse coordinates and verify SGR mode support in the terminal's scroll handler to prevent out-of-bounds values, as well as a suggestion to reuse TextDecoder instances in the input handler for improved performance.
| if (this.wasmTerm?.hasMouseTracking()) { | ||
| // App negotiated mouse tracking (e.g. vim `set mouse=a`): send SGR | ||
| // scroll sequence so the app scrolls its buffer, not the cursor. | ||
| const metrics = this.renderer?.getMetrics(); | ||
| const canvas = this.canvas; | ||
| if (metrics && canvas) { | ||
| const rect = canvas.getBoundingClientRect(); | ||
| const col = Math.max(1, Math.floor((e.clientX - rect.left) / metrics.width) + 1); | ||
| const row = Math.max(1, Math.floor((e.clientY - rect.top) / metrics.height) + 1); | ||
| const btn = e.deltaY < 0 ? 64 : 65; | ||
| this.dataEmitter.fire(`\x1b[<${btn};${col};${row}M`); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
The mouse coordinates (col, row) should be clamped to the terminal dimensions (this.cols, this.rows) to ensure that the generated escape sequence doesn't contain out-of-bounds values, which could confuse some TUI applications. Additionally, since the SGR sequence (\x1b[<...) is hardcoded, it is recommended to verify that SGR mouse mode (1006) is actually enabled before sending it. If SGR is not enabled, falling back to the arrow-key behavior is more robust for applications that don't support extended mouse tracking.
| if (this.wasmTerm?.hasMouseTracking()) { | |
| // App negotiated mouse tracking (e.g. vim `set mouse=a`): send SGR | |
| // scroll sequence so the app scrolls its buffer, not the cursor. | |
| const metrics = this.renderer?.getMetrics(); | |
| const canvas = this.canvas; | |
| if (metrics && canvas) { | |
| const rect = canvas.getBoundingClientRect(); | |
| const col = Math.max(1, Math.floor((e.clientX - rect.left) / metrics.width) + 1); | |
| const row = Math.max(1, Math.floor((e.clientY - rect.top) / metrics.height) + 1); | |
| const btn = e.deltaY < 0 ? 64 : 65; | |
| this.dataEmitter.fire(`\x1b[<${btn};${col};${row}M`); | |
| } | |
| return; | |
| } | |
| if (this.wasmTerm?.hasMouseTracking() && this.wasmTerm?.getMode(1006, false)) { | |
| // App negotiated mouse tracking (e.g. vim `set mouse=a`): send SGR | |
| // scroll sequence so the app scrolls its buffer, not the cursor. | |
| const metrics = this.renderer?.getMetrics(); | |
| const canvas = this.canvas; | |
| if (metrics && canvas) { | |
| const rect = canvas.getBoundingClientRect(); | |
| const col = Math.max(1, Math.min(this.cols, Math.floor((e.clientX - rect.left) / metrics.width) + 1)); | |
| const row = Math.max(1, Math.min(this.rows, Math.floor((e.clientY - rect.top) / metrics.height) + 1)); | |
| const btn = e.deltaY < 0 ? 64 : 65; | |
| this.dataEmitter.fire(`\x1b[<${btn};${col};${row}M`); | |
| } | |
| return; | |
| } |
| action: KeyAction.PRESS, | ||
| }); | ||
| if (encoded.length > 0) { | ||
| this.onDataCallback(new TextDecoder().decode(encoded)); |
There was a problem hiding this comment.
Instantiating a new TextDecoder() inside the handleKeyDown event handler is inefficient as it creates a new object on every Ctrl+V/Cmd+V press. Consider reusing a single TextDecoder instance (e.g., as a private member of the class) to improve performance and reduce garbage collection pressure, especially since this pattern is repeated elsewhere in the file.
…alt screen Three independent PTY-input gaps wrapped in a single port from upstream coder#147 because they share the same WASM-side patch surface. 1. **DECSCUSR (cursor shape)** — apps that send the `CSI Ps SP q` DECSCUSR sequence to change cursor shape (block/bar/underline) and blink state used to be silently ignored on the JS side. WASM now exports `render_state_get_cursor_style` and `render_state_get_cursor_blinking`; the renderer queries them each frame so vim/tmux insert-mode cursor styles take effect. 2. **Ctrl+V forwarding** — Ctrl+V used to be intercepted and dropped so the browser paste event could handle it. That broke apps that read raw \\x16 from the PTY (e.g. opencode triggering osascript image paste). Ctrl+V now emits \\x16 via the Ghostty key encoder AND still lets the paste event fire for text content. Cmd+V on macOS behaves as before (no byte emitted, paste event handles it). 3. **Mouse scroll in alt screen** — wheel events while in the alt screen buffer (vim, less, htop) used to bypass mouse-tracking. Now they go through the same mouse-tracking path as the main screen, so apps that subscribe to wheel events receive them in alt screen too. WASM-API patch updates: - New exports for cursor_style / cursor_blinking - Hunk headers in patches/ghostty-wasm-api.patch recounted to reflect the added lines (the original patch upstream had stale @@ headers that prevented `git apply` from succeeding) The two pre-existing "Ctrl+V/Cmd+V should not emit onData" tests were documenting the old (now-incorrect) behaviour and have been rewritten to assert the new contract: Ctrl+V → \\x16, Cmd+V → empty (encoder returns no bytes for Super modifier). Co-authored-by: Jesse Peng <jesse23@gmail.com> Inspired-by: coder#147
diegosouzapw
force-pushed
the
fix/port-pr-147-decscusr-ctrlv-altscreen
branch
from
3133b9c to
b688df8
Compare
|
1 participant
Summary
Ports upstream PR #147 — three PTY-input gaps that share the same WASM-side patch surface.
1. DECSCUSR (cursor shape) → renderer respects it
Apps that send
CSI Ps SP qto change cursor shape (block/bar/underline) and blink state were silently ignored. WASM now exportsrender_state_get_cursor_styleandrender_state_get_cursor_blinking; the renderer queries them per frame so vim/tmux insert-mode cursors render correctly.2. Ctrl+V → emits
\x16then lets paste event firePreviously Ctrl+V was swallowed entirely (only the browser paste event ran). That broke apps that read raw
\x16natively from the PTY (e.g. opencode image-paste via osascript). Ctrl+V now emits\x16through the Ghostty encoder AND still allows the paste event for text content.Cmd+V on macOS still emits no byte (no terminal sequence is associated with the Super modifier) and still allows the paste event.
3. Wheel events in alt screen → mouse tracking sees them
Wheel events while in the alt screen buffer (vim, less, htop) used to bypass mouse-tracking. Now they go through the same path as the main screen.
Adaptation details
Two adjustments vs upstream's raw diff:
patches/ghostty-wasm-api.patchhad stale@@counts (the added lines weren't reflected in+X,Yline counts) and failedgit apply. Headers are corrected in this commit sobun run build:wasmsucceeds.allows Ctrl+V to trigger paste/allows Cmd+V to trigger pastetests asserted the OLD behaviour (dataReceived.length === 0). They now assert the new contract — Ctrl+V emits\x16, Cmd+V emits nothing — and explain why.Attribution
Thanks to @jesse23 for the original implementation.
Test plan
bun run fmt && bun run lint && bun run typecheckbun run build:wasm— Ghostty submodule + updated patch + Zig 0.15.2 → 416 KBbun test— 331 tests pass (2 updated, 0 new), 0 failbun run build:libbun run dev:\x16Risk
Medium. Three independent fixes touch WASM-API and renderer paths. Each is small in isolation; together they meaningfully improve real-world TUI compatibility.