fix(vt): keep the soft link when a deferred wrap scrolls the bottom row

commit: f82f38ed4dd43f758a4a50583bbb8467b05d08ae
author: brice · 2mo ago · 2026-07-25 10:07 UTC
parent: e522e52
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

3 files · +169 -10

M pkg/vt/screen_scroll.go +20 -8


@@ -6,19 +6,25 @@ import (
6	6		"github.com/bnema/vev/pkg/renderer"
7	7		)
8	8
9		-	func (s *Screen) index() {
	9	+	// index moves to the next physical row, scrolling the region when the cursor
	10	+	// already sits on its last row. It reports whether the row the cursor was on
	11	+	// survived that scroll by moving up to s.Row-1, which only a deferred wrap
	12	+	// cares about: a wrap must relink to the row it flowed out of, and
	13	+	// buffer.scrollUp severs that link on the way. Callers that merely advance,
	14	+	// such as LF and ESC D, ignore the result.
	15	+	func (s *Screen) index() (movedUp bool) {
10	16		if s.Frame.Height == 0 {
11		-	return
	17	+	return false
12	18		}
13	19		if s.Row == s.scrollBottom && s.Row >= s.scrollTop {
14		-	s.scrollUpRegion(s.scrollTop, s.scrollBottom, 1)
15		-	return
	20	+	return s.scrollUpRegion(s.scrollTop, s.scrollBottom, 1)
16	21		}
17	22		if s.Row+1 < s.Frame.Height {
18	23		s.Row++
19		-	return
	24	+	return false
20	25		}
21	26		s.Row = s.Frame.Height - 1
	27	+	return false
22	28		}
23	29
24	30		func (s *Screen) nextLine() {

@@ -56,13 +62,18 @@ func (s *Screen) scrollDownBy(n int) {
56	62		s.scrollDownRegion(s.scrollTop, s.scrollBottom, n)
57	63		}
58	64
59		-	func (s *Screen) scrollUpRegion(top, bottom, n int) {
	65	+	// scrollUpRegion scrolls [top,bottom] up by n. It reports whether any row
	66	+	// survived by moving up: a region exactly n rows tall is blanked in place, so
	67	+	// the receiving range [top,bottom-n] is empty and nothing survives. Reporting
	68	+	// what the scroll observed keeps callers from re-deriving a predicate that this
	69	+	// function may have declined to act on at all.
	70	+	func (s *Screen) scrollUpRegion(top, bottom, n int) (shifted bool) {
60	71		if s.Frame.Width == 0 \|\| s.Frame.Height == 0 \|\| n <= 0 {
61		-	return
	72	+	return false
62	73		}
63	74		top, bottom, ok := s.normalizedRegion(top, bottom)
64	75		if !ok {
65		-	return
	76	+	return false
66	77		}
67	78		w := s.Frame.Width
68	79		height := bottom - top + 1

@@ -77,6 +88,7 @@ func (s *Screen) scrollUpRegion(top, bottom, n int) {
77	88		s.buffer.scrollUp(top, bottom, n)
78	89		s.record(renderer.Damage{Kind: renderer.DamageScrollUp, X: 0, Y: top, Width: w, Height: height, Count: n})
79	90		s.record(renderer.Damage{Kind: renderer.DamageText, X: 0, Y: bottom - n + 1, Width: w, Height: n, Count: 1})
	91	+	return bottom-n >= top
80	92		}
81	93
82	94		func (s *Screen) emitLineEvicted(top, n int) {

 	"github.com/bnema/vev/pkg/renderer"
 )
-func (s *Screen) index() {
+// index moves to the next physical row, scrolling the region when the cursor
+// already sits on its last row. It reports whether the row the cursor was on
+// survived that scroll by moving up to s.Row-1, which only a deferred wrap
+// cares about: a wrap must relink to the row it flowed out of, and
+// buffer.scrollUp severs that link on the way. Callers that merely advance,
+// such as LF and ESC D, ignore the result.
+func (s *Screen) index() (movedUp bool) {
 	if s.Frame.Height == 0 {
-		return
+		return false
 	}
 	if s.Row == s.scrollBottom && s.Row >= s.scrollTop {
-		s.scrollUpRegion(s.scrollTop, s.scrollBottom, 1)
+		return s.scrollUpRegion(s.scrollTop, s.scrollBottom, 1)
-		return
 	}
 	if s.Row+1 < s.Frame.Height {
 		s.Row++
-		return
+		return false
 	}
 	s.Row = s.Frame.Height - 1
+	return false
 }
 func (s *Screen) nextLine() {
 	s.scrollDownRegion(s.scrollTop, s.scrollBottom, n)
 }
-func (s *Screen) scrollUpRegion(top, bottom, n int) {
+// scrollUpRegion scrolls [top,bottom] up by n. It reports whether any row
+// survived by moving up: a region exactly n rows tall is blanked in place, so
+// the receiving range [top,bottom-n] is empty and nothing survives. Reporting
+// what the scroll observed keeps callers from re-deriving a predicate that this
+// function may have declined to act on at all.
+func (s *Screen) scrollUpRegion(top, bottom, n int) (shifted bool) {
 	if s.Frame.Width == 0 || s.Frame.Height == 0 || n <= 0 {
-		return
+		return false
 	}
 	top, bottom, ok := s.normalizedRegion(top, bottom)
 	if !ok {
-		return
+		return false
 	}
 	w := s.Frame.Width
 	height := bottom - top + 1
 	s.buffer.scrollUp(top, bottom, n)
 	s.record(renderer.Damage{Kind: renderer.DamageScrollUp, X: 0, Y: top, Width: w, Height: height, Count: n})
 	s.record(renderer.Damage{Kind: renderer.DamageText, X: 0, Y: bottom - n + 1, Width: w, Height: n, Count: 1})
+	return bottom-n >= top
 }
 func (s *Screen) emitLineEvicted(top, n int) {

M pkg/vt/screen_write.go +14 -2


@@ -40,6 +40,18 @@ func (s *Screen) putRune(r rune) {
40	40		}
41	41		}
42	42
	43	+	// relinkWrap restores the soft link a deferred wrap just set when index()
	44	+	// scrolled the row out from under it. buffer.scrollUp severs the last moved
	45	+	// row on purpose — a linefeed there really does stop preceding anything — but
	46	+	// a wrap is the one case where that row flowed into the row below it, so the
	47	+	// link has to be put back after the rotation moved it to s.Row-1. movedUp is
	48	+	// false when nothing survived the scroll, and then there is no row to link.
	49	+	func (s *Screen) relinkWrap(movedUp bool) {
	50	+	if movedUp {
	51	+	s.buffer.continueRow(s.Row - 1)
	52	+	}
	53	+	}
	54	+
43	55		func (s *Screen) putPrintable(r rune) {
44	56		w := renderer.RuneWidth(r)
45	57		// Skip combining marks and zero-width characters.

@@ -53,7 +65,7 @@ func (s *Screen) putPrintable(r rune) {
53	65		if s.Col >= s.Frame.Width {
54	66		s.buffer.soft(s.Row)
55	67		s.Col = 0
56		-	s.index()
	68	+	s.relinkWrap(s.index())
57	69		}
58	70		if s.Row >= s.Frame.Height {
59	71		s.Row = s.Frame.Height - 1

@@ -80,7 +92,7 @@ func (s *Screen) putPrintable(r rune) {
80	92		s.buffer.continueRow(s.Row)
81	93		s.record(renderer.Damage{Kind: renderer.DamageText, X: cx, Y: s.Row, Width: s.Col - cx + 1, Height: 1, Count: 1})
82	94		s.Col = 0
83		-	s.index()
	95	+	s.relinkWrap(s.index())
84	96		if s.Row >= s.Frame.Height {
85	97		s.Row = s.Frame.Height - 1
86	98		}

 	}
 }
+// relinkWrap restores the soft link a deferred wrap just set when index()
+// scrolled the row out from under it. buffer.scrollUp severs the last moved
+// row on purpose — a linefeed there really does stop preceding anything — but
+// a wrap is the one case where that row flowed into the row below it, so the
+// link has to be put back after the rotation moved it to s.Row-1. movedUp is
+// false when nothing survived the scroll, and then there is no row to link.
+func (s *Screen) relinkWrap(movedUp bool) {
+	if movedUp {
+		s.buffer.continueRow(s.Row - 1)
+	}
+}
 func (s *Screen) putPrintable(r rune) {
 	w := renderer.RuneWidth(r)
 	// Skip combining marks and zero-width characters.
 	if s.Col >= s.Frame.Width {
 		s.buffer.soft(s.Row)
 		s.Col = 0
-		s.index()
+		s.relinkWrap(s.index())
 	}
 	if s.Row >= s.Frame.Height {
 		s.Row = s.Frame.Height - 1
 			s.buffer.continueRow(s.Row)
 			s.record(renderer.Damage{Kind: renderer.DamageText, X: cx, Y: s.Row, Width: s.Col - cx + 1, Height: 1, Count: 1})
 			s.Col = 0
-			s.index()
+			s.relinkWrap(s.index())
 			if s.Row >= s.Frame.Height {
 				s.Row = s.Frame.Height - 1
 			}

A pkg/vt/wrap_scroll_soft_test.go +135 -0

@@ -0,0 +1,135 @@
	1	+	package vt
	2	+
	3	+	import (
	4	+	"testing"
	5	+
	6	+	"github.com/stretchr/testify/require"
	7	+	)
	8	+
	9	+	// A deferred wrap out of the last row of a scroll region scrolls that region,
	10	+	// and buffer.scrollUp deliberately severs the link on the last moved row. That
	11	+	// sever is right for a linefeed and wrong when the wrap itself caused the
	12	+	// scroll, so these cases pin both sides of the distinction.
	13	+	func TestSoftLinkSurvivesTheScrollItCaused(t *testing.T) {
	14	+	const width = 8
	15	+	cases := []struct {
	16	+	name string
	17	+	height int
	18	+	input string
	19	+	// wantBounds is indexed like the live grid.
	20	+	wantBounds map[int]LineBound
	21	+	// wantRows pins the grid text so a bound is never read off a row that
	22	+	// holds something other than what the case describes.
	23	+	wantRows map[int]string
	24	+	}{
	25	+	{
	26	+	// The bug: the cursor sits on the last row, as it does in any full
	27	+	// shell session, so the wrap scrolls and the link must survive.
	28	+	name: "narrow wrap out of the bottom row keeps the link",
	29	+	height: 4,
	30	+	input: "1\r\n2\r\n3\r\n4\r\n5\r\nabcdefghij",
	31	+	wantBounds: map[int]LineBound{
	32	+	2: {End: 8, Soft: true},
	33	+	3: {End: 2},
	34	+	},
	35	+	wantRows: map[int]string{2: "abcdefgh", 3: "ij "},
	36	+	},
	37	+	{
	38	+	// Same shape at the wide-rune wrap site: the wide rune does not fit
	39	+	// in the last column, so the writer abandons that cell and wraps.
	40	+	name: "wide-rune wrap out of the bottom row keeps the link",
	41	+	height: 4,
	42	+	input: "1\r\n2\r\n3\r\nabcdefg漢",
	43	+	wantBounds: map[int]LineBound{
	44	+	2: {End: 7, Soft: true},
	45	+	3: {End: 2},
	46	+	},
	47	+	wantRows: map[int]string{2: "abcdefg "},
	48	+	},
	49	+	{
	50	+	// The sever that must survive: a full-width row ending in an explicit
	51	+	// newline is a hard line that merely happens to fill the grid.
	52	+	name: "explicit newline after a full-width bottom row severs",
	53	+	height: 4,
	54	+	input: "1\r\n2\r\n3\r\nabcdefgh\r\n",
	55	+	wantBounds: map[int]LineBound{
	56	+	2: {End: 8, Soft: false},
	57	+	},
	58	+	wantRows: map[int]string{2: "abcdefgh"},
	59	+	},
	60	+	{
	61	+	// A DECSTBM region scroll is a local mutation, not a wrap: it must
	62	+	// still sever at the row above the region (top-1), whose
	63	+	// continuation the scroll just destroyed, and at the last moved row.
	64	+	name: "DECSTBM region scroll severs at both edges",
	65	+	height: 4,
	66	+	input: "abcdefghij" + "\x1b[2;3r" + "\x1b[3;1H" + "\n",
	67	+	wantBounds: map[int]LineBound{
	68	+	0: {End: 8, Soft: false},
	69	+	1: {End: 0, Soft: false},
	70	+	},
	71	+	wantRows: map[int]string{0: "abcdefgh"},
	72	+	},
	73	+	{
	74	+	// A region exactly one row tall blanks the wrapped row in place
	75	+	// instead of moving it up, so no surviving grid row may be linked.
	76	+	// setScrollRegion rejects top >= bottom, so the only reachable
	77	+	// one-row region is a one-row screen.
	78	+	name: "wrap on a one-row screen links nothing on the grid",
	79	+	height: 1,
	80	+	input: "abcdefghij",
	81	+	wantBounds: map[int]LineBound{
	82	+	0: {End: 2, Soft: false},
	83	+	},
	84	+	wantRows: map[int]string{0: "ij "},
	85	+	},
	86	+	}
	87	+	for _, tc := range cases {
	88	+	t.Run(tc.name, func(t *testing.T) {
	89	+	s := NewScreenWithHistory(width, tc.height, HistoryConfig{MaxRows: 64, MaxCells: 4096})
	90	+	s.Write([]byte(tc.input))
	91	+
	92	+	bounds := s.LineBounds()
	93	+	for y, want := range tc.wantBounds {
	94	+	require.Equal(t, want, bounds[y], "LineBounds()[%d]", y)
	95	+	}
	96	+	for y, want := range tc.wantRows {
	97	+	require.Equal(t, want, lineText(s, y), "row %d text", y)
	98	+	}
	99	+	})
	100	+	}
	101	+	}
	102	+
	103	+	// The soft link also drives reflow, so restoring it fixes resize too: a line
	104	+	// wrapped out of the bottom row used to be two unrelated physical rows, and
	105	+	// widening the pane left it split instead of rejoining it.
	106	+	func TestResizeRejoinsALineWrappedOutOfTheBottomRow(t *testing.T) {
	107	+	s := NewScreenWithHistory(8, 4, HistoryConfig{MaxRows: 64, MaxCells: 4096})
	108	+	s.Write([]byte("1\r\n2\r\n3\r\n4\r\n5\r\nabcdefghij"))
	109	+
	110	+	s.Resize(16, 4)
	111	+
	112	+	require.Equal(t, "abcdefghij ", lineText(s, 2), "the wrapped line must rejoin on one row")
	113	+	require.Equal(t, LineBound{End: 10}, s.LineBounds()[2])
	114	+	require.Equal(t, " ", lineText(s, 3), "its continuation row must be consumed by the rejoin")
	115	+	}
	116	+
	117	+	// A wrap that scrolls the primary screen must hand history the same soft link
	118	+	// it leaves on the grid, so a line wrapped at the bottom of a full pane still
	119	+	// rejoins after it scrolls away.
	120	+	func TestSoftLinkFromABottomRowWrapReachesHistory(t *testing.T) {
	121	+	const (
	122	+	width = 8
	123	+	height = 4
	124	+	)
	125	+	s := NewScreenWithHistory(width, height, HistoryConfig{MaxRows: 64, MaxCells: 4096})
	126	+	// Wrap out of the bottom row, then push both physical rows into history.
	127	+	s.Write([]byte("1\r\n2\r\n3\r\n4\r\n5\r\nabcdefghij\r\nx\r\ny\r\nz\r\n"))
	128	+
	129	+	view := s.History().SealAndView()
	130	+	require.Equal(t, 7, view.Len(), "history row count")
	131	+	require.Equal(t, "abcdefgh", rowString(view.BorrowedRow(5)))
	132	+	require.Equal(t, LineBound{End: 8, Soft: true}, view.Bound(5), "the wrapped row must reach history soft")
	133	+	require.Equal(t, "ij ", rowString(view.BorrowedRow(6)))
	134	+	require.Equal(t, LineBound{End: 2, Soft: false}, view.Bound(6), "its continuation is a hard line end")
	135	+	}

@@ -0,0 +1,135 @@
	1	+	package vt
	2	+
	3	+	import (
	4	+	"testing"
	5	+
	6	+	"github.com/stretchr/testify/require"
	7	+	)
	8	+
	9	+	// A deferred wrap out of the last row of a scroll region scrolls that region,
	10	+	// and buffer.scrollUp deliberately severs the link on the last moved row. That
	11	+	// sever is right for a linefeed and wrong when the wrap itself caused the
	12	+	// scroll, so these cases pin both sides of the distinction.
	13	+	func TestSoftLinkSurvivesTheScrollItCaused(t *testing.T) {
	14	+	const width = 8
	15	+	cases := []struct {
	16	+	name string
	17	+	height int
	18	+	input string
	19	+	// wantBounds is indexed like the live grid.
	20	+	wantBounds map[int]LineBound
	21	+	// wantRows pins the grid text so a bound is never read off a row that
	22	+	// holds something other than what the case describes.
	23	+	wantRows map[int]string
	24	+	}{
	25	+	{
	26	+	// The bug: the cursor sits on the last row, as it does in any full
	27	+	// shell session, so the wrap scrolls and the link must survive.
	28	+	name: "narrow wrap out of the bottom row keeps the link",
	29	+	height: 4,
	30	+	input: "1\r\n2\r\n3\r\n4\r\n5\r\nabcdefghij",
	31	+	wantBounds: map[int]LineBound{
	32	+	2: {End: 8, Soft: true},
	33	+	3: {End: 2},
	34	+	},
	35	+	wantRows: map[int]string{2: "abcdefgh", 3: "ij "},
	36	+	},
	37	+	{
	38	+	// Same shape at the wide-rune wrap site: the wide rune does not fit
	39	+	// in the last column, so the writer abandons that cell and wraps.
	40	+	name: "wide-rune wrap out of the bottom row keeps the link",
	41	+	height: 4,
	42	+	input: "1\r\n2\r\n3\r\nabcdefg漢",
	43	+	wantBounds: map[int]LineBound{
	44	+	2: {End: 7, Soft: true},
	45	+	3: {End: 2},
	46	+	},
	47	+	wantRows: map[int]string{2: "abcdefg "},
	48	+	},
	49	+	{
	50	+	// The sever that must survive: a full-width row ending in an explicit
	51	+	// newline is a hard line that merely happens to fill the grid.
	52	+	name: "explicit newline after a full-width bottom row severs",
	53	+	height: 4,
	54	+	input: "1\r\n2\r\n3\r\nabcdefgh\r\n",
	55	+	wantBounds: map[int]LineBound{
	56	+	2: {End: 8, Soft: false},
	57	+	},
	58	+	wantRows: map[int]string{2: "abcdefgh"},
	59	+	},
	60	+	{
	61	+	// A DECSTBM region scroll is a local mutation, not a wrap: it must
	62	+	// still sever at the row above the region (top-1), whose
	63	+	// continuation the scroll just destroyed, and at the last moved row.
	64	+	name: "DECSTBM region scroll severs at both edges",
	65	+	height: 4,
	66	+	input: "abcdefghij" + "\x1b[2;3r" + "\x1b[3;1H" + "\n",
	67	+	wantBounds: map[int]LineBound{
	68	+	0: {End: 8, Soft: false},
	69	+	1: {End: 0, Soft: false},
	70	+	},
	71	+	wantRows: map[int]string{0: "abcdefgh"},
	72	+	},
	73	+	{
	74	+	// A region exactly one row tall blanks the wrapped row in place
	75	+	// instead of moving it up, so no surviving grid row may be linked.
	76	+	// setScrollRegion rejects top >= bottom, so the only reachable
	77	+	// one-row region is a one-row screen.
	78	+	name: "wrap on a one-row screen links nothing on the grid",
	79	+	height: 1,
	80	+	input: "abcdefghij",
	81	+	wantBounds: map[int]LineBound{
	82	+	0: {End: 2, Soft: false},
	83	+	},
	84	+	wantRows: map[int]string{0: "ij "},
	85	+	},
	86	+	}
	87	+	for _, tc := range cases {
	88	+	t.Run(tc.name, func(t *testing.T) {
	89	+	s := NewScreenWithHistory(width, tc.height, HistoryConfig{MaxRows: 64, MaxCells: 4096})
	90	+	s.Write([]byte(tc.input))
	91	+
	92	+	bounds := s.LineBounds()
	93	+	for y, want := range tc.wantBounds {
	94	+	require.Equal(t, want, bounds[y], "LineBounds()[%d]", y)
	95	+	}
	96	+	for y, want := range tc.wantRows {
	97	+	require.Equal(t, want, lineText(s, y), "row %d text", y)
	98	+	}
	99	+	})
	100	+	}
	101	+	}
	102	+
	103	+	// The soft link also drives reflow, so restoring it fixes resize too: a line
	104	+	// wrapped out of the bottom row used to be two unrelated physical rows, and
	105	+	// widening the pane left it split instead of rejoining it.
	106	+	func TestResizeRejoinsALineWrappedOutOfTheBottomRow(t *testing.T) {
	107	+	s := NewScreenWithHistory(8, 4, HistoryConfig{MaxRows: 64, MaxCells: 4096})
	108	+	s.Write([]byte("1\r\n2\r\n3\r\n4\r\n5\r\nabcdefghij"))
	109	+
	110	+	s.Resize(16, 4)
	111	+
	112	+	require.Equal(t, "abcdefghij ", lineText(s, 2), "the wrapped line must rejoin on one row")
	113	+	require.Equal(t, LineBound{End: 10}, s.LineBounds()[2])
	114	+	require.Equal(t, " ", lineText(s, 3), "its continuation row must be consumed by the rejoin")
	115	+	}
	116	+
	117	+	// A wrap that scrolls the primary screen must hand history the same soft link
	118	+	// it leaves on the grid, so a line wrapped at the bottom of a full pane still
	119	+	// rejoins after it scrolls away.
	120	+	func TestSoftLinkFromABottomRowWrapReachesHistory(t *testing.T) {
	121	+	const (
	122	+	width = 8
	123	+	height = 4
	124	+	)
	125	+	s := NewScreenWithHistory(width, height, HistoryConfig{MaxRows: 64, MaxCells: 4096})
	126	+	// Wrap out of the bottom row, then push both physical rows into history.
	127	+	s.Write([]byte("1\r\n2\r\n3\r\n4\r\n5\r\nabcdefghij\r\nx\r\ny\r\nz\r\n"))
	128	+
	129	+	view := s.History().SealAndView()
	130	+	require.Equal(t, 7, view.Len(), "history row count")
	131	+	require.Equal(t, "abcdefgh", rowString(view.BorrowedRow(5)))
	132	+	require.Equal(t, LineBound{End: 8, Soft: true}, view.Bound(5), "the wrapped row must reach history soft")
	133	+	require.Equal(t, "ij ", rowString(view.BorrowedRow(6)))
	134	+	require.Equal(t, LineBound{End: 2, Soft: false}, view.Bound(6), "its continuation is a hard line end")
	135	+	}