diff --git a/CHANGELOG.md b/CHANGELOG.md index 7366aae3..34dda100 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ Every release is cut from an annotated tag whose message _is_ the release note, [GitHub releases](https://github.com/cmj0121/baton/releases) always carry the full story — the upgrade notes, the caveats, and why each change exists. This file is the index. +## Unreleased + +- **A zoom no longer tears when a program sets a scroll region taller than the screen.** A program that still + believes an older, taller PTY size — every zoom passes through that window between its attach and its resize — + asked the emulator for margins past its last row, and the next delete-line panicked inside the emulator. The cockpit + recovered by dropping the rest of that chunk, so the panel showed a torn screen until the program repainted; the + daemon log carried the trace as `recovered an emulator write panic`. The margins are now clamped on the way in, + as a real terminal does (#139). +- **The terminal's cursor follows the zoomed program.** The zoom painted a reverse-video cell where the program's + cursor was and left the real cursor at the end of the footer, which is where an input method draws the text it is + composing: CJK marked text there wrapped the line and scrolled the screen out from under the renderer. The zoom now + hands the terminal the program's cursor, so composition happens at the prompt, as in a plain terminal (#140). +- **Wide glyphs in scrollback are clipped by cell.** A scrollback line with CJK text captured at a wider size was + clipped by rune count and could still overflow its tile by up to twice its width. + ## [v2.4.2](https://github.com/cmj0121/baton/releases/tag/v2.4.2) — SYS follows the session you are in 2026-09-23 diff --git a/internal/tui/clipcells_test.go b/internal/tui/clipcells_test.go new file mode 100644 index 00000000..bcce64d5 --- /dev/null +++ b/internal/tui/clipcells_test.go @@ -0,0 +1,33 @@ +package tui + +import ( + "testing" + + "github.com/charmbracelet/x/ansi" +) + +// TestClipVisibleCounts cells, not runes: a scrollback line of CJK captured at a +// wider size must come back no wider than its tile, or the terminal wraps it and +// every row below shifts (#140). +func TestClipVisibleCounts(t *testing.T) { + cases := []struct { + in string + width int + want string + }{ + {"中文中文中文", 6, "中文中\x1b[0m"}, + {"中文中文中文", 5, "中文\x1b[0m"}, // a wide glyph that would straddle the cut is dropped + {"ab中c", 4, "ab中\x1b[0m"}, + {"\x1b[31m中文\x1b[0m中", 4, "\x1b[31m中文\x1b[0m\x1b[0m"}, + {"abc", 5, "abc"}, // fits: no clip, no reset + } + for _, c := range cases { + got := clipVisible(c.in, c.width) + if got != c.want { + t.Errorf("clipVisible(%q, %d) = %q, want %q", c.in, c.width, got, c.want) + } + if w := ansi.StringWidth(got); w > c.width { + t.Errorf("clipVisible(%q, %d) is %d cells wide", c.in, c.width, w) + } + } +} diff --git a/internal/tui/margins_test.go b/internal/tui/margins_test.go new file mode 100644 index 00000000..19a7ed30 --- /dev/null +++ b/internal/tui/margins_test.go @@ -0,0 +1,42 @@ +package tui + +import ( + "strings" + "testing" + + vt "github.com/charmbracelet/x/vt" + + "github.com/cmj0121/baton/internal/vtirm" +) + +// TestWriteEmuSurvivesTallScrollRegion pins #139 at the choke point: a program that +// still believes a taller PTY sets a scroll region past this emulator's last row, +// then deletes a line inside it. Unclamped, the emulator indexed past its buffer, +// the write panicked, and every byte after the delete in that chunk was lost — +// here, the prompt. Clamped, the chunk lands whole. +func TestWriteEmuSurvivesTallScrollRegion(t *testing.T) { + for _, op := range []string{"\x1b[M", "\x1b[L", "\x1bM", "\x1b[2S", "\x1b[2T"} { + emu := vt.NewSafeEmulator(80, 24) + irm := &vtirm.Filter{} + writeEmu(emu, irm, []byte("\x1b[1;40r\x1b[5;1Hold")) + writeEmu(emu, irm, []byte("\x1b[5;1H"+op+"\x1b[24;1H$ prompt")) + if !strings.Contains(emu.Render(), "$ prompt") { + t.Errorf("%q after a too-tall margin: the rest of the chunk was dropped", op) + } + } +} + +// TestWriteEmuKeepsScrollRegionInside proves the clamp is a no-op for a region that +// fits: the margins the program asked for are the ones the emulator scrolls within. +func TestWriteEmuKeepsScrollRegionInside(t *testing.T) { + emu := vt.NewSafeEmulator(80, 24) + irm := &vtirm.Filter{} + writeEmu(emu, irm, []byte("\x1b[1;1Hkeep\x1b[2;24r\x1b[24;1Hbottom\n")) + rows := strings.Split(emu.Render(), "\n") + if !strings.HasPrefix(rows[0], "keep") { + t.Fatalf("row 0 = %q: the line above the region must not scroll", rows[0]) + } + if !strings.HasPrefix(rows[22], "bottom") { + t.Fatalf("row 22 = %q: the region should have scrolled its bottom line up", rows[22]) + } +} diff --git a/internal/tui/testdata/frames/zoom-scrolling.bin b/internal/tui/testdata/frames/zoom-scrolling.bin index d0831cdb..a34b74e0 100644 --- a/internal/tui/testdata/frames/zoom-scrolling.bin +++ b/internal/tui/testdata/frames/zoom-scrolling.bin @@ -1,7 +1,7 @@ $ make test ok baton/internal/tui 0.42s PASS -$   +$ diff --git a/internal/tui/testdata/frames/zoom.bin b/internal/tui/testdata/frames/zoom.bin index ba7f2cf5..6040f101 100644 --- a/internal/tui/testdata/frames/zoom.bin +++ b/internal/tui/testdata/frames/zoom.bin @@ -1,7 +1,7 @@ $ make test ok baton/internal/tui 0.42s PASS -$   +$ diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 641741e3..d9e12e85 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -4076,6 +4076,9 @@ func (m model) cols() int { func (m model) View() tea.View { v := tea.NewView(m.frame()) v.AltScreen = true + if x, y, ok := m.zoomCursor(); ok { + v.Cursor = tea.NewCursor(x, y) + } if m.mouseEnabled { // Cell motion reports clicks and the wheel without the noise of every // pointer move, which is all the cockpit's wheel handling needs. @@ -4229,20 +4232,38 @@ func (m model) zoomView() string { lines := make([]string, rows) if m.emu != nil { lines = m.selectWindow(m.emu, m.width, rows, m.scrollOff) - // Draw the cursor only on the live bottom (a scrolled-back view is history), - // and only when the program has not hidden it — so a full-screen reader that - // turns the cursor off does not show a phantom block. - if m.scrollOff == 0 && !m.cursorHiddenNow() { - cur := m.emu.CursorPosition() - if cur.Y >= 0 && cur.Y < len(lines) { - lines[cur.Y] = overlayCursor(lines[cur.Y], cur.X) - } - } } footer := m.zoomFooter() return strings.Join(lines, "\n") + "\n" + footer } +// zoomCursor is where the terminal's own cursor belongs while a zoom is on screen: +// on the cell the zoomed program's cursor is on, so the terminal composes there. +// The zoom used to paint a reverse-video cell instead and leave the real cursor +// wherever the renderer last wrote. That is where an input method draws the text +// it is composing — the end of the footer — and CJK marked text there wraps the +// line and scrolls the alternate screen out from under the diff renderer (#140). +// A real terminal composes at the program's cursor; now so does the cockpit. +// +// No cursor — ok false, and the terminal keeps it hidden — outside a zoom, on a +// cockpit too small to draw one, while scrolled back into history, while a copy +// selection is being marked, while a text-input overlay (the search prompt) holds +// the keyboard, or while the program has hidden it (DECTCEM), so a full-screen +// reader that turns the cursor off does not show a phantom block. +func (m model) zoomCursor() (x, y int, ok bool) { + if m.mode != modeZoom || m.emu == nil || m.width < minWidth || m.height < minHeight { + return 0, 0, false + } + if m.scrollOff != 0 || m.copySelecting || m.input != inputNone || m.cursorHiddenNow() { + return 0, 0, false + } + cur := m.emu.CursorPosition() + if cur.Y < 0 || cur.Y >= m.zoomRows() || cur.X < 0 || cur.X >= m.width { + return 0, 0, false + } + return cur.X, cur.Y, true +} + // dashboardView renders the status summary strip above the fleet. A small fleet // shows as a card grid; once it grows past treeThreshold it switches to a // space-efficient tree + preview split. diff --git a/internal/tui/zoom.go b/internal/tui/zoom.go index add736ca..f56ec8c5 100644 --- a/internal/tui/zoom.go +++ b/internal/tui/zoom.go @@ -35,12 +35,21 @@ import ( // bursts into the ICH the emulator implements, so a character typed into the middle of a // line pushes its neighbours right instead of landing on top of one. It carries state // across chunks, so each emulator needs its own; a nil filter leaves the stream alone. +// +// The filter is also told the emulator's height on every write, so a scroll region a +// program sets for a taller screen than this one — the window between a zoom's attach and +// its resize — is clamped before the emulator stores it. Left as asked, the next delete-line +// indexes past the buffer; the recover below then drops the rest of the chunk and the panel +// shows a torn screen until the program repaints (#139). func writeEmu(emu *vt.SafeEmulator, irm *vtirm.Filter, data []byte) { defer func() { if r := recover(); r != nil { log.Error().Interface("panic", r).Bytes("stack", debug.Stack()).Msg("recovered an emulator write panic") } }() + if irm != nil { + irm.Rows = emu.Height() + } _, _ = emu.Write(irm.Rewrite(vtquery.Strip(data))) } @@ -195,6 +204,10 @@ func emuWindow(emu *vt.SafeEmulator, cols, rows, off int) []string { // copying escape sequences verbatim since they cost no columns. A scrollback line // captured at a wider size is thus trimmed to fit, and a trailing reset is added // when the clip lands mid-styling so a colour cannot bleed past the cut. +// +// Columns are display cells, not runes: a wide (CJK) glyph takes two, and one that +// would straddle the cut is dropped rather than let the line run a cell over and +// wrap in its tile (#140). func clipVisible(s string, width int) string { if width < 1 { return "" @@ -208,13 +221,14 @@ func clipVisible(s string, width int) string { i += n continue } - if vis >= width { + r, size := utf8.DecodeRuneInString(s[i:]) + w := cellWidth(r) + if vis+w > width { clipped = true break } - _, size := utf8.DecodeRuneInString(s[i:]) + vis += w out.WriteString(s[i : i+size]) - vis++ i += size } if clipped { diff --git a/internal/tui/zoom_test.go b/internal/tui/zoom_test.go index e8cb4fa1..2a0d453c 100644 --- a/internal/tui/zoom_test.go +++ b/internal/tui/zoom_test.go @@ -214,18 +214,21 @@ func TestZoomTracksCursorVisibility(t *testing.T) { } } -// TestZoomViewHidesCursor proves the zoom view draws the cursor only while the -// program keeps it visible. +// TestZoomViewHidesCursor proves the zoom hands the terminal the program's cursor +// only while the program keeps it visible — and never paints one of its own. func TestZoomViewHidesCursor(t *testing.T) { - emu := vt.NewSafeEmulator(20, 5) + emu := vt.NewSafeEmulator(80, 23) hidden := false - m := model{emu: emu, mode: modeZoom, width: 20, height: 6, cursorHidden: &hidden} - if !strings.Contains(m.zoomView(), "\x1b[7m") { - t.Fatal("a visible cursor should be drawn") + m := model{emu: emu, mode: modeZoom, width: 80, height: 24, cursorHidden: &hidden} + if m.View().Cursor == nil { + t.Fatal("a visible cursor should be handed to the terminal") } - hidden = true if strings.Contains(m.zoomView(), "\x1b[7m") { - t.Fatal("a hidden cursor should not be drawn") + t.Fatal("the zoom should not paint a reverse-video cursor of its own") + } + hidden = true + if m.View().Cursor != nil { + t.Fatal("a hidden cursor should not be handed to the terminal") } } diff --git a/internal/tui/zoomcursor_test.go b/internal/tui/zoomcursor_test.go new file mode 100644 index 00000000..33b61323 --- /dev/null +++ b/internal/tui/zoomcursor_test.go @@ -0,0 +1,49 @@ +package tui + +import ( + "testing" + + vt "github.com/charmbracelet/x/vt" +) + +// TestZoomCursorFollowsProgram proves the terminal cursor the zoom reports is the +// zoomed program's own, cell for cell — including after a wide glyph, where a cursor +// counted in runes would land a column short (#140). +func TestZoomCursorFollowsProgram(t *testing.T) { + emu := vt.NewSafeEmulator(80, 23) + hidden := false + m := model{emu: emu, mode: modeZoom, width: 80, height: 24, cursorHidden: &hidden} + _, _ = emu.Write([]byte("\x1b[3;1H❯ hello 中文")) + cur := m.View().Cursor + if cur == nil { + t.Fatal("the zoom should report a cursor") + } + if cur.X != 12 || cur.Y != 2 { + t.Fatalf("cursor = (%d,%d), want (12,2): two wide glyphs take four cells", cur.X, cur.Y) + } +} + +// TestZoomCursorWithheld lists the states in which the zoom must not report a +// cursor: history (scrolled back), a copy selection, a text-input overlay that +// holds the keyboard, the dashboard, a cockpit too small to draw a zoom at all. +func TestZoomCursorWithheld(t *testing.T) { + emu := vt.NewSafeEmulator(80, 23) + hidden := false + base := model{emu: emu, mode: modeZoom, width: 80, height: 24, cursorHidden: &hidden} + cases := map[string]func(m model) model{ + "scrolled back": func(m model) model { m.scrollOff = 3; return m }, + "copy selecting": func(m model) model { m.copySelecting = true; return m }, + "typing a search": func(m model) model { m.input = inputSearch; return m }, + "not zoomed": func(m model) model { m.mode = modeDashboard; return m }, + "too small": func(m model) model { m.width, m.height = 2, 2; return m }, + "no emulator yet": func(m model) model { m.emu = nil; return m }, + } + if base.View().Cursor == nil { + t.Fatal("the base zoom should report a cursor") + } + for name, tweak := range cases { + if tweak(base).View().Cursor != nil { + t.Errorf("%s: the zoom should withhold the cursor", name) + } + } +} diff --git a/internal/vtirm/margins_test.go b/internal/vtirm/margins_test.go new file mode 100644 index 00000000..d2ed8219 --- /dev/null +++ b/internal/vtirm/margins_test.go @@ -0,0 +1,52 @@ +package vtirm + +import ( + "bytes" + "testing" +) + +// TestClampMargins tables what a DECSTBM becomes on a 24-row emulator. +func TestClampMargins(t *testing.T) { + cases := []struct { + in, want string + }{ + {"\x1b[1;24r", "\x1b[1;24r"}, // fits: untouched + {"\x1b[r", "\x1b[r"}, // the whole screen: untouched + {"\x1b[1;40r", "\x1b[1;24r"}, // too tall: bottom pinned to the last row + {"\x1b[10;99r", "\x1b[10;24r"}, // top kept, bottom pinned + {"\x1b[30;40r", ""}, // wholly off screen: dropped, as xterm ignores it + {"\x1b[24;40r", ""}, // one row after the clamp: dropped + {"\x1b[0;40r", "\x1b[1;24r"}, // zero means default + {"\x1b[?1;40r", "\x1b[?1;40r"}, // a private sequence is not DECSTBM + {"\x1b[1;2;3r", "\x1b[1;2;3r"}, // too many parameters: not ours to judge + {"\x1b[1;40m", "\x1b[1;40m"}, // not a margin at all + } + for _, c := range cases { + f := &Filter{Rows: 24} + if got := f.Rewrite([]byte(c.in)); string(got) != c.want { + t.Errorf("Rewrite(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// TestClampMarginsNeedsRows proves a Filter that was never told a height leaves +// margins alone rather than clamping to nothing. +func TestClampMarginsNeedsRows(t *testing.T) { + f := &Filter{} + in := []byte("\x1b[1;40r") + if got := f.Rewrite(in); !bytes.Equal(got, in) { + t.Fatalf("Rewrite with Rows=0 = %q, want %q untouched", got, in) + } +} + +// TestClampMarginsAcrossChunks proves a DECSTBM cut by a chunk boundary is still +// clamped once its second half arrives, the same hold that keeps IRM honest. +func TestClampMarginsAcrossChunks(t *testing.T) { + f := &Filter{Rows: 24} + var out []byte + out = append(out, f.Rewrite([]byte("\x1b[1;4"))...) + out = append(out, f.Rewrite([]byte("0r$ "))...) + if want := "\x1b[1;24r$ "; string(out) != want { + t.Fatalf("split DECSTBM = %q, want %q", out, want) + } +} diff --git a/internal/vtirm/vtirm.go b/internal/vtirm/vtirm.go index 4dc6f6c9..7fc83919 100644 --- a/internal/vtirm/vtirm.go +++ b/internal/vtirm/vtirm.go @@ -22,6 +22,15 @@ // Mode 4 itself is taken out of the stream, so the emulator never sees it. That keeps the // rewrite honest in one direction that matters: should a future x/vt learn IRM natively, // it cannot double up on the ICH this package already emits. +// +// The same pass clamps DECSTBM (CSI t ; b r) to the emulator's height. The emulator stores +// whatever margins a program asks for, and its line operations index the buffer through +// them: a bottom margin past the last row makes the next DL / IL / RI / SU / SD index past +// the buffer and panic, which the cockpit recovers from by dropping the rest of that chunk +// — the screen then diverges until the program's next full repaint. A program asks for such +// a margin whenever it still believes an older, taller PTY size, which every zoom passes +// through between its attach and its resize (github.com/cmj0121/baton#139). A real terminal +// clamps the request; the Filter does it on the way in. package vtirm import ( @@ -54,6 +63,10 @@ const ( // a caller that feeds an emulator it does not own — a test, a one-shot render — needs no // filter at all. type Filter struct { + // Rows is the emulator's height, the ceiling a DECSTBM bottom margin is clamped to. + // Zero (a caller that never set it) leaves margins untouched. + Rows int + insert bool // IRM is set: printed cells push the rest of the line right state byte // ansi decoder state, carried because a chunk may split a sequence held []byte // a CSI sequence split across chunks, held until it completes @@ -145,12 +158,73 @@ func (f *Filter) Rewrite(b []byte) []byte { out = append(out, residual...) continue } + if clamped, ok := clampMargins(seq, f.Rows); ok { + out = append(out, clamped...) + continue + } out = append(out, seq...) } flush() return out } +// clampMargins rewrites a DECSTBM whose margins reach past the emulator's rows to the +// margins a real terminal would keep: a bottom past the last row becomes the last row, +// and a request whose top is not above its (clamped) bottom is dropped whole, which is +// how xterm treats it. A margin already inside the screen, a sequence that is not a +// plain numeric DECSTBM, or a Filter with no Rows are passed through untouched, ok false. +func clampMargins(seq []byte, rows int) (clamped []byte, ok bool) { + if rows <= 0 || len(seq) < 3 || seq[0] != ansi.ESC || seq[1] != '[' || seq[len(seq)-1] != 'r' { + return nil, false + } + params := seq[2 : len(seq)-1] + for _, c := range params { + if (c < '0' || c > '9') && c != ';' { + return nil, false + } + } + fields := bytes.Split(params, []byte{';'}) + if len(fields) > 2 { + return nil, false + } + top, bottom := 1, rows + if len(fields) >= 1 && len(fields[0]) > 0 { + top = margin(fields[0], 1) + } + if len(fields) == 2 && len(fields[1]) > 0 { + bottom = margin(fields[1], rows) + } + if bottom <= rows && top < bottom { + return nil, false // already what the emulator can hold + } + bottom = min(bottom, rows) + if top >= bottom { + return nil, true // xterm ignores a region that is not at least two rows + } + clamped = append(clamped, ansi.ESC, '[') + clamped = strconv.AppendInt(clamped, int64(top), 10) + clamped = append(clamped, ';') + clamped = strconv.AppendInt(clamped, int64(bottom), 10) + return append(clamped, 'r'), true +} + +// margin reads one DECSTBM parameter. A zero means "default" in DECSTBM, and a run of +// digits long enough to overflow is not a row anyone meant, so both fall back to the +// default the caller supplied. +func margin(b []byte, def int) int { + if len(b) > 6 { + return def + } + n := 0 + for _, c := range b { + n = n*10 + int(c-'0') + } + if n == 0 { + return def + } + return n +} + // holdable reports whether the half-read bytes are still worth waiting for: they can only // become a CSI sequence, and they have not outgrown the cap. func (f *Filter) holdable() bool {