Skip to content

fix(input): parse default mouse reports - #2312

Merged
ogulcancelik merged 3 commits into
masterfrom
akbash/2309-default-mouse-encoding
Aug 4, 2026
Merged

fix(input): parse default mouse reports#2312
ogulcancelik merged 3 commits into
masterfrom
akbash/2309-default-mouse-encoding

Conversation

@akbash-bot

Copy link
Copy Markdown
Collaborator

Summary

  • recognize fixed-width DEFAULT/X10 host mouse reports before generic CSI parsing
  • decode raw button and coordinate bytes into semantic mouse events
  • preserve pending Escape separation for DEFAULT reports

Checks

  • cargo test default_mouse -- --nocapture
  • cargo test raw_input::tests -- --nocapture
  • cargo fmt --check
  • cargo clippy --all-targets --locked -- -D warnings
  • just check reached unrelated integration failures because the Kennel-provided external CARGO_TARGET_DIR conflicts with the test harness path check in tests/support/mod.rs

refs #2309

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4c073aa0-d631-44c7-a91d-ad9af39aeec8

📥 Commits

Reviewing files that changed from the base of the PR and between 51f4933 and 2925b93.

📒 Files selected for processing (2)
  • src/client/input.rs
  • src/raw_input.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/raw_input.rs

📝 Walkthrough

Walkthrough

Default xterm mouse escape sequences are parsed into RawInputEvent::Mouse events. The parser decodes coordinates, buttons, and modifiers. Framing and timeout handling support complete, split, and doubled-escape reports.

Changes

Default xterm mouse input

Layer / File(s) Summary
Mouse report decoding
src/raw_input.rs
Complete six-byte default mouse reports are validated and decoded into mouse events with zero-based coordinates, buttons, and modifiers. Tests cover decoding and invalid-frame boundaries.
Escape-sequence framing
src/raw_input.rs
Framing recognizes complete and incomplete default mouse reports. Doubled-escape framing preserves the leading Escape.
Mouse-sequence timeout handling
src/raw_input.rs, src/client/input.rs
The generalized pending-mouse check covers SGR and default sequences. Unix idle flushing applies the extended timeout to both formats while plain Escape uses the normal timeout. Tests cover split reports and timeout selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant InputStream
  participant EscapeFramer
  participant MouseDecoder
  participant IdleFlusher
  participant RawInputEvent
  InputStream->>EscapeFramer: provide default xterm mouse bytes
  EscapeFramer->>MouseDecoder: pass complete six-byte report
  MouseDecoder->>RawInputEvent: create decoded mouse event
  EscapeFramer->>RawInputEvent: preserve preceding Escape when present
  IdleFlusher->>EscapeFramer: check incomplete mouse sequence
  EscapeFramer-->>IdleFlusher: report pending SGR or default sequence
Loading

Possibly related PRs

  • herdrdev/herdr#2311: This PR extends the X10 mouse-report parsing and timeout handling introduced there.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: parsing default mouse reports in input handling.
Description check ✅ Passed The description explains the default mouse parsing changes, tests, and validation results.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch akbash/2309-default-mouse-encoding

Comment @coderabbitai help to get the list of available commands.

@kangal-bot kangal-bot added the ai-review Trigger automated AI reviews for pull requests admitted by the PR gate label Aug 4, 2026
@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

The PR adds fixed-width DEFAULT/X10 mouse-report parsing and extends incomplete-report reassembly to this encoding.

  • Decodes DEFAULT button, coordinate, and modifier bytes into semantic mouse events.
  • Preserves separation between a pending Escape and a following DEFAULT report.
  • Applies the extended mouse timeout to partial DEFAULT reports in both input paths.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported partial DEFAULT-report timeout now receives the extended mouse reassembly window.

Important Files Changed

Filename Overview
src/raw_input.rs Adds DEFAULT mouse framing, parsing, doubled-Escape handling, timeout classification, and focused regression coverage; the previously reported partial-report timeout is fixed.
src/client/input.rs Generalizes the active mouse-capture timeout selection to include incomplete DEFAULT reports and tests the new classification.

Reviews (3): Last reviewed commit: "Merge branch 'master' into akbash/2309-d..." | Re-trigger Greptile

Comment thread src/raw_input.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/raw_input.rs (1)

1394-1404: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add coverage for button and modifier decoding.

This test covers only MouseEventKind::Moved with empty KeyModifiers. Add cases for button presses, release, wheel events, and the SHIFT, ALT, and CONTROL bits.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8239483f-0d4a-4540-9ddb-429f2e70ae98

📥 Commits

Reviewing files that changed from the base of the PR and between 1997b88 and 51f4933.

📒 Files selected for processing (1)
  • src/raw_input.rs

Comment thread src/raw_input.rs
@akbash-bot

Copy link
Copy Markdown
Collaborator Author

Regarding CodeRabbit review 4857764169: I’m not adding a button/modifier matrix. DEFAULT decoding delegates to the shared parse_mouse_cb, whose button and modifier semantics are already covered by the SGR tests; the new tests target only the distinct fixed-width framing and byte-boundary behavior.

@ogulcancelik
ogulcancelik merged commit ee8429f into master Aug 4, 2026
8 checks passed
abhijit-s pushed a commit to abhijit-s/herdr that referenced this pull request Aug 4, 2026
Integrates upstream range e7c9620..1f1e434 (16 commits) onto fork master.

Key conflict resolutions:
- src/ui/status.rs + config: adopt upstream herdrdev#2282 distinct status indicators
  (state_icon_symbol Dots/Symbols) wholesale. Marks stay static (no animated
  spinner or animation-tick reintroduced); fork's config-diagnostic-as-toast
  delta preserved (render_config_diagnostic dropped, unused).
- src/app/input: adopt upstream input fixes (herdrdev#2303 modifyOtherKeys releases,
  herdrdev#2259 shift-tab in automation, herdrdev#2312 default mouse reports, herdrdev#2291 pending url
  clicks) while preserving the deliberate herdrdev#1876 non-US shifted keybinding revert.
- src/config: keep fork StatusConfig (status_right/status_right_length/
  status_interval) additively alongside upstream herdrdev#2295 retired agent panel scope
  and StatusIndicatorStyle.
- justfile: preserve fork install-local recipe alongside upstream changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Trigger automated AI reviews for pull requests admitted by the PR gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants