Drop the pending-drag button field that is now always primary - #730
Conversation
beginPendingDrag returns before assigning pendingDrag for any non-primary press, so the recorded button was a constant 0: the guard on it was unreachable and the mouseup comparison was a comparison with 0.
Deploying mouseterm with
|
| Latest commit: |
f79ba15
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1445c55d.mouseterm.pages.dev |
| Branch Preview URL: | https://fix-drop-dead-pending-drag-b.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark ready when you want the full review.
The dead-state claim holds: pendingDrag has a single assignment site, downstream of beginPendingDrag's if (ev.button !== 0) return true;. What the change leaves behind is a duplicate — finishPendingOrActiveDrag now carries two identical ev.button !== 0 guards, the new one inside the if (pendingDrag) branch and the pre-existing one just past it gating the active-drag path. Neither branch does anything before its guard, so hoisting a single check above if (pendingDrag) { is exactly equivalent and lets the invariant comment sit once instead of over half the function. The second guard is outside the diff, so there's no inline suggestion for it; I can push the consolidation once this is out of draft.
… paths Dropping pendingDrag.button left finishPendingOrActiveDrag with two identical `ev.button !== 0` guards, each the first statement of its path, so hoisting one above `if (pendingDrag)` is equivalent for every (pendingDrag, ev.button) pair. The invariant comment now sits once and covers both paths, and no longer claims the event is a release — the window-mousemove backstop finalizes with a mousemove, which also reports button 0.
2 participants
pendingDrag.buttoninlib/src/lib/terminal-mouse-router.tsis dead state.beginPendingDragreturns before the assignment for any non-primary press —if (ev.button !== 0) return true;, added in #710 so a right-click leaves no pendingDrag behind — so the recorded button is always0. That makes the guard inupdatePendingOrActiveDrag(if (pendingDrag.button !== 0) return;) unreachable, and the mouseup check infinishPendingOrActiveDraga comparison against0written indirectly. This drops the field, and leavesfinishPendingOrActiveDragwith a singleev.button !== 0guard hoisted above both of its paths, commented with where the invariant comes from.Hoisting is what makes the second half of the change worth having: once the pendingDrag branch compares against a literal
0, it is identical to the guard that already sat just past it gating the active-drag path, and both are the first statement of their path. One guard aboveif (pendingDrag)is equivalent for every(pendingDrag, ev.button)pair, and the invariant comment sits once instead of over half the function.No behavior change, so there is no test that would have failed before it: the removed guard could not fire, and the remaining check keeps the same truth value for every
ev.button.lib/src/lib/terminal-mouse-router.test.tsandmouse-selection.test.tspass unchanged (53 tests), andtsc --noEmitoverlibis clean.Equivalence across the four call sites of
finishPendingOrActiveDragThe hoist is a source transform, so it is worth recording that each caller reaches the guard with the same
ev.buttonit reached the old pair of guards with:ev.buttononWindowMouseUp!== pendingDrag.button(always0); active path!== 0!== 0onWindowMouseMovebackstop0— amousemovereports no button transitiononWindowPointerUp, mouse0for the captured primary releaseonWindowPointerUp, touch/pen0for contact removalbeginDraghas exactly one non-test caller — insideif (pendingDrag)inupdatePendingOrActiveDrag— which is what makes "an active drag only ever grows out of a pendingDrag" hold, and with it the claim that the active-drag path never sees a non-primary release either.docs/specs/mouse-and-clipboard.md§2 ("Does not count: a non-primary click") and §3.1 are unaffected: neither states the field, and the non-primary mouseup still returns beforeclearTemporaryOverrideAfterMouseDispatch, which is what that rule pins.