fix: make drag coordinate order explicit - #269
Conversation
Signed-off-by: nightcityblade <nightcityblade@gmail.com>
vyncint
left a comment
There was a problem hiding this comment.
Reviewed and verified. The signature change does exactly what the issue asked.
What I checked beyond CI:
- The transposition is now a compile error rather than a silent wrong gesture: passing a
Screen::findresult straight in fails with "this method takes 5 arguments but 3 arguments were supplied". - The drag suite passes unchanged, including the per-cell motion test and the off-grid endpoint test, so the four arguments reach
check_mouse_in_gridin the same order the tuples did. - Reconstructing
from/toinside the body keeps the diff to the boundary, which is the right call.
Taking option 1 from the issue was the better of the two, and the cross-reference you added on Screen::find is the half I would have forgotten.
One more change, alongside the shared one below
skills/termlens/SKILL.md landed on main a few hours after you opened this, and it documents the old two-tuple signature in three places. It is the file coding agents copy from, so it has to move with the API:
- line 99, in the coordinate-order rule:
`drag(button, (col, row), (col, row))`becomes`drag(button, from_col, from_row, to_col, to_row)`, and the sentence after it that says "the tuple types match and the axes do not" now needs to say that the four arguments make the transposition unwritable. - line 369, in the API cheat sheet:
`drag(MouseButton::Left, (c, r), (c, r))`. - line 414, in the pitfalls table: the row whose left column is
`t.drag(b, s.find("x").unwrap(), …)`.
.github/scripts/check-skill-snippets.sh compiles the skill's ```rust blocks, but these three are prose and tables, so CI cannot catch them. Sorry for the collision — it is entirely an accident of merge order, not something you could have seen when you opened this.
One thing to fix, and it is the same in all five of your PRs
The CHANGELOG entry landed inside the published ## [0.9.0] - 2026-09-05 section rather than under ## [Unreleased]. Easy to miss: [Unreleased] was empty when you branched, so the nearest heading with content under it belongs to 0.9.0.
It matters for two reasons. 0.9.0 is already on crates.io and its GitHub Release notes were generated from that section, so the entry now describes a release that does not contain the change. And docs/RELEASING.md cuts the next release by moving [Unreleased] down into a new version heading — an entry parked in [0.9.0] would never appear in any release notes at all.
Move it up under ## [Unreleased], adding the ### Added / ### Changed / ### Fixed sub-heading there if it does not exist yet. main has moved on by a commit since you branched, so a rebase is wanted anyway:
git fetch origin && git rebase origin/mainThanks for these — five well-scoped fixes with tests and docs, and the PR bodies say what you decided and why. That is exactly the shape this repository asks for.
What & why
Change
Terminal::dragto accept four explicitly ordered column/row arguments, matching the other mouse APIs. This prevents the row-first tuple returned byScreen::findfrom being passed directly and silently transposed. The relevant docs and drag tests now use the unambiguous spelling.Closes #257.
Checklist
cargo fmt --allandcargo clippy --workspace --all-targets --all-featuresare cleangit commit -s) — see CONTRIBUTING.md §5CHANGELOG.mdupdated under[Unreleased](user-facing changes only)cargo insta review, not blind-accepted (N/A: no snapshots changed)