Repository navigation
fix: open latched link on release with press-time chord + defer-clear latch (#71 follow-up) - #78
Conversation
…latch Two review follow-ups (codex) unifying the link-click decision so the open path and the report-suppression path agree: - P2 (swallowed click): when the ctrl/super modifier was released before the left button, the report was still suppressed (latched), but processLinks re-derived linkAtPos from the now-empty live mods, so the link no longer matched and the click was swallowed. linkAtPos now uses the latched chord while link_click_active is set, and the release path attempts processLinks whenever the click is latched (not only while over_link is still set), so the latched click opens its link instead of being dropped. - P3 (stale latch): the unconditional reset was placed after the report block, but the successful-link and prompt-click branches return early and never reached it. Clear the latch with a function-level defer so it is reset on every left-release return path. The press is the only setter. Part of manaflow-ai/cmux#5128. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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)
📝 WalkthroughWalkthroughThis PR modifies ChangesLink activation latch and dispatch
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Greptile SummaryThis PR fixes two follow-up issues from the link-under-mouse-reporting work: a swallowed click when the ctrl/super modifier is released before the mouse button, and a stale latch when the early-return paths in
Confidence Score: 4/5Safe to merge; changes are tightly scoped to the link-click latch lifecycle with no impact on unrelated mouse paths. Both fixes are logically sound and the defer-based clear correctly covers all return paths. The one subtle edge case worth noting — pressing ctrl+left when not over any link still causes the click to be swallowed (report suppressed, no link opened), but this was pre-existing behaviour unchanged by the PR. The expanded processLinks guard and linkAtPos mod synthesis introduce no new code paths that could produce incorrect results. No files require special attention; the single changed file src/Surface.zig has clear, well-commented diffs. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant mouseButtonCallback
participant processLinks
participant linkAtPos
Note over User,linkAtPos: Happy path: modifier held through release
User->>mouseButtonCallback: left press (ctrl/super held)
mouseButtonCallback->>mouseButtonCallback: "link_click_active = true"
User->>mouseButtonCallback: left release (ctrl/super still held)
mouseButtonCallback->>mouseButtonCallback: defer registered (clears latch on return)
mouseButtonCallback->>processLinks: over_link OR link_click_active → call
processLinks->>linkAtPos: linkAtPos(pos)
linkAtPos->>linkAtPos: "link_click_active=true → use ctrlOrSuper mods"
linkAtPos-->>processLinks: Link found
processLinks-->>mouseButtonCallback: "processed=true → return true"
mouseButtonCallback->>mouseButtonCallback: "defer fires: link_click_active = false"
Note over User,linkAtPos: Edge case fix: modifier released before button
User->>mouseButtonCallback: left press (ctrl/super held)
mouseButtonCallback->>mouseButtonCallback: "link_click_active = true"
User->>mouseButtonCallback: modifier released (over_link cleared)
User->>mouseButtonCallback: "left release (modifier gone, over_link=false)"
mouseButtonCallback->>mouseButtonCallback: defer registered
mouseButtonCallback->>processLinks: "link_click_active=true → still enters block"
processLinks->>linkAtPos: linkAtPos(pos)
linkAtPos->>linkAtPos: "link_click_active=true → synthesize ctrlOrSuper mods"
linkAtPos-->>processLinks: Link found (mods not from live state)
processLinks-->>mouseButtonCallback: "processed=true → return true"
mouseButtonCallback->>mouseButtonCallback: "defer fires: link_click_active = false"
Reviews (1): Last reviewed commit: "fix: open latched link on release with p..." | Re-trigger Greptile |
Two review follow-ups (codex) on the link-under-mouse-reporting work (#71/#74/#75/#76/#77, manaflow-ai/cmux#5128), unifying the link-click decision so the open path and the report-suppression path use the same press-time chord.
Swallowed click (P2). When the ctrl/super modifier was released before the left button, the report was still suppressed (latched at press), but
processLinksre-derivedlinkAtPosfrom the now-empty live mods — so the link no longer matched and the click was swallowed (link never opened, nothing reached the program).linkAtPosnow uses the latched chord whilelink_click_activeis set, and the release path attemptsprocessLinkswhenever the click is latched (not only whileover_linkis still set). The latched click opens its link instead of being dropped.Stale latch (P3). The unconditional reset was placed after the report block, but the successful-link (
if (processed) return true) and prompt-click branches return early and never reached it. The latch is now cleared with a function-leveldefer, so it resets on every left-release return path. The press remains the only setter.Net: a single press-time decision drives press/drag/release suppression and link opening, consistently — closing the modifier-timing edge cases.
zig fmt/ast-checkclean; behavior is integration-level, verified by cmux dogfood.AI disclosure (per AI_POLICY.md): developed with Claude Code (Claude Opus 4.8); reviewed by a maintainer.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes swallowed link clicks when ctrl/super is released before the left mouse button. We latch the press-time chord for the full click and always clear it on release, so latched clicks open reliably. Addresses
manaflow-ai/cmux#5128.link_click_activeto resolvelinkAtPos, and attempt open on release even ifover_linkchanged.Written for commit 9f014e9. Summary will update on new commits.
Summary by CodeRabbit