Repository navigation
feat: support pixel-precision mouse reporting (CSI ?1016h) - #1077
Merged
Merged
Conversation
Adds a new MouseFlags value, MousePixelEvents, that acts as a modifier on top of MouseButtonEvents/MouseDragEvents/MouseMotionEvents. When set, EnableMouse emits PmMouseSgrPixel instead of PmMouseSgr and the input parser skips its cell-size clamp on mouse coordinates so pixel positions pass through to EventMouse.Position() unchanged. Default behavior is unchanged. Includes tests pinning both the existing clip behavior and the new pixel-mode passthrough including the toggle-off transition.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds pixel-coordinate mouse reporting support by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Term as Terminal Screen
participant Parser as Input Parser
participant TTY as Terminal / TTY
App->>Term: EnableMouse(with/without MousePixelEvents)
Term->>Term: Determine pixel requested (MousePixelEvents)
Term->>Parser: SetPixelMouse(pixel)
Term->>Term: Enable vt.PmMouseSgrPixel or vt.PmMouseSgr
TTY->>Parser: SGR mouse event (CSI ... M / m) with params
Parser->>Parser: Compute x = params[1]-1, y = params[2]-1
alt pixelMouse = true
Parser->>Parser: Do not clamp coordinates (pixel units)
else
Parser->>Parser: Clamp coordinates to [0..cols-1]/[0..rows-1]
end
Parser->>App: Emit mouse event with Position()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@input.go`:
- Around line 125-136: SetPixelMouse sets ip.pixelMouse and forwards to
ip.nested when nested exists, but the lazy constructor in handleWinKey that
creates ip.nested copies evch, rows, cols, and advanced but forgets to copy
pixelMouse; update the nested-creation code in handleWinKey to initialize the
new nested parser's pixelMouse field from ip.pixelMouse (and ensure any mutex
locking around ip.pixelMouse is respected, e.g., read it under
ip.l.Lock()/Unlock()) so the lazily-created nested parser uses the same
pixelMouse setting as the parent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9af5f262-c4fc-499e-b814-c39014843f20
📒 Files selected for processing (5)
input.goinput_test.gomouse.goscreen.gotscreen.go
handleWinKey constructs a nested inputParser the first time it sees a doubly-encoded win32-input-mode mouse event (a workaround for Alacritty and WezTerm). It copied evch/rows/cols/advanced but not pixelMouse, so events routed through the nested parser would still be clipped to cell bounds even after EnableMouse(MousePixelEvents). Carry pixelMouse over on construction so the nested parser inherits the same setting.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@input.go`:
- Around line 128-136: SetPixelMouse currently returns early when ip.nested !=
nil which leaves the parent inputParser.pixelMouse unchanged; change it to
update the parent's pixelMouse under ip.l lock and then call
ip.nested.SetPixelMouse(on) (mirroring the SetSize pattern) so both parent
(field pixelMouse) and nested parser stay in sync; ensure locking around
ip.pixelMouse assignment and avoid race conditions when accessing ip.nested and
calling its SetPixelMouse.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
SetPixelMouse previously short-circuited to the nested parser when one existed, leaving the parent parser's pixelMouse field unchanged. Since normal SGR mouse events are processed by the parent (the nested parser is only used for the ASCII-byte workaround in handleWinKey), the parent needs the flag set too. Set the parent's pixelMouse under lock and then forward to the nested parser if present, so both stay consistent.
Owner
|
It really would be nice to have this in the vt layer as well. One problem is that applications will need to know how to map pixels to character cells. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1077 +/- ##
=======================================
Coverage 87.92% 87.92%
=======================================
Files 46 46
Lines 5449 5466 +17
=======================================
+ Hits 4791 4806 +15
+ Misses 502 501 -1
- Partials 156 159 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gdamore
approved these changes
Apr 29, 2026
ImGajeed76
added a commit
to ImGajeed76/glife
that referenced
this pull request
Apr 29, 2026
…nstall Renames the module path so users can run go install github.com/ImGajeed76/glife@latest. Updates README install section and replaces the old "Patched tcell" note (the fork is gone) with a pointer to gdamore/tcell#1077.
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.
Demo
This is from glife, a Conway's Game of Life renderer that uses braille (2x4 dots per cell) for its grid. With pixel mouse mode the cursor resolves to the individual dot under it instead of snapping to a whole cell. This PR is the upstream counterpart of a local patch that glife had been carrying to make this work.
Motivation
xterm's SGR-Pixel mouse mode (
CSI ?1016h,PmMouseSgrPixel) reports mouse coordinates in terminal pixels rather than character cells. It is the only way for an application to resolve where the cursor is within a cell, which is useful for things like braille-grid drawing tools, mouse-driven sub-cell selection, and pixel-accurate widgets in terminals that reportxpixel/ypixelviaTIOCGWINSZ.tcellalready knows the mode exists;PmMouseSgrPixelis defined invt/mode.go. ButScreenhas no way to ask for it, and the input parser unconditionally clamps mouse coordinates to the cell-based screen size inhandleMouse, which destroys pixel coordinates before the app ever sees them. Today, applications that want this have to either fork tcell or emitCSI ?1016hthemselves and live with the clip silently breaking drag events outside the visible grid.Supported by Kitty, WezTerm, iTerm2, Konsole, recent xterm, VS Code's integrated terminal, Alacritty, and Ghostty. tmux and older terminals fall back to cell coordinates, so the existing flags continue to work unchanged in those cases.
Change
Adds a new
MouseFlagsvalue,MousePixelEvents, that acts as a modifier on top of the existingMouseButtonEvents,MouseDragEvents, andMouseMotionEvents. When set:tScreen.enableMouseemitsPmMouseSgrPixelinstead ofPmMouseSgr.EventMouse.Position()unchanged.EnableMouseandDisableMousealso disablePmMouseSgrPixelon reset, mirroring the existing handling forPmMouseSgr.The flag is purely additive (value
8, no overlap with the existing flags) and does nothing on its own. At least one of the other mouse flags must still be set for any events to be delivered. Default behavior is unchanged.EventMouse.Position()documents that the unit is pixels when the screen was started withMousePixelEvents, otherwise cells (today's behavior).Usage
Tests
Two new tests in
input_test.go:TestInputMouseSgrClipsToScreenpins the existing clip behavior as a regression guard.TestInputMouseSgrPixelNoClipexercisesSetPixelMouse(true)with a 1006-format SGR report whose coordinates exceed the screen, and verifies they pass through unchanged. It also toggles the flag back off and confirms clipping resumes.go test ./...is green across all packages.Note on the VT emulator
The
vtterminal emulator used bymouse_test.go's integration tests currently has no branch forPmMouseSgrPixel; it always emits 1006-style cell coordinates, even when 1016 is enabled. Because of that, exercising the new flag end-to-end throughMockScreenwould require the emulator to track a synthetic cell-pixel size, which felt out of scope for this PR. The unit-level tests above cover both code paths in the input parser directly. Happy to extend the emulator in a follow-up if you would prefer richer integration coverage.Summary by CodeRabbit
New Features
Behavior
Documentation
Tests