fix(vt): preserve tab wide-rune and history semantics
- commit
- ad8c50bb85f04ac7924fb80eb705285d3c94a8e8
- author
- brice · · 2026-07-15 04:54 UTC
- parent
- 8138266
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
5 files · +103 -20
M pkg/vt/history_test.go +45 -0
| @@ -147,6 +147,51 @@ func TestHistoryOwnedByScreenIgnoresAlternateScreenEvictions(t *testing.T) { | |||
| 147 | 147 | } | |
| 148 | 148 | } | |
| 149 | 149 | ||
| 150 | + | func TestHistoryRecordsOnlyTopEdgeScrollEvictions(t *testing.T) { | |
| 151 | + | tests := []struct { | |
| 152 | + | name string | |
| 153 | + | top int | |
| 154 | + | bottom int | |
| 155 | + | wantRows int | |
| 156 | + | wantEvents int | |
| 157 | + | }{ | |
| 158 | + | { | |
| 159 | + | name: "interior scroll region does not enter global history", | |
| 160 | + | top: 1, | |
| 161 | + | bottom: 3, | |
| 162 | + | wantRows: 0, | |
| 163 | + | wantEvents: 0, | |
| 164 | + | }, | |
| 165 | + | { | |
| 166 | + | name: "top-edge scroll enters global history", | |
| 167 | + | top: 0, | |
| 168 | + | bottom: 3, | |
| 169 | + | wantRows: 1, | |
| 170 | + | wantEvents: 1, | |
| 171 | + | }, | |
| 172 | + | } | |
| 173 | + | ||
| 174 | + | for _, tt := range tests { | |
| 175 | + | t.Run(tt.name, func(t *testing.T) { | |
| 176 | + | s := NewScreenWithHistory(4, 4, HistoryConfig{MaxRows: 8, ChunkRows: 2}) | |
| 177 | + | for y := range s.Frame.Height { | |
| 178 | + | copy(s.Frame.Row(y), historyRow(string([]byte{byte('A' + y), byte('A' + y), byte('A' + y), byte('A' + y)}))) | |
| 179 | + | } | |
| 180 | + | events := 0 | |
| 181 | + | s.OnLineEvicted = func([]renderer.Cell) { events++ } | |
| 182 | + | ||
| 183 | + | s.scrollUpRegion(tt.top, tt.bottom, 1) | |
| 184 | + | ||
| 185 | + | if got := s.History().View().Len(); got != tt.wantRows { | |
| 186 | + | t.Errorf("history rows = %d, want %d", got, tt.wantRows) | |
| 187 | + | } | |
| 188 | + | if events != tt.wantEvents { | |
| 189 | + | t.Errorf("eviction events = %d, want %d", events, tt.wantEvents) | |
| 190 | + | } | |
| 191 | + | }) | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | ||
| 150 | 195 | func historyRow(text string) []renderer.Cell { | |
| 151 | 196 | row := make([]renderer.Cell, len(text)) | |
| 152 | 197 | for i, r := range text { | |
| @@ -147,6 +147,51 @@ func TestHistoryOwnedByScreenIgnoresAlternateScreenEvictions(t *testing.T) { | |||||
| 147 | } | 147 | } | ||
| 148 | } | 148 | } | ||
| 149 | 149 | ||||
| 150 | + | func TestHistoryRecordsOnlyTopEdgeScrollEvictions(t *testing.T) { | |||
| 151 | + | tests := []struct { | |||
| 152 | + | name string | |||
| 153 | + | top int | |||
| 154 | + | bottom int | |||
| 155 | + | wantRows int | |||
| 156 | + | wantEvents int | |||
| 157 | + | }{ | |||
| 158 | + | { | |||
| 159 | + | name: "interior scroll region does not enter global history", | |||
| 160 | + | top: 1, | |||
| 161 | + | bottom: 3, | |||
| 162 | + | wantRows: 0, | |||
| 163 | + | wantEvents: 0, | |||
| 164 | + | }, | |||
| 165 | + | { | |||
| 166 | + | name: "top-edge scroll enters global history", | |||
| 167 | + | top: 0, | |||
| 168 | + | bottom: 3, | |||
| 169 | + | wantRows: 1, | |||
| 170 | + | wantEvents: 1, | |||
| 171 | + | }, | |||
| 172 | + | } | |||
| 173 | + | ||||
| 174 | + | for _, tt := range tests { | |||
| 175 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 176 | + | s := NewScreenWithHistory(4, 4, HistoryConfig{MaxRows: 8, ChunkRows: 2}) | |||
| 177 | + | for y := range s.Frame.Height { | |||
| 178 | + | copy(s.Frame.Row(y), historyRow(string([]byte{byte('A' + y), byte('A' + y), byte('A' + y), byte('A' + y)}))) | |||
| 179 | + | } | |||
| 180 | + | events := 0 | |||
| 181 | + | s.OnLineEvicted = func([]renderer.Cell) { events++ } | |||
| 182 | + | ||||
| 183 | + | s.scrollUpRegion(tt.top, tt.bottom, 1) | |||
| 184 | + | ||||
| 185 | + | if got := s.History().View().Len(); got != tt.wantRows { | |||
| 186 | + | t.Errorf("history rows = %d, want %d", got, tt.wantRows) | |||
| 187 | + | } | |||
| 188 | + | if events != tt.wantEvents { | |||
| 189 | + | t.Errorf("eviction events = %d, want %d", events, tt.wantEvents) | |||
| 190 | + | } | |||
| 191 | + | }) | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | ||||
| 150 | func historyRow(text string) []renderer.Cell { | 195 | func historyRow(text string) []renderer.Cell { | ||
| 151 | row := make([]renderer.Cell, len(text)) | 196 | row := make([]renderer.Cell, len(text)) | ||
| 152 | for i, r := range text { | 197 | for i, r := range text { | ||
M pkg/vt/screen_helpers_test.go +8 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package vt | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "reflect" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/bnema/vev/pkg/renderer" | |
| @@ -27,6 +28,13 @@ func cellAt(s *Screen, x, y int) renderer.Cell { | |||
| 27 | 28 | return s.Frame.At(x, y) | |
| 28 | 29 | } | |
| 29 | 30 | ||
| 31 | + | func assertFramesEqual(t *testing.T, got, want renderer.Frame) { | |
| 32 | + | t.Helper() | |
| 33 | + | if !reflect.DeepEqual(got, want) { | |
| 34 | + | t.Fatal("frame changed unexpectedly") | |
| 35 | + | } | |
| 36 | + | } | |
| 37 | + | ||
| 30 | 38 | func assertCell(t *testing.T, s *Screen, x, y int, expected rune) { | |
| 31 | 39 | t.Helper() | |
| 32 | 40 | c := cellAt(s, x, y) | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package vt | 1 | package vt | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "reflect" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/bnema/vev/pkg/renderer" | 7 | "github.com/bnema/vev/pkg/renderer" | ||
| @@ -27,6 +28,13 @@ func cellAt(s *Screen, x, y int) renderer.Cell { | |||||
| 27 | return s.Frame.At(x, y) | 28 | return s.Frame.At(x, y) | ||
| 28 | } | 29 | } | ||
| 29 | 30 | ||||
| 31 | + | func assertFramesEqual(t *testing.T, got, want renderer.Frame) { | |||
| 32 | + | t.Helper() | |||
| 33 | + | if !reflect.DeepEqual(got, want) { | |||
| 34 | + | t.Fatal("frame changed unexpectedly") | |||
| 35 | + | } | |||
| 36 | + | } | |||
| 37 | + | ||||
| 30 | func assertCell(t *testing.T, s *Screen, x, y int, expected rune) { | 38 | func assertCell(t *testing.T, s *Screen, x, y int, expected rune) { | ||
| 31 | t.Helper() | 39 | t.Helper() | ||
| 32 | c := cellAt(s, x, y) | 40 | c := cellAt(s, x, y) | ||
M pkg/vt/screen_scroll.go +3 -1
| @@ -74,7 +74,9 @@ func (s *Screen) scrollUpRegion(top, bottom, n int) { | |||
| 74 | 74 | } | |
| 75 | 75 | ||
| 76 | 76 | func (s *Screen) emitLineEvicted(top, n int) { | |
| 77 | - | if s.alternate != nil { | |
| 77 | + | // Only rows leaving the top edge of the primary screen belong to global | |
| 78 | + | // scrollback. Interior DECSTBM scroll regions are local mutations. | |
| 79 | + | if s.alternate != nil || top != 0 { | |
| 78 | 80 | return | |
| 79 | 81 | } | |
| 80 | 82 | for y := top; y < top+n; y++ { | |
| @@ -74,7 +74,9 @@ func (s *Screen) scrollUpRegion(top, bottom, n int) { | |||||
| 74 | } | 74 | } | ||
| 75 | 75 | ||||
| 76 | func (s *Screen) emitLineEvicted(top, n int) { | 76 | func (s *Screen) emitLineEvicted(top, n int) { | ||
| 77 | - | if s.alternate != nil { | 77 | + | // Only rows leaving the top edge of the primary screen belong to global |
| 78 | + | // scrollback. Interior DECSTBM scroll regions are local mutations. | |||
| 79 | + | if s.alternate != nil || top != 0 { | |||
| 78 | return | 80 | return | ||
| 79 | } | 81 | } | ||
| 80 | for y := top; y < top+n; y++ { | 82 | for y := top; y < top+n; y++ { | ||
M pkg/vt/screen_write.go +8 -7
| @@ -23,9 +23,10 @@ func (s *Screen) putRune(r rune) { | |||
| 23 | 23 | s.Col-- | |
| 24 | 24 | } | |
| 25 | 25 | case '\t': | |
| 26 | - | spaces := 8 - (s.Col % 8) | |
| 27 | - | for range spaces { | |
| 28 | - | s.putPrintable(' ') | |
| 26 | + | // HT moves to the next tab stop without modifying the frame. Clamp at | |
| 27 | + | // the right edge rather than triggering deferred wrap or a scroll. | |
| 28 | + | if s.Frame.Width > 0 { | |
| 29 | + | s.Col = min(s.Col+8-s.Col%8, s.Frame.Width-1) | |
| 29 | 30 | } | |
| 30 | 31 | default: | |
| 31 | 32 | // Drop C1 control characters (U+0080-U+009F). | |
| @@ -60,10 +61,10 @@ func (s *Screen) putPrintable(r rune) { | |||
| 60 | 61 | // current line, clear the abandoned last cell and wrap to the next line. | |
| 61 | 62 | if w == 2 && s.Col+1 >= s.Frame.Width { | |
| 62 | 63 | if s.Frame.Width < 2 { | |
| 63 | - | // The screen is too narrow to ever hold a wide rune; degrade to a | |
| 64 | - | // single narrow cell so the cursor stays aligned and we never write | |
| 65 | - | // a continuation cell out of bounds. | |
| 66 | - | w = 1 | |
| 64 | + | // The screen is too narrow to ever hold a wide rune; store a narrow | |
| 65 | + | // replacement so the cell's renderer width matches its layout. | |
| 66 | + | r = '\uFFFD' | |
| 67 | + | w = renderer.RuneWidth(r) | |
| 67 | 68 | } else { | |
| 68 | 69 | // The abandoned cell may itself be the continuation of a pair whose | |
| 69 | 70 | // left half sits one column back; report damage over the full span. | |
| @@ -23,9 +23,10 @@ func (s *Screen) putRune(r rune) { | |||||
| 23 | s.Col-- | 23 | s.Col-- | ||
| 24 | } | 24 | } | ||
| 25 | case '\t': | 25 | case '\t': | ||
| 26 | - | spaces := 8 - (s.Col % 8) | 26 | + | // HT moves to the next tab stop without modifying the frame. Clamp at |
| 27 | - | for range spaces { | 27 | + | // the right edge rather than triggering deferred wrap or a scroll. |
| 28 | - | s.putPrintable(' ') | 28 | + | if s.Frame.Width > 0 { |
| 29 | + | s.Col = min(s.Col+8-s.Col%8, s.Frame.Width-1) | |||
| 29 | } | 30 | } | ||
| 30 | default: | 31 | default: | ||
| 31 | // Drop C1 control characters (U+0080-U+009F). | 32 | // Drop C1 control characters (U+0080-U+009F). | ||
| @@ -60,10 +61,10 @@ func (s *Screen) putPrintable(r rune) { | |||||
| 60 | // current line, clear the abandoned last cell and wrap to the next line. | 61 | // current line, clear the abandoned last cell and wrap to the next line. | ||
| 61 | if w == 2 && s.Col+1 >= s.Frame.Width { | 62 | if w == 2 && s.Col+1 >= s.Frame.Width { | ||
| 62 | if s.Frame.Width < 2 { | 63 | if s.Frame.Width < 2 { | ||
| 63 | - | // The screen is too narrow to ever hold a wide rune; degrade to a | 64 | + | // The screen is too narrow to ever hold a wide rune; store a narrow |
| 64 | - | // single narrow cell so the cursor stays aligned and we never write | 65 | + | // replacement so the cell's renderer width matches its layout. |
| 65 | - | // a continuation cell out of bounds. | 66 | + | r = '\uFFFD' |
| 66 | - | w = 1 | 67 | + | w = renderer.RuneWidth(r) |
| 67 | } else { | 68 | } else { | ||
| 68 | // The abandoned cell may itself be the continuation of a pair whose | 69 | // The abandoned cell may itself be the continuation of a pair whose | ||
| 69 | // left half sits one column back; report damage over the full span. | 70 | // left half sits one column back; report damage over the full span. | ||
M pkg/vt/screen_write_test.go +39 -12
| @@ -190,17 +190,37 @@ func TestCursorControlChars(t *testing.T) { | |||
| 190 | 190 | }, | |
| 191 | 191 | }, | |
| 192 | 192 | { | |
| 193 | - | name: "tab advances to next 8-column boundary", | |
| 193 | + | name: "tab moves cursor without changing cells or damage", | |
| 194 | 194 | run: func(t *testing.T) { | |
| 195 | - | s := NewScreen(20, 3) | |
| 196 | - | s.Write([]byte("A\tB")) | |
| 195 | + | s := NewScreen(10, 2) | |
| 196 | + | for y := range s.Frame.Height { | |
| 197 | + | for x := range s.Frame.Width { | |
| 198 | + | s.Frame.Set(x, y, renderer.Cell{Rune: rune('a' + y*s.Frame.Width + x)}) | |
| 199 | + | } | |
| 200 | + | } | |
| 201 | + | before := s.Frame.Clone() | |
| 202 | + | s.Row, s.Col = 1, 6 | |
| 203 | + | s.ClearDamage() | |
| 197 | 204 | ||
| 198 | - | assertCell(t, s, 0, 0, 'A') | |
| 199 | - | // Tab advances to next 8-column boundary (col 8). | |
| 200 | - | for i := 1; i < 8; i++ { | |
| 201 | - | assertCell(t, s, i, 0, ' ') | |
| 205 | + | s.Write([]byte("\t")) | |
| 206 | + | if s.Row != 1 || s.Col != 8 { | |
| 207 | + | t.Errorf("cursor after tab = row=%d col=%d, want row=1 col=8", s.Row, s.Col) | |
| 208 | + | } | |
| 209 | + | assertFramesEqual(t, s.Frame, before) | |
| 210 | + | if got := s.Damage(); len(got) != 0 { | |
| 211 | + | t.Errorf("tab damage = %+v, want none", got) | |
| 212 | + | } | |
| 213 | + | ||
| 214 | + | // A tab whose next stop lies past the edge clamps at the last | |
| 215 | + | // column; it must not wrap or scroll the bottom row. | |
| 216 | + | s.Write([]byte("\t")) | |
| 217 | + | if s.Row != 1 || s.Col != 9 { | |
| 218 | + | t.Errorf("cursor after clamped tab = row=%d col=%d, want row=1 col=9", s.Row, s.Col) | |
| 219 | + | } | |
| 220 | + | assertFramesEqual(t, s.Frame, before) | |
| 221 | + | if got := s.Damage(); len(got) != 0 { | |
| 222 | + | t.Errorf("clamped tab damage = %+v, want none", got) | |
| 202 | 223 | } | |
| 203 | - | assertCell(t, s, 8, 0, 'B') | |
| 204 | 224 | }, | |
| 205 | 225 | }, | |
| 206 | 226 | } | |
| @@ -675,14 +695,21 @@ func TestWideAndZeroWidthRunes(t *testing.T) { | |||
| 675 | 695 | }, | |
| 676 | 696 | }, | |
| 677 | 697 | { | |
| 678 | - | name: "wide char on a width-1 screen degrades to a single cell", | |
| 698 | + | name: "wide char on a width-1 screen becomes a narrow replacement rune", | |
| 679 | 699 | run: func(t *testing.T) { | |
| 680 | 700 | s := NewScreen(1, 2) | |
| 681 | - | // A wide rune cannot fit on a 1-column screen; it must not write | |
| 682 | - | // an out-of-bounds continuation cell. | |
| 701 | + | // A wide rune cannot fit on a 1-column screen. Store a narrow | |
| 702 | + | // replacement instead so renderer width matches the cell layout. | |
| 683 | 703 | s.Write([]byte("你")) | |
| 684 | 704 | ||
| 685 | - | assertCell(t, s, 0, 0, '你') | |
| 705 | + | assertCell(t, s, 0, 0, '\uFFFD') | |
| 706 | + | if got := renderer.RuneWidth(cellAt(s, 0, 0).Rune); got != 1 { | |
| 707 | + | t.Errorf("stored rune width = %d, want 1", got) | |
| 708 | + | } | |
| 709 | + | if cellAt(s, 0, 0).Continuation { | |
| 710 | + | t.Error("width-1 replacement must not be a continuation") | |
| 711 | + | } | |
| 712 | + | assertNoOrphanWideCells(t, s) | |
| 686 | 713 | if s.Col != 1 { | |
| 687 | 714 | t.Errorf("cursor at col=%d, want 1", s.Col) | |
| 688 | 715 | } | |
| @@ -190,17 +190,37 @@ func TestCursorControlChars(t *testing.T) { | |||||
| 190 | }, | 190 | }, | ||
| 191 | }, | 191 | }, | ||
| 192 | { | 192 | { | ||
| 193 | - | name: "tab advances to next 8-column boundary", | 193 | + | name: "tab moves cursor without changing cells or damage", |
| 194 | run: func(t *testing.T) { | 194 | run: func(t *testing.T) { | ||
| 195 | - | s := NewScreen(20, 3) | 195 | + | s := NewScreen(10, 2) |
| 196 | - | s.Write([]byte("A\tB")) | 196 | + | for y := range s.Frame.Height { |
| 197 | + | for x := range s.Frame.Width { | |||
| 198 | + | s.Frame.Set(x, y, renderer.Cell{Rune: rune('a' + y*s.Frame.Width + x)}) | |||
| 199 | + | } | |||
| 200 | + | } | |||
| 201 | + | before := s.Frame.Clone() | |||
| 202 | + | s.Row, s.Col = 1, 6 | |||
| 203 | + | s.ClearDamage() | |||
| 197 | 204 | ||||
| 198 | - | assertCell(t, s, 0, 0, 'A') | 205 | + | s.Write([]byte("\t")) |
| 199 | - | // Tab advances to next 8-column boundary (col 8). | 206 | + | if s.Row != 1 || s.Col != 8 { |
| 200 | - | for i := 1; i < 8; i++ { | 207 | + | t.Errorf("cursor after tab = row=%d col=%d, want row=1 col=8", s.Row, s.Col) |
| 201 | - | assertCell(t, s, i, 0, ' ') | 208 | + | } |
| 209 | + | assertFramesEqual(t, s.Frame, before) | |||
| 210 | + | if got := s.Damage(); len(got) != 0 { | |||
| 211 | + | t.Errorf("tab damage = %+v, want none", got) | |||
| 212 | + | } | |||
| 213 | + | ||||
| 214 | + | // A tab whose next stop lies past the edge clamps at the last | |||
| 215 | + | // column; it must not wrap or scroll the bottom row. | |||
| 216 | + | s.Write([]byte("\t")) | |||
| 217 | + | if s.Row != 1 || s.Col != 9 { | |||
| 218 | + | t.Errorf("cursor after clamped tab = row=%d col=%d, want row=1 col=9", s.Row, s.Col) | |||
| 219 | + | } | |||
| 220 | + | assertFramesEqual(t, s.Frame, before) | |||
| 221 | + | if got := s.Damage(); len(got) != 0 { | |||
| 222 | + | t.Errorf("clamped tab damage = %+v, want none", got) | |||
| 202 | } | 223 | } | ||
| 203 | - | assertCell(t, s, 8, 0, 'B') | |||
| 204 | }, | 224 | }, | ||
| 205 | }, | 225 | }, | ||
| 206 | } | 226 | } | ||
| @@ -675,14 +695,21 @@ func TestWideAndZeroWidthRunes(t *testing.T) { | |||||
| 675 | }, | 695 | }, | ||
| 676 | }, | 696 | }, | ||
| 677 | { | 697 | { | ||
| 678 | - | name: "wide char on a width-1 screen degrades to a single cell", | 698 | + | name: "wide char on a width-1 screen becomes a narrow replacement rune", |
| 679 | run: func(t *testing.T) { | 699 | run: func(t *testing.T) { | ||
| 680 | s := NewScreen(1, 2) | 700 | s := NewScreen(1, 2) | ||
| 681 | - | // A wide rune cannot fit on a 1-column screen; it must not write | 701 | + | // A wide rune cannot fit on a 1-column screen. Store a narrow |
| 682 | - | // an out-of-bounds continuation cell. | 702 | + | // replacement instead so renderer width matches the cell layout. |
| 683 | s.Write([]byte("你")) | 703 | s.Write([]byte("你")) | ||
| 684 | 704 | ||||
| 685 | - | assertCell(t, s, 0, 0, '你') | 705 | + | assertCell(t, s, 0, 0, '\uFFFD') |
| 706 | + | if got := renderer.RuneWidth(cellAt(s, 0, 0).Rune); got != 1 { | |||
| 707 | + | t.Errorf("stored rune width = %d, want 1", got) | |||
| 708 | + | } | |||
| 709 | + | if cellAt(s, 0, 0).Continuation { | |||
| 710 | + | t.Error("width-1 replacement must not be a continuation") | |||
| 711 | + | } | |||
| 712 | + | assertNoOrphanWideCells(t, s) | |||
| 686 | if s.Col != 1 { | 713 | if s.Col != 1 { | ||
| 687 | t.Errorf("cursor at col=%d, want 1", s.Col) | 714 | t.Errorf("cursor at col=%d, want 1", s.Col) | ||
| 688 | } | 715 | } | ||