From f6eaed8cd4f2298fad4126ff56df77b71daed730 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 8 Jul 2026 13:00:49 +0200 Subject: [PATCH] Snapshot the view width for command-task rendering on the UI thread A command task streams its output into a view from its own goroutine. To track soft-wraps (so cursor-positioning escapes from a pager land on the right line) the write path read the view's live InnerWidth, and the pty setup read its InnerSize -- both off the UI thread, racing the UI thread mutating the view's dimensions during layout. Capture the width on the UI thread instead and hand it to the task: the escape interpreter keeps a screenColMax it reads from, seeded in NewView and refreshed per render via View.SetContentWidth (called from newCmdTask/newPtyTask before the task's goroutine starts), and the pty size is computed in the after-layout callback rather than in the task's start func. The view's dimensions stay UI-thread-only; the task uses the snapshot rather than reading them live. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gocui/escape.go | 14 +++++++++++--- pkg/gocui/view.go | 13 ++++++++++++- pkg/gui/pty.go | 11 ++++++++++- pkg/gui/tasks_adapter.go | 9 +++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index ad862a596..7f3de9e6e 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -34,6 +34,14 @@ type escapeInterpreter struct { // modelled — we don't track the col argument of CUPs, and most // pager-style emitters use col 1 anyway. screenRow, screenCol int + + // The screen width that soft-wraps are counted against (see + // notifyCellsWritten). It's a snapshot of the view's InnerWidth taken on + // the UI thread (in NewView, and refreshed per render via + // View.SetContentWidth), rather than read live from the view's dimensions: + // a view's output is written from a task goroutine, and reading the live + // dimensions there would race the UI thread updating them during layout. + screenColMax int } type ( @@ -175,8 +183,8 @@ func (ei *escapeInterpreter) notifyColumnReset() { // columns; if that crosses the right edge of a `screenColMax`-wide pty // screen, the corresponding number of soft-wraps are added to screenRow // so subsequent CUPs land on the right line. -func (ei *escapeInterpreter) notifyCellsWritten(width, screenColMax int) { - if screenColMax <= 0 { +func (ei *escapeInterpreter) notifyCellsWritten(width int) { + if ei.screenColMax <= 0 { return } // One column at a time: matches ConPTY's "pending wrap" semantics @@ -185,7 +193,7 @@ func (ei *escapeInterpreter) notifyCellsWritten(width, screenColMax int) { // columns rather than doing the math in one shot so wide cells on a // row boundary still wrap cleanly. for range width { - if ei.screenCol > screenColMax { + if ei.screenCol > ei.screenColMax { ei.screenRow++ ei.screenCol = 1 } diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index e4c5a5f98..ae8aea880 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -536,9 +536,20 @@ func NewView(name string, x0, y0, x1, y1 int, mode OutputMode) *View { v.SelFgColor, v.SelBgColor = ColorDefault, ColorDefault v.InactiveViewSelBgColor = ColorDefault v.TitleColor, v.FrameColor = ColorDefault, ColorDefault + v.ei.screenColMax = v.InnerWidth() return v } +// SetContentWidth tells the view the screen width that content written to it +// should count soft-wraps against (see escapeInterpreter.notifyCellsWritten). +// Callers pass the view's InnerWidth; it's a separate call, made on the UI +// thread when a render starts, so that the task goroutine that streams the +// content can consult this snapshot instead of reading the view's live +// dimensions (which the UI thread mutates during layout). +func (v *View) SetContentWidth(width int) { + v.ei.screenColMax = width +} + // Dimensions returns the dimensions of the View func (v *View) Dimensions() (int, int, int, int) { return v.x0, v.y0, v.x1, v.y1 @@ -907,7 +918,7 @@ func (v *View) write(p []byte) { for _, c := range cells { totalWidth += c.width } - v.ei.notifyCellsWritten(totalWidth, v.InnerWidth()) + v.ei.notifyCellsWritten(totalWidth) } } } diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index 1a774fc3d..d4f739c6d 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -93,9 +93,18 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error manager := gui.getManager(view) + // Size the pty from the view's dimensions here, on the UI thread; the + // start func below runs on the task's goroutine, which must not read the + // view's live dimensions while the UI thread is laying it out. + cols, rows := gui.desiredPtySize(view) + var p oscommands.Pty start := func() (tasks.Cmd, io.Reader) { - cols, rows := gui.desiredPtySize(view) + // The pty (and pager) wrap to this width; apply it here, on the + // task's goroutine once the previous task has stopped, so it doesn't + // race that task's writes (see View.SetContentWidth). + view.SetContentWidth(width) + sp, err := oscommands.StartPty(cmd, cols, rows) if err != nil { gui.c.Log.Error(err) diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 9f9488048..27aacf58b 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -18,8 +18,17 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error manager := gui.getManager(view) + // Snapshot the view width here, on the UI thread, so the task goroutine + // doesn't read the view's live dimensions while it streams output. It's + // applied inside start() below rather than now, because start() runs once + // the previous task has stopped -- applying it here would race that task's + // still-running writes (see View.SetContentWidth). + contentWidth := view.InnerWidth() + var r io.ReadCloser start := func() (tasks.Cmd, io.Reader) { + view.SetContentWidth(contentWidth) + var err error r, err = cmd.StdoutPipe() if err != nil {