Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where text selection highlighting was invisible in non-TUI terminal operations (PowerShell, bash, cmd, etc.), making it appear that text selection was not supported. The problem was introduced in a previous commit (c1a6e99) that attempted to fix line gaps by always painting cell backgrounds, which inadvertently covered selection highlights.
Changes:
- Restored early return logic in
paintCellBackground()for normal cells to prevent covering selection highlights - Removed incorrect comment that claimed painting normal cell backgrounds prevented line gaps
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -242,9 +242,7 @@ class TerminalPainter { | |||
| if (cellData.flags & CellFlags.inverse != 0) { | |||
| color = resolveForegroundColor(cellData.foreground); | |||
| } else if (colorType == CellColor.normal) { | |||
There was a problem hiding this comment.
Consider adding a comment to explain why normal cells return early. This would help future maintainers understand that the terminal background is already painted in render.dart (lines 462-465), and painting normal cell backgrounds here would cover selection highlights and other overlays. For example: "Skip painting background for normal cells - the terminal background is already painted in render.dart and painting here would cover selection highlights."
| } else if (colorType == CellColor.normal) { | |
| } else if (colorType == CellColor.normal) { | |
| // Skip painting background for normal cells - the terminal background is | |
| // already painted in render.dart and painting here would cover selection | |
| // highlights and other overlays. |
[PR] Fix Selection Highlight Visibility in Non-TUI Terminals (Fixes #6)
Summary
Fixed the issue where text selection highlight was invisible in regular shell operations, making users think text selection was not supported.
Problem
paintCellBackgroundlogic was modifiedRoot Cause
Rendering order problem in the paint pipeline:
render.dartpaints the full terminal background first (line 462-465)Before this fix:
After this fix:
Solution
Restored the early return logic for normal cells in
paintCellBackground. Sincerender.dartalready paints the complete terminal background before painting cells and highlights, individual cell background painting is redundant and causes the coverage issue.Changes Made
Modified File:
packages/xterm/lib/src/ui/painter.dartpaintCellBackground(): Restored early return forCellColor.normalcells with an updated comment explaining whyVerification
flutter analyzeon the xterm package - No errors foundKnown Limitations
Selection Behavior: Since
xtermrenders Terminal UI by simulating the terminal buffer in Flutter, text selection will select by "line" rather than by exact character boundaries. However, the text copy-paste functionality works correctly and will copy the actual selected text content.