fix: bunch of bugs related to text highlighting #59
Conversation
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
Contributor
sreya
commented
Nov 25, 2025
sreya
commented
- Fixes highlighting not clearing when you click somewhere else
- Fixes autoscroll not working when you highlight and drag up or down
- Fixes copy/paste related to highlighting
…election
Bug fixes:
1. Fixed selection highlight not clearing when clicking elsewhere
- Root cause: mousemove handler was overwriting previousSelection before
the renderer could read it, causing the old selection rows to never
get redrawn
- Solution: Changed from single previousSelection object to a Set of
dirty rows (dirtySelectionRows) that accumulates rows needing redraw
2. Added auto-scroll during drag selection
- When dragging selection above or below the terminal viewport, the
terminal now automatically scrolls to allow selecting text beyond
the visible area
- Implemented edge detection (30px from edges triggers scroll)
- Added document-level mousemove handler to track mouse position
even when outside the canvas during drag
- Selection extends to viewport edges during auto-scroll
Implementation details:
- SelectionManager now uses dirtySelectionRows Set instead of previousSelection
- Added markCurrentSelectionDirty() helper method
- Added auto-scroll state management (interval, direction, constants)
- Added mouseenter/mouseleave handlers for scroll control
- Updated renderer to use getDirtySelectionRows()/clearDirtySelectionRows()
- Properly cleans up interval and event listeners on dispose()
Fixes:
1. Selection no longer disappears when terminal receives new data
- Removed the clearSelection() call in writeInternal()
- Modern terminals (iTerm2, xterm.js, etc.) preserve selection when
new data arrives - it should only clear on user interaction
2. Clarified auto-scroll direction comments
- drag up → scroll up into history (scrollLines negative)
- drag down → scroll down to newer content (scrollLines positive)
When completing a drag selection (especially outside the window), a click event can fire immediately after mouseup. Added a brief 100ms flag (justFinishedSelecting) to prevent the click handler from clearing the selection in this case.
Instead of a race-prone setTimeout, track where mousedown occurred. Only clear selection on click if mousedown was also outside the canvas. This properly handles drag selection that ends outside the window.
Selection now uses absolute buffer coordinates (viewportY + viewportRow)
instead of viewport-relative coordinates. This ensures:
1. Selection persists correctly when scrolling during drag
2. Text that scrolls off-screen remains highlighted
3. Auto-scroll properly extends selection into scrollback
Key changes:
- selectionStart/End now store { col, absoluteRow } instead of { col, row }
- Added helper methods: getViewportY(), viewportRowToAbsolute(), absoluteRowToViewport()
- normalizeSelection() converts back to viewport coords for rendering
- All selection setters now convert viewport rows to absolute rows
When selection extends beyond visible viewport (during scroll), clamp the rendered coordinates to the visible area. This prevents: - Negative row indices - Row indices beyond terminal height - Renderer trying to draw millions of rows Selection still tracks full absolute range, but rendering is clamped.
The bug: during auto-scroll, selectionEnd was being reset to the viewport edge on each tick. As the viewport moved, the same edge position meant a smaller selection range. The fix: only update selectionEnd if the new position actually extends the selection (lower absoluteRow when scrolling up, higher when scrolling down). This makes the selection grow as you scroll, not shrink.
The bug: document mousemove handler was constantly resetting selectionEnd to the clamped mouse position (viewport edge) during auto-scroll. This overwrote the extended selection from the scroll handler. The fix: skip selectionEnd updates in mousemove when auto-scroll is active. The scroll interval handler is responsible for extending the selection during auto-scroll.
The formula was wrong. Absolute row should be an index into the combined buffer (scrollback + screen). The correct conversions: - viewportRow -> absolute: scrollbackLength + viewportRow - viewportY - absolute -> viewportRow: absoluteRow - scrollbackLength + viewportY This ensures that when you scroll, the same absolute row maps to different viewport rows, keeping the selection tied to the actual content.
…ed content Previously getSelection() was using viewport-relative coordinates from normalizeSelection(), which meant it only copied the visible portion. Now it directly uses the absolute row coordinates from selectionStart/End, iterating through scrollback and screen buffer based on absolute position. This correctly copies all selected text even when the selection extends beyond the visible viewport.
Added 22 tests covering: - Basic API methods (hasSelection, clearSelection, getSelection, etc.) - Selection with absolute coordinates - Text extraction from screen buffer, scrollback, and spanning both - Selection persistence during scroll - Viewport coordinate conversion after scrolling - Dirty row tracking for efficient redraws - Backward selection (right-to-left, bottom-to-top) - selectAll, select(), and selectLines() APIs Also updated existing terminal.test.ts selection tests to use the new absolute coordinate format via helper function.
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.