fix(vt): preserve reflow boundaries across regions
- commit
- f6aceffdd6755b61ec5c82fe9d4b476d9f98fb5e
- author
- brice · · 2026-07-15 15:42 UTC
- parent
- 8040975
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +119 -6
M pkg/vt/buffer.go +16 -6
| @@ -67,17 +67,27 @@ func (b *buffer) clear(y, x0, x1 int) { | |||
| 67 | 67 | } | |
| 68 | 68 | ||
| 69 | 69 | func (b *buffer) scrollUp(top, bottom, n int) { | |
| 70 | - | for ; n > 0; n-- { | |
| 71 | - | copy(b.boundaries[top:bottom], b.boundaries[top+1:bottom+1]) | |
| 72 | - | b.boundaries[bottom] = lineBoundary{} | |
| 70 | + | // A region operation changes which rows meet at both region edges. A soft | |
| 71 | + | // boundary belongs to the row it follows, not the cells moved into its | |
| 72 | + | // neighbor, so sever links crossing either edge before reflow can observe | |
| 73 | + | // them. | |
| 74 | + | b.hard(top - 1) | |
| 75 | + | copy(b.boundaries[top:bottom-n+1], b.boundaries[top+n:bottom+1]) | |
| 76 | + | for y := bottom - n + 1; y <= bottom; y++ { | |
| 77 | + | b.boundaries[y] = lineBoundary{} | |
| 73 | 78 | } | |
| 79 | + | b.hard(bottom - n) | |
| 74 | 80 | } | |
| 75 | 81 | ||
| 76 | 82 | func (b *buffer) scrollDown(top, bottom, n int) { | |
| 77 | - | for ; n > 0; n-- { | |
| 78 | - | copy(b.boundaries[top+1:bottom+1], b.boundaries[top:bottom]) | |
| 79 | - | b.boundaries[top] = lineBoundary{} | |
| 83 | + | // See scrollUp: the old top no longer follows the row above the region, and | |
| 84 | + | // the last moved row no longer precedes the row below it. | |
| 85 | + | b.hard(top - 1) | |
| 86 | + | copy(b.boundaries[top+n:bottom+1], b.boundaries[top:bottom-n+1]) | |
| 87 | + | for y := top; y < top+n; y++ { | |
| 88 | + | b.boundaries[y] = lineBoundary{} | |
| 80 | 89 | } | |
| 90 | + | b.hard(bottom) | |
| 81 | 91 | } | |
| 82 | 92 | ||
| 83 | 93 | func (b *buffer) hydrate() { | |
| @@ -67,17 +67,27 @@ func (b *buffer) clear(y, x0, x1 int) { | |||||
| 67 | } | 67 | } | ||
| 68 | 68 | ||||
| 69 | func (b *buffer) scrollUp(top, bottom, n int) { | 69 | func (b *buffer) scrollUp(top, bottom, n int) { | ||
| 70 | - | for ; n > 0; n-- { | 70 | + | // A region operation changes which rows meet at both region edges. A soft |
| 71 | - | copy(b.boundaries[top:bottom], b.boundaries[top+1:bottom+1]) | 71 | + | // boundary belongs to the row it follows, not the cells moved into its |
| 72 | - | b.boundaries[bottom] = lineBoundary{} | 72 | + | // neighbor, so sever links crossing either edge before reflow can observe |
| 73 | + | // them. | |||
| 74 | + | b.hard(top - 1) | |||
| 75 | + | copy(b.boundaries[top:bottom-n+1], b.boundaries[top+n:bottom+1]) | |||
| 76 | + | for y := bottom - n + 1; y <= bottom; y++ { | |||
| 77 | + | b.boundaries[y] = lineBoundary{} | |||
| 73 | } | 78 | } | ||
| 79 | + | b.hard(bottom - n) | |||
| 74 | } | 80 | } | ||
| 75 | 81 | ||||
| 76 | func (b *buffer) scrollDown(top, bottom, n int) { | 82 | func (b *buffer) scrollDown(top, bottom, n int) { | ||
| 77 | - | for ; n > 0; n-- { | 83 | + | // See scrollUp: the old top no longer follows the row above the region, and |
| 78 | - | copy(b.boundaries[top+1:bottom+1], b.boundaries[top:bottom]) | 84 | + | // the last moved row no longer precedes the row below it. |
| 79 | - | b.boundaries[top] = lineBoundary{} | 85 | + | b.hard(top - 1) |
| 86 | + | copy(b.boundaries[top+n:bottom+1], b.boundaries[top:bottom-n+1]) | |||
| 87 | + | for y := top; y < top+n; y++ { | |||
| 88 | + | b.boundaries[y] = lineBoundary{} | |||
| 80 | } | 89 | } | ||
| 90 | + | b.hard(bottom) | |||
| 81 | } | 91 | } | ||
| 82 | 92 | ||||
| 83 | func (b *buffer) hydrate() { | 93 | func (b *buffer) hydrate() { | ||
A pkg/vt/region_reflow_test.go +52 -0
| @@ -0,0 +1,52 @@ | |||
| 1 | + | package vt | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/require" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestResizeSeversSoftWrapsAtRegionalScrollEdges(t *testing.T) { | |
| 10 | + | tests := []struct { | |
| 11 | + | name string | |
| 12 | + | seq string | |
| 13 | + | want []string | |
| 14 | + | }{ | |
| 15 | + | { | |
| 16 | + | name: "CSI S", | |
| 17 | + | seq: "\x1b[2;4r\x1b[S", | |
| 18 | + | want: []string{"ABC", "IJK", "LMN", "OP ", " ", "QRS"}, | |
| 19 | + | }, | |
| 20 | + | { | |
| 21 | + | name: "CSI T", | |
| 22 | + | seq: "\x1b[2;4r\x1b[T", | |
| 23 | + | want: []string{"ABC", " ", "EFG", "HIJ", "KL ", "QRS"}, | |
| 24 | + | }, | |
| 25 | + | { | |
| 26 | + | name: "insert lines", | |
| 27 | + | seq: "\x1b[2;4r\x1b[2;1H\x1b[L", | |
| 28 | + | want: []string{"ABC", " ", "EFG", "HIJ", "KL ", "QRS"}, | |
| 29 | + | }, | |
| 30 | + | { | |
| 31 | + | name: "delete lines", | |
| 32 | + | seq: "\x1b[2;4r\x1b[2;1H\x1b[M", | |
| 33 | + | want: []string{"ABC", "IJK", "LMN", "OP ", " ", "QRS"}, | |
| 34 | + | }, | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | for _, tt := range tests { | |
| 38 | + | t.Run(tt.name, func(t *testing.T) { | |
| 39 | + | s := NewScreen(4, 5) | |
| 40 | + | s.Write([]byte("ABCDEFGHIJKLMNOPQRST")) | |
| 41 | + | s.Write([]byte(tt.seq)) | |
| 42 | + | ||
| 43 | + | s.Resize(3, 6) | |
| 44 | + | ||
| 45 | + | got := make([]string, s.Frame.Height) | |
| 46 | + | for y := range s.Frame.Height { | |
| 47 | + | got[y] = rowString(s.Frame.Row(y)) | |
| 48 | + | } | |
| 49 | + | require.Equal(t, tt.want, got) | |
| 50 | + | }) | |
| 51 | + | } | |
| 52 | + | } | |
| @@ -0,0 +1,52 @@ | |||||
| 1 | + | package vt | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/require" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestResizeSeversSoftWrapsAtRegionalScrollEdges(t *testing.T) { | |||
| 10 | + | tests := []struct { | |||
| 11 | + | name string | |||
| 12 | + | seq string | |||
| 13 | + | want []string | |||
| 14 | + | }{ | |||
| 15 | + | { | |||
| 16 | + | name: "CSI S", | |||
| 17 | + | seq: "\x1b[2;4r\x1b[S", | |||
| 18 | + | want: []string{"ABC", "IJK", "LMN", "OP ", " ", "QRS"}, | |||
| 19 | + | }, | |||
| 20 | + | { | |||
| 21 | + | name: "CSI T", | |||
| 22 | + | seq: "\x1b[2;4r\x1b[T", | |||
| 23 | + | want: []string{"ABC", " ", "EFG", "HIJ", "KL ", "QRS"}, | |||
| 24 | + | }, | |||
| 25 | + | { | |||
| 26 | + | name: "insert lines", | |||
| 27 | + | seq: "\x1b[2;4r\x1b[2;1H\x1b[L", | |||
| 28 | + | want: []string{"ABC", " ", "EFG", "HIJ", "KL ", "QRS"}, | |||
| 29 | + | }, | |||
| 30 | + | { | |||
| 31 | + | name: "delete lines", | |||
| 32 | + | seq: "\x1b[2;4r\x1b[2;1H\x1b[M", | |||
| 33 | + | want: []string{"ABC", "IJK", "LMN", "OP ", " ", "QRS"}, | |||
| 34 | + | }, | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | for _, tt := range tests { | |||
| 38 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 39 | + | s := NewScreen(4, 5) | |||
| 40 | + | s.Write([]byte("ABCDEFGHIJKLMNOPQRST")) | |||
| 41 | + | s.Write([]byte(tt.seq)) | |||
| 42 | + | ||||
| 43 | + | s.Resize(3, 6) | |||
| 44 | + | ||||
| 45 | + | got := make([]string, s.Frame.Height) | |||
| 46 | + | for y := range s.Frame.Height { | |||
| 47 | + | got[y] = rowString(s.Frame.Row(y)) | |||
| 48 | + | } | |||
| 49 | + | require.Equal(t, tt.want, got) | |||
| 50 | + | }) | |||
| 51 | + | } | |||
| 52 | + | } | |||
M pkg/vt/snapshot.go +8 -0
| @@ -199,6 +199,14 @@ func PreflightVisibleBlob(data []byte) (DecodeStats, error) { | |||
| 199 | 199 | return DecodeStats{}, fmt.Errorf("preflight visible: %w", errInvalidHistory) | |
| 200 | 200 | } | |
| 201 | 201 | } | |
| 202 | + | if string(data[:4]) == visibleMagicV2 { | |
| 203 | + | for range height { | |
| 204 | + | if uint64(binary.BigEndian.Uint32(p.data[:4])) > width || p.data[4] > 1 { | |
| 205 | + | return DecodeStats{}, fmt.Errorf("preflight visible: %w", errInvalidHistory) | |
| 206 | + | } | |
| 207 | + | p.data = p.data[5:] | |
| 208 | + | } | |
| 209 | + | } | |
| 202 | 210 | return DecodeStats{Rows: height, Cells: cells, Styles: cells, Bytes: bytes}, nil | |
| 203 | 211 | } | |
| 204 | 212 | ||
| @@ -199,6 +199,14 @@ func PreflightVisibleBlob(data []byte) (DecodeStats, error) { | |||||
| 199 | return DecodeStats{}, fmt.Errorf("preflight visible: %w", errInvalidHistory) | 199 | return DecodeStats{}, fmt.Errorf("preflight visible: %w", errInvalidHistory) | ||
| 200 | } | 200 | } | ||
| 201 | } | 201 | } | ||
| 202 | + | if string(data[:4]) == visibleMagicV2 { | |||
| 203 | + | for range height { | |||
| 204 | + | if uint64(binary.BigEndian.Uint32(p.data[:4])) > width || p.data[4] > 1 { | |||
| 205 | + | return DecodeStats{}, fmt.Errorf("preflight visible: %w", errInvalidHistory) | |||
| 206 | + | } | |||
| 207 | + | p.data = p.data[5:] | |||
| 208 | + | } | |||
| 209 | + | } | |||
| 202 | return DecodeStats{Rows: height, Cells: cells, Styles: cells, Bytes: bytes}, nil | 210 | return DecodeStats{Rows: height, Cells: cells, Styles: cells, Bytes: bytes}, nil | ||
| 203 | } | 211 | } | ||
| 204 | 212 | ||||
A pkg/vt/snapshot_boundary_test.go +43 -0
| @@ -0,0 +1,43 @@ | |||
| 1 | + | package vt | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "encoding/binary" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/require" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func TestPreflightVisibleV2RejectsMalformedBoundaries(t *testing.T) { | |
| 11 | + | s := NewScreen(4, 2) | |
| 12 | + | s.Write([]byte("abcdef")) | |
| 13 | + | blob, err := s.MarshalPrimaryVisible() | |
| 14 | + | require.NoError(t, err) | |
| 15 | + | ||
| 16 | + | boundaryOffset := 13 + 2*4*historyCellBytes | |
| 17 | + | tests := []struct { | |
| 18 | + | name string | |
| 19 | + | mutate func([]byte) | |
| 20 | + | }{ | |
| 21 | + | { | |
| 22 | + | name: "boundary end exceeds row width", | |
| 23 | + | mutate: func(data []byte) { | |
| 24 | + | binary.BigEndian.PutUint32(data[boundaryOffset:boundaryOffset+4], 5) | |
| 25 | + | }, | |
| 26 | + | }, | |
| 27 | + | { | |
| 28 | + | name: "soft flag is not canonical", | |
| 29 | + | mutate: func(data []byte) { | |
| 30 | + | data[boundaryOffset+4] = 2 | |
| 31 | + | }, | |
| 32 | + | }, | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | for _, tt := range tests { | |
| 36 | + | t.Run(tt.name, func(t *testing.T) { | |
| 37 | + | data := append([]byte(nil), blob...) | |
| 38 | + | tt.mutate(data) | |
| 39 | + | _, err := PreflightVisibleBlob(data) | |
| 40 | + | require.Error(t, err) | |
| 41 | + | }) | |
| 42 | + | } | |
| 43 | + | } | |
| @@ -0,0 +1,43 @@ | |||||
| 1 | + | package vt | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "encoding/binary" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/require" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func TestPreflightVisibleV2RejectsMalformedBoundaries(t *testing.T) { | |||
| 11 | + | s := NewScreen(4, 2) | |||
| 12 | + | s.Write([]byte("abcdef")) | |||
| 13 | + | blob, err := s.MarshalPrimaryVisible() | |||
| 14 | + | require.NoError(t, err) | |||
| 15 | + | ||||
| 16 | + | boundaryOffset := 13 + 2*4*historyCellBytes | |||
| 17 | + | tests := []struct { | |||
| 18 | + | name string | |||
| 19 | + | mutate func([]byte) | |||
| 20 | + | }{ | |||
| 21 | + | { | |||
| 22 | + | name: "boundary end exceeds row width", | |||
| 23 | + | mutate: func(data []byte) { | |||
| 24 | + | binary.BigEndian.PutUint32(data[boundaryOffset:boundaryOffset+4], 5) | |||
| 25 | + | }, | |||
| 26 | + | }, | |||
| 27 | + | { | |||
| 28 | + | name: "soft flag is not canonical", | |||
| 29 | + | mutate: func(data []byte) { | |||
| 30 | + | data[boundaryOffset+4] = 2 | |||
| 31 | + | }, | |||
| 32 | + | }, | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | for _, tt := range tests { | |||
| 36 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 37 | + | data := append([]byte(nil), blob...) | |||
| 38 | + | tt.mutate(data) | |||
| 39 | + | _, err := PreflightVisibleBlob(data) | |||
| 40 | + | require.Error(t, err) | |||
| 41 | + | }) | |||
| 42 | + | } | |||
| 43 | + | } | |||