From 60debda32674f6be66b042e9419084cf4f2e2697 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 8 Jun 2026 09:53:40 +0200 Subject: [PATCH] =?UTF-8?q?Unify=20the=20view-line=E2=86=92buffer-line=20r?= =?UTF-8?q?eaders=20onto=20one=20helper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HyperLinkInLine, DiffLineMetadataInLine and BufferLineForViewLine each repeated the same preamble: take the lock, refresh the view lines, range- check the view line, and guard against a stale viewLines entry pointing past a shrunk buffer. They all need that mapping to stay consistent with the buffer they then read, so the logic belongs in one place. Extract it into bufferLineForViewLine and have all three call it. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gocui/view.go | 44 ++++++++++++++++---------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 1e3a584bc..cbfbfb8cb 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -1655,22 +1655,11 @@ func (v *View) Word(x, y int) (string, bool) { } func (v *View) HyperLinkInLine(y int, urlScheme string) (string, bool) { - // Take the lock so we don't race a concurrent re-render that is rebuilding the - // buffer. v.writeMutex.Lock() defer v.writeMutex.Unlock() - v.refreshViewLinesIfNeeded() - - if y < 0 || y >= len(v.viewLines) { - return "", false - } - - // refreshViewLinesIfNeeded overwrites viewLines in place without truncating, - // so while a shorter re-render is loading, the tail of viewLines can still - // hold stale entries pointing past the (shrunk) v.lines. Guard against that. - linesY := v.viewLines[y].linesY - if linesY >= len(v.lines) { + linesY, ok := v.bufferLineForViewLine(y) + if !ok { return "", false } @@ -1688,22 +1677,11 @@ func (v *View) HyperLinkInLine(y int, urlScheme string) (string, bool) { // single-column case every cell of the line carries the same payload, so the // first non-empty one is the answer. See diff-line-metadata-notes.md. func (v *View) DiffLineMetadataInLine(y int) (string, bool) { - // Take the lock so we don't race a concurrent re-render that is rebuilding the - // buffer. v.writeMutex.Lock() defer v.writeMutex.Unlock() - v.refreshViewLinesIfNeeded() - - if y < 0 || y >= len(v.viewLines) { - return "", false - } - - // refreshViewLinesIfNeeded overwrites viewLines in place without truncating, - // so while a shorter re-render is loading, the tail of viewLines can still - // hold stale entries pointing past the (shrunk) v.lines. Guard against that. - linesY := v.viewLines[y].linesY - if linesY >= len(v.lines) { + linesY, ok := v.bufferLineForViewLine(y) + if !ok { return "", false } @@ -1721,11 +1699,21 @@ func (v *View) DiffLineMetadataInLine(y int) (string, bool) { // returned by BufferLines). Several view lines can map to the same buffer line // when wrapping is on. Returns false if the view line is out of range. func (v *View) BufferLineForViewLine(y int) (int, bool) { - // Take the lock so we don't race a concurrent re-render that is rebuilding - // the buffer. v.writeMutex.Lock() defer v.writeMutex.Unlock() + return v.bufferLineForViewLine(y) +} + +// bufferLineForViewLine maps a (wrapped) view line index to the index of the +// corresponding line in the unwrapped internal buffer (v.lines). It is the +// shared core of the public readers that look up information about the buffer +// line under a given view line (its buffer index, its hyperlink, its diff +// metadata); they all need the same view-line→buffer-line mapping to stay +// consistent with the buffer they then read. The caller must hold writeMutex, +// so that the mapping and the subsequent read of v.lines see the same buffer +// even if a concurrent re-render is rebuilding it. +func (v *View) bufferLineForViewLine(y int) (int, bool) { v.refreshViewLinesIfNeeded() if y < 0 || y >= len(v.viewLines) {