Fix tool frame repainting and color retention
- commit
- c98b3d241f7f32f84d81aba79f45a7195bce5e44
- author
- patriceckhart · · 2026-07-15 16:37 UTC
- parent
- 8fb72f3
- signature
- unsigned
- refs
- tag: v0.2.87
4 files · +94 -27
M packages/tui/render.go +32 -7
| @@ -625,14 +625,39 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||
| 625 | 625 | } | |
| 626 | 626 | } | |
| 627 | 627 | ||
| 628 | + | // Rows that have scrolled above the viewport are immutable terminal | |
| 629 | + | // history. Replaying the full logical buffer to update one of them | |
| 630 | + | // duplicates that history, and erasing it first destroys native text | |
| 631 | + | // selections. Ignore inaccessible changes and patch only the first | |
| 632 | + | // changed row that is still addressable. The cached logical buffer is | |
| 633 | + | // updated below, so ignored rows remain historical snapshots while new | |
| 634 | + | // output naturally pushes them farther into scrollback. | |
| 635 | + | if firstChanged >= 0 && firstChanged < r.logViewportTop { | |
| 636 | + | firstChanged = -1 | |
| 637 | + | for idx := r.logViewportTop; idx < maxLines; idx++ { | |
| 638 | + | oldLine := "" | |
| 639 | + | if idx < len(r.logLines) { | |
| 640 | + | oldLine = r.logLines[idx] | |
| 641 | + | } | |
| 642 | + | newLine := "" | |
| 643 | + | if idx < len(lines) { | |
| 644 | + | newLine = lines[idx] | |
| 645 | + | } | |
| 646 | + | if oldLine != newLine { | |
| 647 | + | firstChanged = idx | |
| 648 | + | break | |
| 649 | + | } | |
| 650 | + | } | |
| 651 | + | // A newly appended blank row compares equal to the implicit empty | |
| 652 | + | // row past the old slice, but it still has to advance the terminal. | |
| 653 | + | if firstChanged == -1 && len(lines) > len(r.logLines) && len(r.logLines) >= r.logViewportTop { | |
| 654 | + | firstChanged = len(r.logLines) | |
| 655 | + | } | |
| 656 | + | } | |
| 657 | + | ||
| 628 | 658 | if firstChanged == -1 { | |
| 629 | - | // No content changes; the final cursor positioning below may still | |
| 630 | - | // move the hardware cursor if the editor cursor changed. | |
| 631 | - | } else if firstChanged < r.logViewportTop { | |
| 632 | - | // Changes above the visible viewport cannot be patched safely. | |
| 633 | - | // Repaint without erasing scrollback: the terminal may have a | |
| 634 | - | // native text selection anchored there while output streams. | |
| 635 | - | writeFull(true, false) | |
| 659 | + | // No addressable content changes; the final cursor positioning below | |
| 660 | + | // may still move the hardware cursor if the editor cursor changed. | |
| 636 | 661 | } else { | |
| 637 | 662 | prevViewportTop := r.logViewportTop | |
| 638 | 663 | viewportTop := prevViewportTop | |
| @@ -625,14 +625,39 @@ func (r *Renderer) DrawLog(chat, bottom []string, cursorBottomRow, cursorCol int | |||||
| 625 | } | 625 | } | ||
| 626 | } | 626 | } | ||
| 627 | 627 | ||||
| 628 | + | // Rows that have scrolled above the viewport are immutable terminal | |||
| 629 | + | // history. Replaying the full logical buffer to update one of them | |||
| 630 | + | // duplicates that history, and erasing it first destroys native text | |||
| 631 | + | // selections. Ignore inaccessible changes and patch only the first | |||
| 632 | + | // changed row that is still addressable. The cached logical buffer is | |||
| 633 | + | // updated below, so ignored rows remain historical snapshots while new | |||
| 634 | + | // output naturally pushes them farther into scrollback. | |||
| 635 | + | if firstChanged >= 0 && firstChanged < r.logViewportTop { | |||
| 636 | + | firstChanged = -1 | |||
| 637 | + | for idx := r.logViewportTop; idx < maxLines; idx++ { | |||
| 638 | + | oldLine := "" | |||
| 639 | + | if idx < len(r.logLines) { | |||
| 640 | + | oldLine = r.logLines[idx] | |||
| 641 | + | } | |||
| 642 | + | newLine := "" | |||
| 643 | + | if idx < len(lines) { | |||
| 644 | + | newLine = lines[idx] | |||
| 645 | + | } | |||
| 646 | + | if oldLine != newLine { | |||
| 647 | + | firstChanged = idx | |||
| 648 | + | break | |||
| 649 | + | } | |||
| 650 | + | } | |||
| 651 | + | // A newly appended blank row compares equal to the implicit empty | |||
| 652 | + | // row past the old slice, but it still has to advance the terminal. | |||
| 653 | + | if firstChanged == -1 && len(lines) > len(r.logLines) && len(r.logLines) >= r.logViewportTop { | |||
| 654 | + | firstChanged = len(r.logLines) | |||
| 655 | + | } | |||
| 656 | + | } | |||
| 657 | + | ||||
| 628 | if firstChanged == -1 { | 658 | if firstChanged == -1 { | ||
| 629 | - | // No content changes; the final cursor positioning below may still | 659 | + | // No addressable content changes; the final cursor positioning below |
| 630 | - | // move the hardware cursor if the editor cursor changed. | 660 | + | // may still move the hardware cursor if the editor cursor changed. |
| 631 | - | } else if firstChanged < r.logViewportTop { | |||
| 632 | - | // Changes above the visible viewport cannot be patched safely. | |||
| 633 | - | // Repaint without erasing scrollback: the terminal may have a | |||
| 634 | - | // native text selection anchored there while output streams. | |||
| 635 | - | writeFull(true, false) | |||
| 636 | } else { | 661 | } else { | ||
| 637 | prevViewportTop := r.logViewportTop | 662 | prevViewportTop := r.logViewportTop | ||
| 638 | viewportTop := prevViewportTop | 663 | viewportTop := prevViewportTop | ||
M packages/tui/render_test.go +37 -13
| @@ -102,27 +102,51 @@ func TestDrawLogResizeForcesFullRedraw(t *testing.T) { | |||
| 102 | 102 | } | |
| 103 | 103 | } | |
| 104 | 104 | ||
| 105 | - | // TestDrawLogRecoveryPreservesScrollbackSelection covers long streaming | |
| 106 | - | // output whose changing first row has already scrolled above the viewport. | |
| 107 | - | // DrawLog must repaint in that case, but erasing terminal scrollback also | |
| 108 | - | // erases the text backing any native mouse selection the user is making. | |
| 109 | - | func TestDrawLogRecoveryPreservesScrollbackSelection(t *testing.T) { | |
| 105 | + | // TestDrawLogInaccessibleChangePreservesScrollbackSelection covers long | |
| 106 | + | // streaming output whose changing first row has already scrolled above the | |
| 107 | + | // viewport. That row is immutable terminal history: clearing or replaying it | |
| 108 | + | // either destroys a native mouse selection or duplicates stale tool frames. | |
| 109 | + | func TestDrawLogInaccessibleChangePreservesScrollbackSelection(t *testing.T) { | |
| 110 | 110 | t.Setenv("TERM_PROGRAM", "") | |
| 111 | 111 | var buf bytes.Buffer | |
| 112 | 112 | r := NewRenderer(&buf) | |
| 113 | 113 | r.Resize(80, 3) | |
| 114 | - | r.DrawLog([]string{"partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |
| 114 | + | r.DrawLog([]string{"selected partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |
| 115 | 115 | buf.Reset() | |
| 116 | 116 | ||
| 117 | - | // The changed row is above logViewportTop, forcing the recovery repaint | |
| 118 | - | // used while a long streamed response continues to be reformatted. | |
| 119 | - | r.DrawLog([]string{"partial response", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |
| 117 | + | // Only the historical row changed. DrawLog must leave the terminal alone | |
| 118 | + | // instead of clearing and replaying the retained scrollback. | |
| 119 | + | r.DrawLog([]string{"selected partial response", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |
| 120 | 120 | got := buf.String() | |
| 121 | - | if !strings.Contains(got, SeqClearScreenNoHome) { | |
| 122 | - | t.Fatalf("recovery path did not repaint the screen: %q", got) | |
| 121 | + | if strings.Contains(got, SeqClearScreenNoHome) || strings.Contains(got, SeqClearScrollback) { | |
| 122 | + | t.Fatalf("inaccessible change cleared selected scrollback: %q", got) | |
| 123 | 123 | } | |
| 124 | - | if strings.Contains(got, SeqClearScrollback) { | |
| 125 | - | t.Fatalf("recovery repaint erased scrollback and native selection: %q", got) | |
| 124 | + | if strings.Contains(got, "selected partial response") { | |
| 125 | + | t.Fatalf("inaccessible row was replayed into retained scrollback: %q", got) | |
| 126 | + | } | |
| 127 | + | } | |
| 128 | + | ||
| 129 | + | func TestDrawLogInaccessibleChangeStillAppendsNewOutput(t *testing.T) { | |
| 130 | + | t.Setenv("TERM_PROGRAM", "") | |
| 131 | + | var buf bytes.Buffer | |
| 132 | + | r := NewRenderer(&buf) | |
| 133 | + | r.Resize(80, 3) | |
| 134 | + | r.DrawLog([]string{"selected partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |
| 135 | + | buf.Reset() | |
| 136 | + | ||
| 137 | + | // A streaming reflow changes inaccessible history while a tool result is | |
| 138 | + | // appended. Only the new suffix should be emitted, naturally scrolling the | |
| 139 | + | // selected old text upward without replaying the complete frame. | |
| 140 | + | r.DrawLog([]string{"selected partial response", "line 2", "line 3", "line 4", "tool output"}, []string{"input"}, 0, 0) | |
| 141 | + | got := buf.String() | |
| 142 | + | if strings.Contains(got, SeqClearScreenNoHome) || strings.Contains(got, SeqClearScrollback) { | |
| 143 | + | t.Fatalf("append after inaccessible change cleared selected scrollback: %q", got) | |
| 144 | + | } | |
| 145 | + | if strings.Contains(got, "selected partial response") || strings.Contains(got, "line 2") { | |
| 146 | + | t.Fatalf("append replayed historical rows: %q", got) | |
| 147 | + | } | |
| 148 | + | if !strings.Contains(got, "tool output") { | |
| 149 | + | t.Fatalf("new tool output was not appended: %q", got) | |
| 126 | 150 | } | |
| 127 | 151 | } | |
| 128 | 152 | ||
| @@ -102,27 +102,51 @@ func TestDrawLogResizeForcesFullRedraw(t *testing.T) { | |||||
| 102 | } | 102 | } | ||
| 103 | } | 103 | } | ||
| 104 | 104 | ||||
| 105 | - | // TestDrawLogRecoveryPreservesScrollbackSelection covers long streaming | 105 | + | // TestDrawLogInaccessibleChangePreservesScrollbackSelection covers long |
| 106 | - | // output whose changing first row has already scrolled above the viewport. | 106 | + | // streaming output whose changing first row has already scrolled above the |
| 107 | - | // DrawLog must repaint in that case, but erasing terminal scrollback also | 107 | + | // viewport. That row is immutable terminal history: clearing or replaying it |
| 108 | - | // erases the text backing any native mouse selection the user is making. | 108 | + | // either destroys a native mouse selection or duplicates stale tool frames. |
| 109 | - | func TestDrawLogRecoveryPreservesScrollbackSelection(t *testing.T) { | 109 | + | func TestDrawLogInaccessibleChangePreservesScrollbackSelection(t *testing.T) { |
| 110 | t.Setenv("TERM_PROGRAM", "") | 110 | t.Setenv("TERM_PROGRAM", "") | ||
| 111 | var buf bytes.Buffer | 111 | var buf bytes.Buffer | ||
| 112 | r := NewRenderer(&buf) | 112 | r := NewRenderer(&buf) | ||
| 113 | r.Resize(80, 3) | 113 | r.Resize(80, 3) | ||
| 114 | - | r.DrawLog([]string{"partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | 114 | + | r.DrawLog([]string{"selected partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) |
| 115 | buf.Reset() | 115 | buf.Reset() | ||
| 116 | 116 | ||||
| 117 | - | // The changed row is above logViewportTop, forcing the recovery repaint | 117 | + | // Only the historical row changed. DrawLog must leave the terminal alone |
| 118 | - | // used while a long streamed response continues to be reformatted. | 118 | + | // instead of clearing and replaying the retained scrollback. |
| 119 | - | r.DrawLog([]string{"partial response", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | 119 | + | r.DrawLog([]string{"selected partial response", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) |
| 120 | got := buf.String() | 120 | got := buf.String() | ||
| 121 | - | if !strings.Contains(got, SeqClearScreenNoHome) { | 121 | + | if strings.Contains(got, SeqClearScreenNoHome) || strings.Contains(got, SeqClearScrollback) { |
| 122 | - | t.Fatalf("recovery path did not repaint the screen: %q", got) | 122 | + | t.Fatalf("inaccessible change cleared selected scrollback: %q", got) |
| 123 | } | 123 | } | ||
| 124 | - | if strings.Contains(got, SeqClearScrollback) { | 124 | + | if strings.Contains(got, "selected partial response") { |
| 125 | - | t.Fatalf("recovery repaint erased scrollback and native selection: %q", got) | 125 | + | t.Fatalf("inaccessible row was replayed into retained scrollback: %q", got) |
| 126 | + | } | |||
| 127 | + | } | |||
| 128 | + | ||||
| 129 | + | func TestDrawLogInaccessibleChangeStillAppendsNewOutput(t *testing.T) { | |||
| 130 | + | t.Setenv("TERM_PROGRAM", "") | |||
| 131 | + | var buf bytes.Buffer | |||
| 132 | + | r := NewRenderer(&buf) | |||
| 133 | + | r.Resize(80, 3) | |||
| 134 | + | r.DrawLog([]string{"selected partial", "line 2", "line 3", "line 4"}, []string{"input"}, 0, 0) | |||
| 135 | + | buf.Reset() | |||
| 136 | + | ||||
| 137 | + | // A streaming reflow changes inaccessible history while a tool result is | |||
| 138 | + | // appended. Only the new suffix should be emitted, naturally scrolling the | |||
| 139 | + | // selected old text upward without replaying the complete frame. | |||
| 140 | + | r.DrawLog([]string{"selected partial response", "line 2", "line 3", "line 4", "tool output"}, []string{"input"}, 0, 0) | |||
| 141 | + | got := buf.String() | |||
| 142 | + | if strings.Contains(got, SeqClearScreenNoHome) || strings.Contains(got, SeqClearScrollback) { | |||
| 143 | + | t.Fatalf("append after inaccessible change cleared selected scrollback: %q", got) | |||
| 144 | + | } | |||
| 145 | + | if strings.Contains(got, "selected partial response") || strings.Contains(got, "line 2") { | |||
| 146 | + | t.Fatalf("append replayed historical rows: %q", got) | |||
| 147 | + | } | |||
| 148 | + | if !strings.Contains(got, "tool output") { | |||
| 149 | + | t.Fatalf("new tool output was not appended: %q", got) | |||
| 126 | } | 150 | } | ||
| 127 | } | 151 | } | ||
| 128 | 152 | ||||
M packages/tui/view.go +5 -7
| @@ -1323,13 +1323,11 @@ func toolBoxSide(th Theme, line string, width int) string { | |||
| 1323 | 1323 | ||
| 1324 | 1324 | cur := visibleWidth(line) | |
| 1325 | 1325 | if cur > inner { | |
| 1326 | - | // Last-resort truncation. Strips ANSI styling at the cut | |
| 1327 | - | // point; rare path so the simpler implementation is fine. | |
| 1328 | - | line = stripANSI(line) | |
| 1329 | - | runes := []rune(line) | |
| 1330 | - | if len(runes) > inner { | |
| 1331 | - | line = string(runes[:inner]) | |
| 1332 | - | } | |
| 1326 | + | // Last-resort truncation must retain syntax foreground escapes. | |
| 1327 | + | // Stripping ANSI here made long read results switch from their | |
| 1328 | + | // configured syntax color to the terminal's default foreground | |
| 1329 | + | // whenever the window became narrow enough to clip a line. | |
| 1330 | + | line = truncateToWidth(line, inner) | |
| 1333 | 1331 | cur = visibleWidth(line) | |
| 1334 | 1332 | } | |
| 1335 | 1333 | pad := inner - cur | |
| @@ -1323,13 +1323,11 @@ func toolBoxSide(th Theme, line string, width int) string { | |||||
| 1323 | 1323 | ||||
| 1324 | cur := visibleWidth(line) | 1324 | cur := visibleWidth(line) | ||
| 1325 | if cur > inner { | 1325 | if cur > inner { | ||
| 1326 | - | // Last-resort truncation. Strips ANSI styling at the cut | 1326 | + | // Last-resort truncation must retain syntax foreground escapes. |
| 1327 | - | // point; rare path so the simpler implementation is fine. | 1327 | + | // Stripping ANSI here made long read results switch from their |
| 1328 | - | line = stripANSI(line) | 1328 | + | // configured syntax color to the terminal's default foreground |
| 1329 | - | runes := []rune(line) | 1329 | + | // whenever the window became narrow enough to clip a line. |
| 1330 | - | if len(runes) > inner { | 1330 | + | line = truncateToWidth(line, inner) |
| 1331 | - | line = string(runes[:inner]) | |||
| 1332 | - | } | |||
| 1333 | cur = visibleWidth(line) | 1331 | cur = visibleWidth(line) | ||
| 1334 | } | 1332 | } | ||
| 1335 | pad := inner - cur | 1333 | pad := inner - cur | ||
A packages/tui/view_tool_box_test.go +20 -0
| @@ -0,0 +1,20 @@ | |||
| 1 | + | package tui | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "strings" | |
| 5 | + | "testing" | |
| 6 | + | ) | |
| 7 | + | ||
| 8 | + | func TestToolBoxSideTruncationPreservesANSIColor(t *testing.T) { | |
| 9 | + | const width = 32 | |
| 10 | + | const contentColor = 196 | |
| 11 | + | line := " " + Dark.FG256(contentColor, strings.Repeat("x", 80)) | |
| 12 | + | ||
| 13 | + | got := toolBoxSide(Dark, line, width) | |
| 14 | + | if !strings.Contains(got, sgrFG(contentColor)) { | |
| 15 | + | t.Fatalf("narrow tool row lost its content color: %q", got) | |
| 16 | + | } | |
| 17 | + | if gotWidth := visibleWidth(got); gotWidth != width { | |
| 18 | + | t.Fatalf("narrow tool row width = %d, want %d: %q", gotWidth, width, got) | |
| 19 | + | } | |
| 20 | + | } | |
| @@ -0,0 +1,20 @@ | |||||
| 1 | + | package tui | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "strings" | |||
| 5 | + | "testing" | |||
| 6 | + | ) | |||
| 7 | + | ||||
| 8 | + | func TestToolBoxSideTruncationPreservesANSIColor(t *testing.T) { | |||
| 9 | + | const width = 32 | |||
| 10 | + | const contentColor = 196 | |||
| 11 | + | line := " " + Dark.FG256(contentColor, strings.Repeat("x", 80)) | |||
| 12 | + | ||||
| 13 | + | got := toolBoxSide(Dark, line, width) | |||
| 14 | + | if !strings.Contains(got, sgrFG(contentColor)) { | |||
| 15 | + | t.Fatalf("narrow tool row lost its content color: %q", got) | |||
| 16 | + | } | |||
| 17 | + | if gotWidth := visibleWidth(got); gotWidth != width { | |||
| 18 | + | t.Fatalf("narrow tool row width = %d, want %d: %q", gotWidth, width, got) | |||
| 19 | + | } | |||
| 20 | + | } | |||