feat: add mouse support#1
Conversation
WalkthroughAdds mouse-command support: new public mouse types and encoder, stdio parsing and tests for mouse JSON, main loop handling (including MouseClick as press+release with bounds warnings), Session.size() accessor, and expanded README mouse and WebSocket documentation (duplicated block). Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant STDIO_API
participant Parser
participant MainLoop
participant Session
participant Encoder
participant InputChan
Client->>STDIO_API: send `mouse` JSON
STDIO_API->>Parser: deserialize MouseArgs
Parser->>Parser: validate event/button, require row/col ≥ 1
alt click event
Parser-->>MainLoop: Command::MouseClick(MouseEvent)
else other event
Parser-->>MainLoop: Command::Mouse(MouseEvent)
end
MainLoop->>Session: size()
Session-->>MainLoop: (rows, cols)
MainLoop->>MainLoop: validate coordinates vs size
alt out of bounds
MainLoop->>MainLoop: log warning
end
alt Mouse
MainLoop->>Encoder: mouse_to_bytes(event)
Encoder-->>MainLoop: bytes (SGR)
MainLoop->>InputChan: send bytes
else MouseClick
MainLoop->>Encoder: mouse_to_bytes(Press)
Encoder-->>MainLoop: bytes_press
MainLoop->>InputChan: send bytes_press
MainLoop->>Encoder: mouse_to_bytes(Release)
Encoder-->>MainLoop: bytes_release
MainLoop->>InputChan: send bytes_release
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main.rs (1)
114-134: Consider extracting the coordinate validation logic.The MouseClick handler duplicates the same coordinate validation from the Mouse handler. Consider extracting this into a helper function to maintain DRY principles.
Example refactor:
fn validate_mouse_coordinates(mouse_event: &command::MouseEvent, session: &Session) { let (cols, rows) = session.size(); if mouse_event.row > rows || mouse_event.col > cols { eprintln!( "warning: mouse coordinates ({},{}) exceed terminal size ({}x{})", mouse_event.col, mouse_event.row, cols, rows ); } }Then use it in both handlers:
Some(Command::Mouse(mouse_event)) => { validate_mouse_coordinates(&mouse_event, &session); let data = command::mouse_to_bytes(&mouse_event); input_tx.send(data).await?; } Some(Command::MouseClick(mouse_event)) => { validate_mouse_coordinates(&mouse_event, &session); // Send press event ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)src/api/stdio.rs(4 hunks)src/command.rs(2 hunks)src/main.rs(1 hunks)src/session.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main.rs (3)
src/cli.rs (2)
cols(38-40)rows(42-44)src/session.rs (1)
size(79-81)src/command.rs (1)
mouse_to_bytes(66-99)
🔇 Additional comments (8)
src/session.rs (1)
79-81: LGTM!Clean accessor that exposes terminal dimensions for coordinate validation in mouse event handling.
src/main.rs (1)
101-112: Mouse event handling looks correct.The coordinate validation and encoding logic is appropriate. Note that out-of-bounds coordinates trigger a warning but are still forwarded to the application, which aligns with the documented behavior.
README.md (1)
208-250: Excellent documentation!The mouse command documentation is comprehensive and clear:
- JSON payload format with all fields explained
- Event types and their semantics
- Supported buttons including wheel events
- Important notes about 1-indexed coordinates and SGR protocol requirements
- Practical examples with modifiers
This will help users understand how to properly use the mouse command for integration testing.
src/api/stdio.rs (3)
27-39: Well-structured MouseArgs for deserialization.The struct properly captures all mouse event fields with appropriate defaults for optional modifiers using
#[serde(default)].
123-170: Robust mouse command parsing with proper validation.The implementation correctly:
- Distinguishes between click (convenience) and explicit event types
- Validates event type and button strings with clear error messages
- Enforces 1-indexed coordinates by rejecting zero values
- Constructs MouseEvent with all required fields and modifiers
554-688: Comprehensive test coverage for mouse parsing.The test suite thoroughly validates:
- All event types (click, press, release, drag)
- All button types (left, middle, right, wheel_up, wheel_down)
- Modifier combinations
- Error cases (invalid event, invalid button, zero coordinates, missing args)
This provides excellent confidence in the parsing logic.
src/command.rs (2)
34-64: Clean and well-structured mouse event types.The data structures properly model:
- Event lifecycle (Press, Release, Drag)
- Button types including wheel events
- Optional modifiers with sensible defaults
- All fields are public for easy construction and inspection
The
CloneandPartialEqderives enable flexible usage patterns.
66-99: Correct SGR extended mouse protocol encoding.The implementation properly encodes mouse events per the SGR protocol specification:
- Button codes: Left(0), Middle(1), Right(2), WheelUp(64), WheelDown(65) ✓
- Modifier bits: Shift(+4), Alt(+8), Control(+16) ✓
- Motion bit: Drag(+32) ✓
- Suffix: 'M' for press/drag, 'm' for release ✓
- Format:
ESC[<btn;col;row<suffix>✓
Useful for integration testing
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main.rs (1)
69-77: Consider validating lower bounds for mouse coordinates.The validation checks upper bounds but doesn't verify that coordinates are at least 1. Since SGR mouse protocol uses 1-based coordinates, coordinates less than 1 are invalid and could cause issues.
Apply this diff to add lower bound validation:
fn validate_mouse_coordinates(mouse_event: &command::MouseEvent, session: &Session) { let (cols, rows) = session.size(); - if mouse_event.row > rows || mouse_event.col > cols { + if mouse_event.row < 1 || mouse_event.row > rows || mouse_event.col < 1 || mouse_event.col > cols { eprintln!( "warning: mouse coordinates ({},{}) exceed terminal size ({}x{})", mouse_event.col, mouse_event.row, cols, rows ); } }README.md (2)
240-242: Clarify optional fields syntax in JSON examples.The example on lines 240-242 shows modifiers as boolean fields. Consider clarifying in the documentation whether omitted modifier fields default to
false, or if they must be explicitly specified. This is especially important for API usability.Current text (lines 234-235) states "default
false", which is good, but the example could be enhanced to show both cases (with explicit modifiers and with omitted modifiers).Consider adding a second example demonstrating omitted modifiers:
Example with modifiers: ```json { "type": "mouse", "event": "click", "button": "left", "row": 10, "col": 25, "shift": true, "control": true }
+Without modifiers (all default to
false):
+json +{ "type": "mouse", "event": "click", "button": "left", "row": 10, "col": 25 } +--- `244-248`: **Document when mouse events are actually sent.** Lines 244-248 correctly note that the application must enable mouse tracking for events to take effect. However, the statement could be clearer about what happens if mouse tracking is not enabled. Consider expanding this note to explicitly state: "If the application has not enabled mouse tracking, the mouse events will be sent but will have no visible effect on the application." This is a minor clarity improvement for users who may not be familiar with terminal mouse protocols. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between b9868f1eb615f81103e4578f1937e9613e0dec93 and 68b14dcdf3b26aa373e112955b51026d533fb635. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `README.md` (1 hunks) * `src/api/stdio.rs` (4 hunks) * `src/command.rs` (2 hunks) * `src/main.rs` (2 hunks) * `src/session.rs` (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (3)</summary> * src/session.rs * src/api/stdio.rs * src/command.rs </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (1)</summary> <details> <summary>src/main.rs (2)</summary><blockquote> <details> <summary>src/cli.rs (2)</summary> * `cols` (38-40) * `rows` (42-44) </details> <details> <summary>src/command.rs (1)</summary> * `mouse_to_bytes` (66-99) </details> </blockquote></details> </details> </details> <details> <summary>🔇 Additional comments (4)</summary><blockquote> <details> <summary>src/main.rs (2)</summary><blockquote> `111-115`: **LGTM!** The mouse event handling is straightforward and follows the same pattern as the existing `Command::Input` handling. --- `117-131`: **Consider the atomicity of press/release events.** If the press event is sent successfully but the release event fails (e.g., channel closes or receiver drops), the terminal could be left with a button in pressed state. While this may be acceptable for integration testing, consider whether transactional semantics or error recovery is needed. This is likely fine for the stated use case, but verify whether: 1. The input channel can close between the two sends 2. A stuck button state would cause issues for your integration tests 3. You need any cleanup/recovery mechanism if the release fails </blockquote></details> <details> <summary>README.md (2)</summary><blockquote> `208-251`: **No duplication found—original review comment's concern is unfounded.** The verification confirms there is only ONE mouse documentation section in README.md (line 208). The "SGR extended mouse protocol" phrase appears once, and there is only one "Event types:" block. The original review comment's claim of duplicate mouse documentation is incorrect and should be disregarded. > Likely an incorrect or invalid review comment. --- `208-251`: **All documented mouse features are fully implemented and functional.** Verification confirms: - ✓ All 4 event types supported: `press`, `release`, `drag`, and `click` (click is a command variant that sends press+release events) - ✓ All 5 buttons: `left`, `middle`, `right`, `wheel_up`, `wheel_down` - ✓ All 3 modifiers: `shift`, `alt`, `control` - ✓ 1-indexed coordinates accepted and passed directly to SGR protocol format - ✓ Out-of-bounds validation with warnings issued (src/main.rs:69-81) - ✓ SGR protocol encoding implemented (src/command.rs:66-98) No implementation gaps found. The code correctly implements the documented behavior, including click as a convenience shorthand that sends both press and release events. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
1 participant
Useful for integration testing
Summary by CodeRabbit
New Features
Documentation