fix(selection): copy the cells that were highlighted - #269
Open
GlassOnTin wants to merge 1 commit into
Open
Conversation
getSelectedText decided each row's column bounds with minOf/maxOf on the two anchor columns, while SelectionRange.contains — the predicate that draws the highlight — is anchor-aware: the first row runs from the START anchor's column, the last row up to the END anchor's column. Those agree only when the drag happens to go down and to the right. Drag down and LEFT and they diverge. Selecting from row 0 col 8 to row 1 col 2 highlights "IJ" then "abc", and copied "CDEFGHIJ" then "abcdefghi" — six characters per row the user never selected, silently, with the highlight still showing the smaller region. Uses the existing getStartPosition/getEndPosition helpers, which already encode the anchor-aware rule the highlight uses, so the two now derive the same bounds from one place. The new test asserts the clipboard against the highlight rather than against a hardcoded string: it reads the selected cells straight off SelectionRange.contains and requires getSelectedText to return exactly that. It fails on the old code with the mismatch above, and a second test pins the down-and-right case that already worked. 43 selection tests pass; termlib and feature:terminal suites green. Found while investigating a separate copy report (Haven #581), which this does NOT explain — that one is smart-copy's panel-border stripping. This bug is unreported.
GlassOnTin
added a commit
to GlassHaven/Haven
that referenced
this pull request
Aug 23, 2026
Copy returns what was highlighted: a selection dragged down and to the left used to copy more than the highlight showed. Also open upstream as connectbot/termlib#269. An RDP desktop shorter than 640 pixels is no longer rounded up, so 800x600 and 640x480 can be set. Refs #572 — that floor is not the cause of the SPICE pointer problem reported there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
SelectionRange.containsandSelectionManager.getSelectedTextdisagree about which cells a multi-row selection covers. As a result, the clipboard can receive text the user never highlighted.containsis the predicate that draws the highlight. It is anchor-aware. The first row runs from the start anchor's column, and the last row up to the end anchor's column:getSelectedTextinstead takesminOf/maxOfof the two columns:These methods give the same answer only when the drag goes down and to the right.
Reproducing it
Drag down and to the left. Selecting from row 0 column 8 to row 1 column 2 over
highlights
IJthenabc, and copies:This includes six characters per row that were never selected. The highlight still shows the smaller region, so nothing on screen suggests the clipboard holds anything else.
An upward drag is unaffected. In that case,
minOf/maxOfpicks the same columns the anchors would.The change
getSelectedTextnow derives its column bounds from the existinggetStartPositionandgetEndPositionhelpers. These helpers already encode the anchor-aware rule the highlight uses. Both sides now read the bounds from one place rather than through two implementations that can drift.Single-row selections are unchanged. Those helpers return
minOf/maxOffor that case, which is what the old code did.Tests
Two tests, both in
SelectionManagerTest.The first asserts the clipboard against the highlight rather than against a hard-coded string. It reads the selected cells straight off
SelectionRange.containsand requiresgetSelectedTextto return exactly those. This keeps checking the property that matters if either side changes later.The second pins the down-and-right drag that already behaved correctly.
Checked against the unmodified source on this branch's base: the down-and-left test fails with
expected:<IJ\nabc> but was:<CDEFGHIJ\nabcdefghi>, the down-and-right test passes, and the rest of the suite is unaffected. With the change applied, 36/36 pass.This was found while investigating an unrelated copy report downstream in Haven, which turned out to have a different cause.