Fix flicker, scroll glitches, and crashes in async diff rendering (#5938)

This is a preparation PR for the upcoming fold-staging-into-main-view
work; see the individual commit messages for details.

The most notable change is probably that we switch to a double-buffering
approach for flicker-free view updates; previously we would overwrite
the view from the top, and keep the existing viewlines below untouched
to update without flicker. This caused numerous problems though that
will become more painful when we start using the main view for more
operations (especially staging); telling whether the selected line still
belongs to the previous task or already to the new one is tricky.
Rendering into an offscreen buffer and swapping it in as soon as we have
enough to fill the screen makes this much easier.
This commit is contained in:
Stefan Haller 2026-08-15 15:52:42 +02:00 committed by GitHub
commit 6032225472
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 891 additions and 228 deletions

View file

@ -204,7 +204,8 @@ func (p *winPty) Close() error {
// slave closes on child exit, but ConPTY keeps the pipe alive until we call
// ClosePseudoConsole explicitly. Without doing that on child exit, the
// scanner in pkg/tasks.NewCmdTask would block forever on the next read and
// the post-content view never gets cleared (FlushStaleCells never fires).
// the render would never reach its end of input, so the new content would
// never be swapped in.
func startWaiter(proc *os.Process, p *winPty) func() error {
done := make(chan struct{})
var waitErr error

View file

@ -1276,7 +1276,7 @@ func calcScrollbarRune(
func calcRealScrollbarStartEnd(v *View) (bool, int, int) {
height := v.InnerHeight()
fullHeight := v.ViewLinesHeight() - v.scrollMargin()
fullHeight := v.scrollbarContentHeight() - v.scrollMargin()
if v.CanScrollPastBottom {
fullHeight += height
@ -1497,7 +1497,7 @@ func (g *Gui) drawSubtitle(v *View, fgColor, bgColor Attribute) error {
// drawListFooter draws the footer of a list view, showing something like '1 of 10'
func (g *Gui) drawListFooter(v *View, fgColor, bgColor Attribute) error {
if len(v.lines) == 0 {
if len(v.buf.lines) == 0 {
return nil
}
@ -1747,13 +1747,13 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
if newY < 0 {
newY = 0
newCy = -v.oy
} else if newY >= len(v.lines) {
newY = len(v.lines) - 1
} else if newY >= len(v.buf.lines) {
newY = len(v.buf.lines) - 1
newCy = newY - v.oy
}
visibleLineWidth := 0
for _, c := range v.lines[newY].cells {
for _, c := range v.buf.lines[newY].cells {
visibleLineWidth += c.width
}
if visibleLineWidth < newX {
@ -1763,10 +1763,8 @@ func (g *Gui) onKey(ev *GocuiEvent) error {
}
if ev.Key.KeyName() == MouseLeft && (ev.Key.Mod()&ModMotion) == 0 && !v.Editable && g.openHyperlink != nil {
if newY >= 0 && newY <= len(v.viewLines)-1 && newX >= 0 && newX <= len(v.viewLines[newY].line)-1 {
if link := v.viewLines[newY].line[newX].hyperlink; link != "" {
return g.openHyperlink(link, v.name)
}
if link := v.hyperlinkAt(newX, newY); link != "" {
return g.openHyperlink(link, v.name)
}
}

View file

@ -25,17 +25,51 @@ const (
RIGHT = 8 // view is overlapping at right edge
)
// viewBuffer holds a view's content as cells, together with the cursor and
// escape-sequence decoder state used to turn incoming bytes into those cells.
// A view normally has a single buffer (the one it displays), but bundling this
// state lets a re-render build a second, off-screen buffer and swap it in
// atomically once the new content is ready, so no reader ever sees a
// half-written buffer.
type viewBuffer struct {
// the view's content: one []cell per unwrapped line
lines []lineType
// write cursor into lines
wx, wy int
// decodes ESC sequences as bytes are written
ei *escapeInterpreter
// If the last character written was a newline, we don't write it but instead
// set pendingNewline to true. If more text is written, we write the newline
// then. This avoids an extra blank line at the end of the view.
pendingNewline bool
}
// A View is a window. It maintains its own internal buffer and cursor
// position.
type View struct {
name string
x0, y0, x1, y1 int // left top right bottom
ox, oy int // view offsets
cx, cy int // cursor position
rx, ry int // Read() offsets
wx, wy int // Write() offsets
lines []lineType // All the data
x0, y0, x1, y1 int // left top right bottom
ox, oy int // view offsets
cx, cy int // cursor position
rx, ry int // Read() offsets
outMode OutputMode
// buf bundles the view's cell buffer and the cursor / escape-parser state
// used to write into it (see the viewBuffer type). It is the buffer every
// reader sees.
buf *viewBuffer
// While non-nil, writes go here instead of buf, so an async re-render can
// build its new content without disturbing what readers (draw, clicks,
// scrolling, …) see. The task swaps it into buf once it has read enough to
// paint (SwapInOffscreenRender), so the displayed content jumps straight
// from the previous render to the new one with no half-written frame in
// between. nil during normal (non-async) writes.
offscreen *viewBuffer
// The y position of the first line of a range selection.
// This is not relative to the view's origin: it is relative to the first line
// of the view's content, so you can scroll the view and this value will remain
@ -74,17 +108,20 @@ type View struct {
// true and viewLines to nil
viewLines []viewLine
// If the last character written was a newline, we don't write it but
// instead set pendingNewline to true. If more text is written, we write the
// newline then. This is to avoid having an extra blank at the end of the view.
pendingNewline bool
// While a re-render is loading new content (see offscreen), the displayed
// buffer is only partially filled once we've swapped the off-screen render
// in: the task keeps appending lines after the first paint, up to the count
// needed for an accurate scrollbar. Sizing the scrollbar from that partial
// view-line count would make the thumb shrink and snap back as the rest
// streams in. So while a load is in progress we hold the scrollbar's height
// at this value — the height the view had when the load began — and let it
// grow only if the new content turns out taller. Zero means no load is in
// progress and the scrollbar tracks the content directly.
scrollbarHeightFloor int
// writeMutex protects locks the write process
writeMutex sync.Mutex
// ei is used to decode ESC sequences on Write
ei *escapeInterpreter
// Visible specifies whether the view is visible.
Visible bool
@ -402,7 +439,7 @@ func (v *View) FocusPoint(cx int, cy int, scrollIntoView bool) {
if scrollIntoView {
height := v.InnerHeight()
v.oy = calculateNewOrigin(cy, v.oy, lineCount, height)
v.SetOriginY(calculateNewOrigin(cy, v.oy, lineCount, height))
}
v.cx = cx
@ -461,7 +498,7 @@ type SearchPosition struct {
}
type viewLine struct {
linesX, linesY int // coordinates relative to v.lines
linesX, linesY int // coordinates relative to v.buf.lines
line []cell
// Colors used to extend the bg past this wrapped segment's content.
@ -470,7 +507,7 @@ type viewLine struct {
trailingFillAttributes *trailingFillAttributes
}
// lineType is one of v.lines: the cells of a source line, plus optional
// lineType is one of v.buf.lines: the cells of a source line, plus optional
// trailingFillAttributes recording the colors used to extend the bg
// past the line's content when the writer emitted '\x1b[K'.
type lineType struct {
@ -536,7 +573,7 @@ func NewView(name string, x0, y0, x1, y1 int, mode OutputMode) *View {
Editor: DefaultEditor,
tainted: true,
outMode: mode,
ei: newEscapeInterpreter(mode),
buf: &viewBuffer{ei: newEscapeInterpreter(mode)},
searcher: &searcher{},
TextArea: &TextArea{},
rangeSelectStartY: -1,
@ -547,7 +584,7 @@ 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()
v.buf.ei.screenColMax = v.InnerWidth()
return v
}
@ -558,7 +595,7 @@ func NewView(name string, x0, y0, x1, y1 int, mode OutputMode) *View {
// 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
v.buf.ei.screenColMax = width
}
// Dimensions returns the dimensions of the View
@ -707,15 +744,8 @@ func (v *View) CursorY() int {
// implement Horizontal and Vertical scrolling with just incrementing
// or decrementing ox and oy.
func (v *View) SetOrigin(x, y int) {
if x < 0 {
x = 0
}
if y < 0 {
y = 0
}
v.ox = x
v.oy = y
v.SetOriginX(x)
v.SetOriginY(y)
}
func (v *View) SetOriginX(x int) {
@ -755,16 +785,16 @@ func (v *View) SetWritePos(x, y int) {
y = 0
}
v.wx = x
v.wy = y
v.buf.wx = x
v.buf.wy = y
// Changing the write position makes a pending newline obsolete
v.pendingNewline = false
v.buf.pendingNewline = false
}
// WritePos returns the current write position of the view's internal buffer.
func (v *View) WritePos() (x, y int) {
return v.wx, v.wy
return v.buf.wx, v.buf.wy
}
// SetReadPos sets the read position of the view's internal buffer.
@ -788,56 +818,56 @@ func (v *View) ReadPos() (x, y int) {
}
// makeWriteable creates empty cells if required to make position (x, y) writeable.
func (v *View) makeWriteable(x, y int) {
func (b *viewBuffer) makeWriteable(x, y int) {
// TODO: make this more efficient
// line `y` must be index-able (that's why `<=`)
for len(v.lines) <= y {
if cap(v.lines) > len(v.lines) {
newLen := cap(v.lines)
for len(b.lines) <= y {
if cap(b.lines) > len(b.lines) {
newLen := cap(b.lines)
if newLen > y {
newLen = y + 1
}
v.lines = v.lines[:newLen]
b.lines = b.lines[:newLen]
} else {
v.lines = append(v.lines, lineType{})
b.lines = append(b.lines, lineType{})
}
}
// cell `x` need not be index-able (that's why `<`)
// append should be used by `lines[y]` user if he wants to write beyond `x`
for len(v.lines[y].cells) < x {
if cap(v.lines[y].cells) > len(v.lines[y].cells) {
newLen := cap(v.lines[y].cells)
for len(b.lines[y].cells) < x {
if cap(b.lines[y].cells) > len(b.lines[y].cells) {
newLen := cap(b.lines[y].cells)
if newLen > x {
newLen = x
}
v.lines[y].cells = v.lines[y].cells[:newLen]
b.lines[y].cells = b.lines[y].cells[:newLen]
} else {
v.lines[y].cells = append(v.lines[y].cells, cell{})
b.lines[y].cells = append(b.lines[y].cells, cell{})
}
}
}
// writeCells copies []cell to (v.wx, v.wy), and advances v.wx accordingly.
// writeCells copies []cell to (b.wx, b.wy), and advances b.wx accordingly.
// !!! caller MUST ensure that specified location (x, y) is writeable by calling makeWriteable
func (v *View) writeCells(cells []cell) {
func (b *viewBuffer) writeCells(cells []cell) {
var newLen int
// use maximum len available
line := v.lines[v.wy].cells[:cap(v.lines[v.wy].cells)]
maxCopy := len(line) - v.wx
line := b.lines[b.wy].cells[:cap(b.lines[b.wy].cells)]
maxCopy := len(line) - b.wx
if maxCopy < len(cells) {
copy(line[v.wx:], cells[:maxCopy])
copy(line[b.wx:], cells[:maxCopy])
line = append(line, cells[maxCopy:]...)
newLen = len(line)
} else { // maxCopy >= len(cells)
copy(line[v.wx:], cells)
newLen = v.wx + len(cells)
if newLen < len(v.lines[v.wy].cells) {
newLen = len(v.lines[v.wy].cells)
copy(line[b.wx:], cells)
newLen = b.wx + len(cells)
if newLen < len(b.lines[b.wy].cells) {
newLen = len(b.lines[b.wy].cells)
}
}
v.lines[v.wy].cells = line[:newLen]
v.wx += len(cells)
b.lines[b.wy].cells = line[:newLen]
b.wx += len(cells)
}
// Write appends a byte slice into the view's internal buffer. Because
@ -854,36 +884,54 @@ func (v *View) Write(p []byte) (n int, err error) {
}
func (v *View) write(p []byte) {
// An async re-render builds into the off-screen buffer (see View.offscreen)
// until it swaps in; until then the displayed buffer, and so everything
// readers see, is left untouched.
if v.offscreen != nil {
v.offscreen.write(v, p)
return
}
v.tainted = true
// write only ever touches lines from v.wy onwards, so any cached wrapping
// write only ever touches lines from v.buf.wy onwards, so any cached wrapping
// below that stays valid.
v.firstDirtyLine = min(v.firstDirtyLine, v.wy)
v.firstDirtyLine = min(v.firstDirtyLine, v.buf.wy)
v.clearHover()
v.buf.write(v, p)
v.updateSearchPositions()
}
// write parses p into cells and appends them to the buffer at its write cursor.
// It only touches the buffer; the View wrapper above handles display-side
// effects (tainting, hover, search). v supplies render config (Editable, colors,
// width, tab width, hyperlink auto-rendering).
func (b *viewBuffer) write(v *View, p []byte) {
// Fill with empty cells, if writing outside current view buffer
v.makeWriteable(v.wx, v.wy)
b.makeWriteable(b.wx, b.wy)
finishLine := func() {
v.autoRenderHyperlinksInCurrentLine()
b.autoRenderHyperlinksInCurrentLine(v)
}
advanceToNextLine := func() {
v.wx = 0
v.wy++
if v.wy >= len(v.lines) {
v.lines = append(v.lines, lineType{})
b.wx = 0
b.wy++
if b.wy >= len(b.lines) {
b.lines = append(b.lines, lineType{})
}
}
if v.pendingNewline {
if b.pendingNewline {
advanceToNextLine()
v.ei.notifyRowAdvance()
v.pendingNewline = false
b.ei.notifyRowAdvance()
b.pendingNewline = false
}
until := len(p)
if !v.Editable && until > 0 && p[until-1] == '\n' {
v.pendingNewline = true
b.pendingNewline = true
until--
}
@ -899,26 +947,26 @@ func (v *View) write(p []byte) {
case characterEquals(chr, '\n') || isCRLF(chr):
finishLine()
advanceToNextLine()
v.ei.notifyRowAdvance()
b.ei.notifyRowAdvance()
case characterEquals(chr, '\r'):
finishLine()
v.wx = 0
v.ei.notifyColumnReset()
b.wx = 0
b.ei.notifyColumnReset()
default:
truncateLine, cells := v.parseInput(chr, width, v.wx, v.wy)
if cd, ok := v.ei.instruction.(cursorDown); ok {
v.ei.instructionRead()
truncateLine, cells := b.parseInput(v, chr, width, b.wx, b.wy)
if cd, ok := b.ei.instruction.(cursorDown); ok {
b.ei.instructionRead()
for range cd.n {
v.autoRenderHyperlinksInCurrentLine()
b.autoRenderHyperlinksInCurrentLine(v)
advanceToNextLine()
}
}
if cells == nil {
continue
}
v.writeCells(cells)
b.writeCells(cells)
if truncateLine {
v.lines[v.wy].cells = v.lines[v.wy].cells[:v.wx]
b.lines[b.wy].cells = b.lines[b.wy].cells[:b.wx]
}
// Soft-wrap tracking. truncateLine is true exactly when the
// cells are from \x1b[K filling to end of line — ConPTY
@ -929,18 +977,16 @@ func (v *View) write(p []byte) {
for _, c := range cells {
totalWidth += c.width
}
v.ei.notifyCellsWritten(totalWidth)
b.ei.notifyCellsWritten(totalWidth)
}
}
}
if v.pendingNewline {
if b.pendingNewline {
finishLine()
} else {
v.autoRenderHyperlinksInCurrentLine()
b.autoRenderHyperlinksInCurrentLine(v)
}
v.updateSearchPositions()
}
// exported functions use the mutex. Non-exported functions are for internal use
@ -983,12 +1029,12 @@ var lineEndCharacters = map[string]bool{
")": true,
}
func (v *View) autoRenderHyperlinksInCurrentLine() {
func (b *viewBuffer) autoRenderHyperlinksInCurrentLine(v *View) {
if !v.AutoRenderHyperLinks {
return
}
line := v.lines[v.wy].cells
line := b.lines[b.wy].cells
start := 0
for {
linkStart := findLinkStart(line[start:])
@ -1005,7 +1051,7 @@ func (v *View) autoRenderHyperlinksInCurrentLine() {
link.WriteString(line[linkEnd].chr)
}
for i := linkStart; i < linkEnd; i++ {
v.lines[v.wy].cells[i].hyperlink = link.String()
b.lines[b.wy].cells[i].hyperlink = link.String()
}
start = linkEnd
}
@ -1014,13 +1060,13 @@ func (v *View) autoRenderHyperlinksInCurrentLine() {
// parseInput parses char by char the input written to the View. It returns nil
// while processing ESC sequences. Otherwise, it returns a cell slice that
// contains the processed data.
func (v *View) parseInput(ch []byte, width int, x int, _ int) (bool, []cell) {
func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bool, []cell) {
cells := []cell{}
truncateLine := false
isEscape, err := v.ei.parseOne(ch)
isEscape, err := b.ei.parseOne(ch)
if err != nil {
for _, chr := range v.ei.characters() {
for _, chr := range b.ei.characters() {
c := cell{
fgColor: v.FgColor,
bgColor: v.BgColor,
@ -1029,28 +1075,28 @@ func (v *View) parseInput(ch []byte, width int, x int, _ int) (bool, []cell) {
}
cells = append(cells, c)
}
v.ei.reset()
b.ei.reset()
} else {
repeatCount := 1
if _, ok := v.ei.instruction.(eraseInLineFromCursor); ok {
if _, ok := b.ei.instruction.(eraseInLineFromCursor); ok {
// Discard any old content past the cursor and record the
// fill colors so draw() paints the trailing area with them.
// This extends the bg to the right edge in both the
// content-fits and content-wraps cases — for the latter,
// the metadata is what reaches every wrapped segment past
// the last word.
v.ei.instructionRead()
b.ei.instructionRead()
truncateLine = true
v.lines[v.wy].trailingFillAttributes = &trailingFillAttributes{
fg: v.ei.curFgColor,
bg: v.ei.curBgColor,
b.lines[b.wy].trailingFillAttributes = &trailingFillAttributes{
fg: b.ei.curFgColor,
bg: b.ei.curBgColor,
}
return truncateLine, []cell{}
} else if cf, ok := v.ei.instruction.(cursorForward); ok {
} else if cf, ok := b.ei.instruction.(cursorForward); ok {
// emit `n` space cells under the parser-tracked SGR — used
// to materialize ConPTY's compressed runs of spaces (which
// it emits as ECH+CUF instead of literal whitespace).
v.ei.instructionRead()
b.ei.instructionRead()
repeatCount = cf.n
ch = []byte{' '}
width = 1
@ -1068,9 +1114,9 @@ func (v *View) parseInput(ch []byte, width int, x int, _ int) (bool, []cell) {
repeatCount = tabWidth - (x % tabWidth)
}
c := cell{
fgColor: v.ei.curFgColor,
bgColor: v.ei.curBgColor,
hyperlink: v.ei.hyperlink.String(),
fgColor: b.ei.curFgColor,
bgColor: b.ei.curBgColor,
hyperlink: b.ei.hyperlink.String(),
chr: string(ch),
width: width,
}
@ -1098,9 +1144,9 @@ func (v *View) Read(p []byte) (n int, err error) {
}
v.readBuffer = nil
}
for v.ry < len(v.lines) {
for v.rx < len(v.lines[v.ry].cells) {
s := v.lines[v.ry].cells[v.rx].chr
for v.ry < len(v.buf.lines) {
for v.rx < len(v.buf.lines[v.ry].cells) {
s := v.buf.lines[v.ry].cells[v.rx].chr
count := len(s)
copy(p[offset:], s)
v.rx++
@ -1122,8 +1168,17 @@ func (v *View) Read(p []byte) (n int, err error) {
// only use this if the calling function has a lock on writeMutex
func (v *View) clear() {
v.rewind()
v.lines = nil
v.buf.lines = nil
v.clearViewLines()
// Abandon any in-progress off-screen render: a synchronous SetContent/Clear
// is taking over the displayed buffer, so writes must go there, not into a
// stale off-screen buffer left by a stopped task.
v.offscreen = nil
// Likewise release any held scrollbar height: the new content is defined
// synchronously (e.g. a string render superseding a still-loading diff), so
// there's no async growth left to smooth over and the scrollbar should track
// the new content directly.
v.scrollbarHeightFloor = 0
}
// Clear empties the view's internal buffer.
@ -1164,10 +1219,10 @@ func (v *View) CopyContent(from *View) {
// This is a shallow clone -- the per-row cell data is immutable once written
// and stays shared, so the cost is proportional to the number of rows, not
// their contents.
v.lines = slices.Clone(from.lines)
v.buf.lines = slices.Clone(from.buf.lines)
v.viewLines = slices.Clone(from.viewLines)
v.ox = from.ox
v.oy = from.oy
v.SetOriginX(from.ox)
v.SetOriginY(from.oy)
v.cx = from.cx
v.cy = from.cy
}
@ -1187,23 +1242,88 @@ func (v *View) Reset() {
defer v.writeMutex.Unlock()
v.rewind()
v.lines = nil
v.buf.lines = nil
// As in clear(): abandon any in-progress off-screen render so writes after a
// reset go to the displayed buffer.
v.offscreen = nil
}
// This is for when we've done a restart for the sake of avoiding a flicker and
// we've reached the end of the new content to display: we need to clear the remaining
// content from the previous round. We do this by setting v.viewLines to nil so that
// we just render the new content from v.lines directly
func (v *View) FlushStaleCells() {
// BeginOffscreenRender starts building a re-render into an off-screen buffer.
// Until SwapInOffscreenRender promotes it, writes go to that buffer and the
// displayed buffer — what every reader sees — is left as it was. This is how an
// async re-render avoids exposing a half-written buffer: it accumulates
// off-screen and swaps in once it has read enough to paint.
func (v *View) BeginOffscreenRender() {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
v.clearViewLines()
ei := newEscapeInterpreter(v.outMode)
// The screen width content is wrapped at is render configuration set by
// SetContentWidth, not per-buffer state, so the off-screen buffer's parser
// needs it too — otherwise it counts no soft wraps and cursor-positioning
// escapes land on the wrong rows.
ei.screenColMax = v.buf.ei.screenColMax
v.offscreen = &viewBuffer{ei: ei}
}
// SwapInOffscreenRender promotes the off-screen buffer (see BeginOffscreenRender)
// to the displayed buffer in one step, so the view jumps straight from the
// previous render to the new one with no half-written frame. Writes after this
// append to the now-displayed buffer directly. It is a no-op if no off-screen
// render is in progress, so it is safe to call more than once (e.g. again at EOF
// after an earlier paint already swapped).
func (v *View) SwapInOffscreenRender() {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
if v.offscreen == nil {
return
}
v.buf = v.offscreen
v.offscreen = nil
v.tainted = true
v.clearHover()
}
// FreezeScrollbarHeight records the view's current content height so the
// scrollbar keeps that size while a re-render loads, instead of shrinking and
// snapping back as the partially-loaded content streams in past the first paint
// (see scrollbarHeightFloor). Call it when a load begins, while the view still
// shows the previous render; UnfreezeScrollbarHeight clears it when the load
// ends.
func (v *View) FreezeScrollbarHeight() {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
v.refreshViewLinesIfNeeded()
v.scrollbarHeightFloor = len(v.viewLines)
}
// UnfreezeScrollbarHeight clears the height held by FreezeScrollbarHeight, so
// the scrollbar tracks the view's content directly again. Call it when a load
// ends.
func (v *View) UnfreezeScrollbarHeight() {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
v.scrollbarHeightFloor = 0
}
// scrollbarContentHeight is the view-line height the scrollbar is sized from.
// While a re-render is loading it is held at the height the view had when the
// load began (see FreezeScrollbarHeight), so the thumb doesn't shrink and jump
// as partially-loaded content streams in.
func (v *View) scrollbarContentHeight() int {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
v.refreshViewLinesIfNeeded()
return max(len(v.viewLines), v.scrollbarHeightFloor)
}
func (v *View) rewind() {
v.ei.reset()
v.ei.resetScreenCursor()
v.buf.ei.reset()
v.buf.ei.resetScreenCursor()
v.SetReadPos(0, 0)
v.SetWritePos(0, 0)
@ -1275,14 +1395,14 @@ func (v *View) updateSearchPositions() {
for _, result := range v.searcher.modelSearchResults {
// This code only works when v.Wrap is false.
if result.Y >= len(v.lines) {
if result.Y >= len(v.buf.lines) {
break
}
// If a view line exists for this line index:
if v.lines[result.Y].cells != nil {
if v.buf.lines[result.Y].cells != nil {
// search this view line for the search string
positions := searchPositionsForLine(v.lines[result.Y].cells, result.Y)
positions := searchPositionsForLine(v.buf.lines[result.Y].cells, result.Y)
if len(positions) > 0 {
// If we found any occurrences, add them
v.searcher.searchPositions = append(v.searcher.searchPositions, positions...)
@ -1335,14 +1455,14 @@ func (v *View) draw(isWindowFocused bool) {
if maxX == 0 {
return
}
v.ox = 0
v.SetOriginX(0)
}
v.refreshViewLinesIfNeeded()
visibleViewLinesHeight := v.viewLineLengthIgnoringTrailingBlankLines()
if v.Autoscroll && visibleViewLinesHeight > maxY {
v.oy = visibleViewLinesHeight - maxY
v.SetOriginY(visibleViewLinesHeight - maxY)
}
if len(v.viewLines) == 0 {
@ -1429,7 +1549,7 @@ func (v *View) refreshViewLinesIfNeeded() {
}
lineIdx := 0
lines := v.lines
lines := v.buf.lines
for i := range lines {
line := &lines[i]
@ -1475,6 +1595,13 @@ func (v *View) refreshViewLinesIfNeeded() {
}
v.firstDirtyLine = len(lines)
// Truncate any entries left over from a previous, longer render. An async
// re-render builds its content off-screen and swaps it in whole (see
// View.offscreen), so the buffer this rebuilds from is always a complete
// render — there is no half-loaded shorter buffer whose tail we'd need to
// keep showing to avoid a flicker, and a leftover tail would just be stale
// lines mapping to the wrong buffer rows.
v.viewLines = v.viewLines[:lineIdx]
v.tainted = false
}
@ -1553,8 +1680,8 @@ func (v *View) BufferLines() []string {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
lines := make([]string, len(v.lines))
for i, l := range v.lines {
lines := make([]string, len(v.buf.lines))
for i, l := range v.buf.lines {
lines[i] = l.cells.String()
}
return lines
@ -1566,7 +1693,7 @@ func (v *View) Buffer() string {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
return linesToString(v.lines)
return linesToString(v.buf.lines)
}
// ViewBufferLines returns the lines in the view's internal
@ -1586,7 +1713,7 @@ func (v *View) ViewBufferLines() []string {
// LinesHeight is the count of view lines (i.e. lines excluding wrapping)
func (v *View) LinesHeight() int {
return len(v.lines)
return len(v.buf.lines)
}
// ViewLinesHeight is the count of view lines (i.e. lines including wrapping)
@ -1617,11 +1744,11 @@ func (v *View) Line(y int) (string, bool) {
return "", false
}
if y < 0 || y >= len(v.lines) {
if y < 0 || y >= len(v.buf.lines) {
return "", false
}
return v.lines[y].cells.String(), true
return v.buf.lines[y].cells.String(), true
}
// Word returns a string with the word of the view's internal buffer
@ -1632,11 +1759,11 @@ func (v *View) Word(x, y int) (string, bool) {
return "", false
}
if x < 0 || y < 0 || y >= len(v.lines) || x >= len(v.lines[y].cells) {
if x < 0 || y < 0 || y >= len(v.buf.lines) || x >= len(v.buf.lines[y].cells) {
return "", false
}
str := v.lines[y].cells.String()
str := v.buf.lines[y].cells.String()
nl := strings.LastIndexFunc(str[:x], indexFunc)
if nl == -1 {
@ -1662,12 +1789,12 @@ func indexFunc(r rune) bool {
// SetHighlight toggles highlighting of separate lines, for custom lists
// or multiple selection in views.
func (v *View) SetHighlight(y int, on bool) {
if y < 0 || y >= len(v.lines) {
if y < 0 || y >= len(v.buf.lines) {
return
}
cells := make([]cell, 0, len(v.lines[y].cells))
for _, c := range v.lines[y].cells {
cells := make([]cell, 0, len(v.buf.lines[y].cells))
for _, c := range v.buf.lines[y].cells {
if on {
c.bgColor = v.SelBgColor
c.fgColor = v.SelFgColor
@ -1679,7 +1806,7 @@ func (v *View) SetHighlight(y int, on bool) {
}
v.tainted = true
v.firstDirtyLine = min(v.firstDirtyLine, y)
v.lines[y].cells = cells
v.buf.lines[y].cells = cells
v.clearHover()
}
@ -1791,7 +1918,7 @@ func (v *View) SelectedLine() string {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
if len(v.lines) == 0 {
if len(v.buf.lines) == 0 {
return ""
}
@ -1803,7 +1930,7 @@ func (v *View) SelectedLines() []string {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
if len(v.lines) == 0 {
if len(v.buf.lines) == 0 {
return nil
}
@ -1818,7 +1945,7 @@ func (v *View) SelectedLines() []string {
}
func (v *View) lineContentAtIdx(idx int) string {
return v.lines[idx].cells.String()
return v.buf.lines[idx].cells.String()
}
func (v *View) SelectedPoint() (int, int) {
@ -1891,8 +2018,8 @@ func (v *View) ClearTextArea() {
func (v *View) overwriteLines(y int, content string) {
// break by newline, then for each line, write it, then add that erase command
v.wx = 0
v.wy = y
v.buf.wx = 0
v.buf.wy = y
v.clearViewLines()
lines := strings.ReplaceAll(content, "\n", "\x1b[K\n")
@ -1904,7 +2031,7 @@ func (v *View) overwriteLines(y int, content string) {
v.writeString(lines)
}
// only call this function if you don't care where v.wx and v.wy end up
// only call this function if you don't care where v.buf.wx and v.buf.wy end up
func (v *View) OverwriteLines(y int, content string) {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
@ -1912,7 +2039,7 @@ func (v *View) OverwriteLines(y int, content string) {
v.overwriteLines(y, content)
}
// only call this function if you don't care where v.wx and v.wy end up
// only call this function if you don't care where v.buf.wx and v.buf.wy end up
func (v *View) OverwriteLinesAndClearEverythingElse(lineCount int, y int, content string) {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
@ -1922,19 +2049,19 @@ func (v *View) OverwriteLinesAndClearEverythingElse(lineCount int, y int, conten
v.overwriteLines(y, content)
for i := range y {
v.lines[i] = lineType{}
v.buf.lines[i] = lineType{}
}
for i := v.wy + 1; i < len(v.lines); i += 1 {
v.lines[i] = lineType{}
for i := v.buf.wy + 1; i < len(v.buf.lines); i += 1 {
v.buf.lines[i] = lineType{}
}
}
func (v *View) setContentLineCount(lineCount int) {
if lineCount > 0 {
v.makeWriteable(0, lineCount-1)
v.buf.makeWriteable(0, lineCount-1)
}
v.lines = v.lines[:lineCount]
v.buf.lines = v.buf.lines[:lineCount]
}
// If the current search result is no longer visible after a scroll up, select the last search
@ -1989,7 +2116,7 @@ func (v *View) ScrollUp(amount int) {
}
if amount != 0 {
v.oy -= amount
v.SetOriginY(v.oy - amount)
v.cy += amount
v.clearHover()
@ -2001,7 +2128,7 @@ func (v *View) ScrollUp(amount int) {
func (v *View) ScrollDown(amount int) {
adjustedAmount := v.adjustDownwardScrollAmount(amount)
if adjustedAmount > 0 {
v.oy += adjustedAmount
v.SetOriginY(v.oy + adjustedAmount)
v.cy -= adjustedAmount
v.clearHover()
@ -2015,7 +2142,7 @@ func (v *View) ScrollLeft(amount int) {
newOx = 0
}
if newOx != v.ox {
v.ox = newOx
v.SetOriginX(newOx)
v.clearHover()
}
@ -2023,7 +2150,7 @@ func (v *View) ScrollLeft(amount int) {
// not applying any limits to this
func (v *View) ScrollRight(amount int) {
v.ox += amount
v.SetOriginX(v.ox + amount)
v.clearHover()
}
@ -2068,7 +2195,7 @@ func (v *View) scrollMargin() int {
// Returns true if the view contains a line containing the given text with the given
// foreground color
func (v *View) ContainsColoredText(fgColor string, text string) bool {
for _, line := range v.lines {
for _, line := range v.buf.lines {
if containsColoredTextInLine(fgColor, text, line.cells) {
return true
}
@ -2105,6 +2232,9 @@ func (v *View) onMouseMove(x int, y int) {
return
}
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
// newCx and newCy are relative to the view port, i.e. to the visible area of the view
newCx := x - v.x0 - 1
newCy := y - v.y0 - 1
@ -2123,6 +2253,19 @@ func (v *View) onMouseMove(x int, y int) {
}
}
// hyperlinkAt returns the hyperlink at the given position of the view's
// content, or an empty string if there is none.
func (v *View) hyperlinkAt(x, y int) string {
v.writeMutex.Lock()
defer v.writeMutex.Unlock()
if y < 0 || y >= len(v.viewLines) || x < 0 || x >= len(v.viewLines[y].line) {
return ""
}
return v.viewLines[y].line[x].hyperlink
}
func (v *View) findHyperlinkAt(x, y int) *SearchPosition {
linkStr := v.viewLines[y].line[x].hyperlink
if linkStr == "" {

View file

@ -101,13 +101,13 @@ func TestWriteString(t *testing.T) {
for _, test := range tests {
v := NewView("name", 0, 0, 10, 10, OutputNormal)
for _, l := range test.existingLines {
v.lines = append(v.lines, lineType{cells: stringToCells(l)})
v.buf.lines = append(v.buf.lines, lineType{cells: stringToCells(l)})
}
for _, s := range test.stringsToWrite {
v.writeString(s)
}
var resultingLines [][]string
for _, l := range v.lines {
for _, l := range v.buf.lines {
resultingLines = append(resultingLines, cellsToStrings(l.cells))
}
assert.Equal(t, test.expectedLines, resultingLines)
@ -144,19 +144,115 @@ func TestAutoRenderingHyperlinks(t *testing.T) {
v.writeString("htt")
// No hyperlinks are generated for incomplete URLs
assert.Equal(t, "", v.lines[0].cells[0].hyperlink)
assert.Equal(t, "", v.buf.lines[0].cells[0].hyperlink)
// Writing more characters to the same line makes the link complete (even
// though we didn't see a newline yet)
v.writeString("ps://example.com")
assert.Equal(t, "https://example.com", v.lines[0].cells[0].hyperlink)
assert.Equal(t, "https://example.com", v.buf.lines[0].cells[0].hyperlink)
v.Clear()
// Valid but incomplete URL
v.writeString("https://exa")
assert.Equal(t, "https://exa", v.lines[0].cells[0].hyperlink)
assert.Equal(t, "https://exa", v.buf.lines[0].cells[0].hyperlink)
// Writing more characters to the same fixes the link
v.writeString("mple.com")
assert.Equal(t, "https://example.com", v.lines[0].cells[0].hyperlink)
assert.Equal(t, "https://example.com", v.buf.lines[0].cells[0].hyperlink)
}
// An async re-render builds into an off-screen buffer and swaps it in once it
// has enough to paint, so readers keep seeing the previous render — coherent and
// consistent — until the new content appears in one step. See View.offscreen.
func TestOffscreenRender(t *testing.T) {
v := NewView("name", 0, 0, 80, 10, OutputNormal)
v.writeString("a\nb\nc")
assert.Equal(t, []string{"a", "b", "c"}, v.ViewBufferLines())
// Render new, longer content off-screen.
v.BeginOffscreenRender()
v.writeString("w\nx\ny\nz")
// The displayed buffer is untouched: readers still see the previous render.
assert.Equal(t, []string{"a", "b", "c"}, v.ViewBufferLines())
// Swapping in reveals the new content in one step.
v.SwapInOffscreenRender()
assert.Equal(t, []string{"w", "x", "y", "z"}, v.ViewBufferLines())
// A further write now appends to the displayed buffer directly.
v.writeString("\nmore")
assert.Equal(t, []string{"w", "x", "y", "z", "more"}, v.ViewBufferLines())
}
// When a render produces fewer view lines than the previous one,
// refreshViewLinesIfNeeded must truncate viewLines to the new content rather
// than leaving the previous render's entries in the tail: with the off-screen
// render there is no half-loaded buffer whose tail we'd want to keep showing,
// and a leftover tail is just stale lines describing content that is gone.
func TestViewLinesTruncatedByShorterRender(t *testing.T) {
v := NewView("name", 0, 0, 10, 10, OutputNormal) // InnerWidth is 9
v.Wrap = true
// Two lines of 27 characters each wrap into 3 view lines apiece.
v.writeString(strings.Repeat("a", 27) + "\n" + strings.Repeat("b", 27))
assert.Equal(t, 6, v.ViewLinesHeight())
// Re-render with three short, unwrapped lines: only 3 view lines remain.
v.BeginOffscreenRender()
v.writeString("aaa\nbbb\nccc")
v.SwapInOffscreenRender()
assert.Equal(t, 3, v.ViewLinesHeight())
assert.Equal(t, []string{"aaa", "bbb", "ccc"}, v.ViewBufferLines())
}
// While an async re-render loads, it swaps in only a partially-filled buffer at
// its first paint and keeps appending lines afterwards. The scrollbar must keep
// using the pre-load height until the load ends, so the thumb doesn't shrink and
// snap back as the rest streams in. See View.scrollbarHeightFloor.
func TestScrollbarHeightHeldWhileLoading(t *testing.T) {
v := NewView("name", 0, 0, 80, 12, OutputNormal)
// Initial render: 100 lines, scrolled well down.
v.writeString(strings.Repeat("x\n", 100))
v.SetOrigin(0, 80)
assert.Equal(t, 100, v.scrollbarContentHeight())
// A re-render begins while the previous render is still shown: hold the
// scrollbar height at the current value.
v.FreezeScrollbarHeight()
// The off-screen render swaps in only a screenful at its first paint.
v.BeginOffscreenRender()
v.writeString(strings.Repeat("y\n", 30))
v.SwapInOffscreenRender()
// The displayed buffer is now short, but the scrollbar height stays held, so
// the thumb keeps its position instead of jumping.
assert.Equal(t, 30, v.ViewLinesHeight())
assert.Equal(t, 100, v.scrollbarContentHeight())
// The rest of the content streams in.
v.writeString(strings.Repeat("y\n", 70))
assert.Equal(t, 100, v.scrollbarContentHeight())
// Once the load ends, the scrollbar tracks the real content directly again.
v.UnfreezeScrollbarHeight()
assert.Equal(t, 100, v.scrollbarContentHeight())
}
// If a synchronous render (e.g. a string render) supersedes a still-loading diff
// before it reaches its end, the held scrollbar height must be released, so the
// scrollbar reflects the new content rather than the abandoned load's height.
func TestScrollbarHeightReleasedWhenContentReplaced(t *testing.T) {
v := NewView("name", 0, 0, 80, 12, OutputNormal)
v.writeString(strings.Repeat("x\n", 100))
v.FreezeScrollbarHeight()
assert.Equal(t, 100, v.scrollbarContentHeight())
// A synchronous render replaces the content before the (notional) load ends.
v.SetContent("just a few\nshort lines\nhere")
assert.Equal(t, 3, v.scrollbarContentHeight())
}
func TestContainsColoredText(t *testing.T) {
@ -233,7 +329,7 @@ func TestContainsColoredText(t *testing.T) {
for j, cells := range test.lines {
lines[j] = lineType{cells: cells}
}
v := &View{lines: lines}
v := &View{buf: &viewBuffer{lines: lines}}
assert.Equal(t, test.expected, v.ContainsColoredText(test.fgColorStr, test.text), "Test %d failed", i)
}
}
@ -248,8 +344,8 @@ func TestWriteCursorPositionEscape(t *testing.T) {
// "a", then "skip to row 3" (i.e. one blank row), then "b".
v.writeString("a\r\n\x1b[3;1Hb\r\n")
got := make([][]string, 0, len(v.lines))
for _, l := range v.lines {
got := make([][]string, 0, len(v.buf.lines))
for _, l := range v.buf.lines {
got = append(got, cellsToStrings(l.cells))
}
@ -269,8 +365,8 @@ func TestWriteCursorPositionEscapeAcrossWrites(t *testing.T) {
// ConPTY is on row 3 here; CUP to row 5 should skip exactly one row.
v.writeString("c\x1b[5;1Hd\n")
got := make([][]string, 0, len(v.lines))
for _, l := range v.lines {
got := make([][]string, 0, len(v.buf.lines))
for _, l := range v.buf.lines {
got = append(got, cellsToStrings(l.cells))
}
assert.Equal(t, [][]string{
@ -282,6 +378,31 @@ func TestWriteCursorPositionEscapeAcrossWrites(t *testing.T) {
}, got)
}
func TestWriteCursorPositionEscapeInOffscreenRender(t *testing.T) {
// Soft-wrap counting has to work in an off-screen render too: the content
// width the parser counts wraps against is set by SetContentWidth before the
// render starts, so the off-screen buffer's parser has to pick it up. If it
// doesn't, no wraps are counted and the CUP below is evaluated against a
// stale row, overshooting into an extra blank line.
v := NewView("name", 0, 0, 30, 30, OutputNormal)
v.SetContentWidth(5)
v.BeginOffscreenRender()
// Seven characters soft-wrap once on a 5-column screen, putting ConPTY on
// row 2; CUP to row 3 should then skip no rows at all.
v.writeString("aaaaaaa\x1b[3;1Hb\n")
v.SwapInOffscreenRender()
got := make([][]string, 0, len(v.buf.lines))
for _, l := range v.buf.lines {
got = append(got, cellsToStrings(l.cells))
}
assert.Equal(t, [][]string{
{"a", "a", "a", "a", "a", "a", "a"},
{"b"},
}, got)
}
func TestWriteCursorForwardEscape(t *testing.T) {
// ConPTY compresses runs of default-colored spaces into ECH (\x1b[NX,
// "clear N cells, cursor stationary") + CUF (\x1b[NC, "cursor forward
@ -292,8 +413,8 @@ func TestWriteCursorForwardEscape(t *testing.T) {
// "a" + ECH 5 + CUF 5 + "b" — visually "a b".
v.writeString("a\x1b[5X\x1b[5Cb\n")
got := make([][]string, 0, len(v.lines))
for _, l := range v.lines {
got := make([][]string, 0, len(v.buf.lines))
for _, l := range v.buf.lines {
got = append(got, cellsToStrings(l.cells))
}
@ -312,8 +433,8 @@ func TestWriteCursorPositionEscapeWithSoftWraps(t *testing.T) {
v.writeString("abcdefghij\n")
v.writeString("\x1b[4;1Hxyz\n")
got := make([][]string, 0, len(v.lines))
for _, l := range v.lines {
got := make([][]string, 0, len(v.buf.lines))
for _, l := range v.buf.lines {
got = append(got, cellsToStrings(l.cells))
}
assert.Equal(t, [][]string{

View file

@ -88,7 +88,13 @@ func (gui *Gui) layout(g *gocui.Gui) error {
if !view.CanScrollPastBottom {
maxOriginY -= newHeight - 1
}
if oldOriginY := view.OriginY(); oldOriginY > maxOriginY {
// Don't scroll up while the view's content is still being loaded: its
// height only reflects what has been read so far, so clamping to it now
// would yank the view to the top even though more content is on the way
// (e.g. when re-rendering a diff the user was scrolled into).
manager := gui.getViewBufferManagerForView(view)
stillLoading := manager != nil && manager.IsLoading()
if oldOriginY := view.OriginY(); oldOriginY > maxOriginY && !stillLoading {
view.ScrollUp(oldOriginY - maxOriginY)
// the view might not have scrolled actually (if it was at the limit
// already), so we need to check if it did

View file

@ -107,16 +107,6 @@ func (gui *Gui) allMainContextPairs() []types.MainContextPair {
}
func (gui *Gui) refreshMainViews(opts types.RefreshMainOpts) {
// need to reset scroll positions of all other main views
for _, pair := range gui.allMainContextPairs() {
if pair.Main != opts.Pair.Main {
pair.Main.GetView().SetOrigin(0, 0)
}
if pair.Secondary != nil && pair.Secondary != opts.Pair.Secondary {
pair.Secondary.GetView().SetOrigin(0, 0)
}
}
gui.moveMainContextPairToTop(opts.Pair)
if opts.Main != nil {
@ -129,6 +119,20 @@ func (gui *Gui) refreshMainViews(opts types.RefreshMainOpts) {
opts.Pair.Secondary.GetView().Clear()
}
// Reset the scroll positions of all the other main views. We do this after
// moving this pair to the top (which copies the previously-shown view's
// content into the now-visible one to avoid a blank frame): resetting first
// would zero that source view's scroll before it gets copied, forcing the
// placeholder to the top instead of leaving it where the screen already was.
for _, pair := range gui.allMainContextPairs() {
if pair.Main != opts.Pair.Main {
pair.Main.GetView().SetOrigin(0, 0)
}
if pair.Secondary != nil && pair.Secondary != opts.Pair.Secondary {
pair.Secondary.GetView().SetOrigin(0, 0)
}
}
gui.splitMainPanel(opts.Secondary != nil)
}

View file

@ -72,6 +72,16 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
cmd.Args = withPtyGitConfig(cmd.Args, runtime.GOOS)
// Mark the view as loading synchronously now, before the layout pass: the
// actual task is created in afterLayout (below), which runs after layout, so
// without this the next layout pass would clamp the scroll position to the
// not-yet-loaded content.
gui.getManager(view).StartLoading()
// Hold the scrollbar at its current height while the re-render loads, so the
// thumb doesn't shrink and snap back when the first partial paint swaps in
// (see the matching call in newCmdTask).
view.FreezeScrollbarHeight()
// Run the pty after layout so that it gets the correct size
gui.afterLayout(func() error {
// Need to get the width and the pager command again because the layout might have

View file

@ -18,6 +18,15 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
).Debug("RunCommand")
manager := gui.getManager(view)
// Mark the view as loading synchronously (before the task's goroutine runs
// and before the next layout pass) so the layout doesn't clamp the scroll
// position to the not-yet-loaded content.
manager.StartLoading()
// Hold the scrollbar at the height the view has now (the previous render),
// while it still shows that render: once the re-render swaps in its first
// partial paint the displayed buffer is briefly short, and we don't want the
// thumb to shrink and snap back as the rest loads.
view.FreezeScrollbarHeight()
// 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
@ -133,12 +142,10 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager {
gui.Log,
view,
func() {
// we could clear here, but that actually has the effect of causing a flicker
// where the view may contain no content momentarily as the gui refreshes.
// Instead, we're rewinding the write pointer so that we will just start
// overwriting the existing content from the top down. Once we've reached
// the end of the content do display, we call view.FlushStaleCells() to
// clear out the remaining content from the previous render.
// Called before showing the "loading..." indicator: clear the
// displayed buffer so only "loading..." is shown. The actual content
// is rendered off-screen (beginRender below) and swapped in, so it
// never overwrites the displayed buffer incrementally.
view.Reset()
},
func() {
@ -150,6 +157,11 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager {
gui.renderContentOnly()
},
func() {
// The content is fully loaded now, so let the scrollbar track it
// directly again (it was held at the previous render's height while
// loading, see FreezeScrollbarHeight).
view.UnfreezeScrollbarHeight()
// Need to check if the content of the view is well past the origin.
linesHeight := view.ViewLinesHeight()
_, originY := view.Origin()
@ -158,12 +170,12 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager {
view.SetOrigin(0, newOriginY)
}
view.FlushStaleCells()
},
func() {
view.SetOrigin(0, 0)
},
view.BeginOffscreenRender,
view.SwapInOffscreenRender,
func() gocui.Task {
// A background task: rendering content into a view is display
// work, not lazygit driving a git operation, so it must not

View file

@ -4,7 +4,9 @@ import (
"bufio"
"fmt"
"io"
"os"
"os/exec"
"strconv"
"sync"
"sync/atomic"
"time"
@ -61,22 +63,59 @@ type ViewBufferManager struct {
writer io.Writer
waitingMutex deadlock.Mutex
taskIDMutex deadlock.Mutex
Log *logrus.Entry
newTaskID int
// Guards newTaskID and taskKey, which identify the most recently requested
// task. Both are written on the goroutine NewTask spawns, and taskKey is
// read from the UI thread (GetTaskKey), so neither may be touched without
// holding this.
taskIDMutex deadlock.Mutex
Log *logrus.Entry
newTaskID int
// The channel by which the currently-running task is told to read more
// lines (e.g. as the user scrolls). Held in an atomic because it's swapped
// out as tasks come and go while ReadLines/ReadToEnd read it from the UI
// thread; nil when no task is running.
readLines atomic.Pointer[chan LinesToRead]
taskKey string
onNewKey func()
// Resets the view's scroll position to the top. A render whose content is
// different from what the view last showed (a different command key) calls
// this — but at its *first paint*, not when the task starts: the off-screen
// render leaves the previous content displayed until the swap, so resetting
// the origin up front would scroll that still-displayed content to the top
// before the new content replaces it. See newContentPending.
resetOrigin func()
// Whether the content the running task is rendering differs from what the
// view is currently showing (i.e. the command key changed). Two things key
// off it: the loading indicator only takes the view over when it is set,
// since there is no point clearing content we are about to render
// identically; and the first paint that reveals the content resets the
// scroll to the top and clears it.
//
// It deliberately outlives the task that set it: a task can be stopped and
// replaced before it ever paints — a background refresh landing just after
// the user clicked a different item, say — and the replacement, which
// renders the same content and so sets nothing of its own, still has to do
// what that task was owed.
newContentPending atomic.Bool
// Whether a command task is currently reading content into the view. While
// this is true the content is still growing, so callers (e.g. the layout)
// must not clamp the view's scroll position to the amount loaded so far.
loading atomic.Bool
// beforeStart is the function that is called before starting a new task
beforeStart func()
refreshView func()
onEndOfInput func()
// beginRender starts an off-screen render: the new content is built without
// disturbing what's displayed. swapInRender then promotes it to the display
// in one step. Together they keep the view showing the previous render until
// the new one has read enough to paint, instead of revealing it line by line.
beginRender func()
swapInRender func()
// see docs/dev/Busy.md
// A gocui task is not the same thing as the tasks defined in this file.
// A gocui task simply represents the fact that lazygit is busy doing something,
@ -115,6 +154,9 @@ type LinesToRead struct {
}
func (self *ViewBufferManager) GetTaskKey() string {
self.taskIDMutex.Lock()
defer self.taskIDMutex.Unlock()
return self.taskKey
}
@ -124,7 +166,9 @@ func NewViewBufferManager(
beforeStart func(),
refreshView func(),
onEndOfInput func(),
onNewKey func(),
resetOrigin func(),
beginRender func(),
swapInRender func(),
newGocuiTask func() gocui.Task,
onUIThread func(f func()) error,
) *ViewBufferManager {
@ -134,7 +178,9 @@ func NewViewBufferManager(
beforeStart: beforeStart,
refreshView: refreshView,
onEndOfInput: onEndOfInput,
onNewKey: onNewKey,
resetOrigin: resetOrigin,
beginRender: beginRender,
swapInRender: swapInRender,
newGocuiTask: newGocuiTask,
onUIThread: onUIThread,
}
@ -153,6 +199,21 @@ func (self *ViewBufferManager) ReadLines(totalLines int) {
}
}
// IsLoading reports whether a command task is currently reading content into the
// view, meaning the content is still growing.
func (self *ViewBufferManager) IsLoading() bool {
return self.loading.Load()
}
// StartLoading marks the view as loading content. It must be called
// synchronously when a command/pty task is started, before the task's goroutine
// runs, so that a layout pass happening in between doesn't clamp the scroll
// position to the not-yet-loaded content. It is cleared when the task reaches
// the end of its input.
func (self *ViewBufferManager) StartLoading() {
self.loading.Store(true)
}
func (self *ViewBufferManager) ReadToEnd(then func()) {
if ch := self.readLines.Load(); ch != nil {
readLines := *ch
@ -271,8 +332,21 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
return
case <-ticker.C:
loadingMutex.Lock()
if !loaded {
// Only take the view over to say "loading..." when the content coming
// is different from what's on screen. A re-render of the same content
// leaves the view showing exactly what it should already, so clearing
// it for the message and then rendering the same thing back is a
// visible flicker for nothing — and a slow re-render of unchanged
// content is common (a background refresh over a repo with submodules
// that have uncommitted changes, say). The pending flag isn't consumed
// here; the first paint still owes the scroll reset.
if !loaded && self.newContentPending.Load() {
self.beforeStart()
// beforeStart cleared the previous content to show "loading...", so
// put the view back at the top for it (beforeStart doesn't touch the
// origin). The origin is view state the UI thread reads while laying
// out, so write it there.
_ = self.onUIThread(self.resetOrigin)
_, _ = self.writer.Write([]byte("loading..."))
self.refreshView()
}
@ -297,8 +371,9 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
// closed the selects below could still service a ready data channel
// instead of bailing. Check stop explicitly first to give it priority:
// a task that's been stopped (it's being replaced by a newer one) must
// not touch the view here — beforeStart clears it and the prefix gets
// written, clobbering what the incoming task is about to render.
// not touch the view here — it would start an off-screen render and
// write the prefix into it, clobbering what the incoming task is about
// to render.
stopped := func() bool {
select {
case <-opts.Stop:
@ -313,6 +388,36 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
// this to work out how many more lines, if any, we still need to read.
linesRead := 0
// The first paint swaps the off-screen render in to reveal the new
// content, and settles the scroll position in the same step — so the new
// content first appears already where it belongs, and no draw can land
// between the two and show it at the previous render's scroll. It happens
// once, either when we've read far enough (below) or at end of input for
// content shorter than that. Callers run it on the UI thread: it writes
// the view's origin.
painted := false
firstPaint := func() {
if painted {
return
}
painted = true
self.swapInRender()
if self.newContentPending.Swap(false) {
self.resetOrigin()
}
}
// Set LAZYGIT_SLOW_RENDER=<milliseconds> to sleep that long after each
// line is written to the view, stretching async loads out so the frames
// of a re-render become visible. Useful for debugging scroll/flicker
// behaviour; has no effect when the variable is unset.
var slowRenderPerLine time.Duration
if v := os.Getenv("LAZYGIT_SLOW_RENDER"); v != "" {
if ms, err := strconv.Atoi(v); err == nil {
slowRenderPerLine = time.Duration(ms) * time.Millisecond
}
}
outer:
for {
if stopped() {
@ -344,7 +449,10 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
loadingMutex.Lock()
if !loaded {
self.beforeStart()
// Build the new content off-screen, leaving the previous render
// displayed until we swap in below; this is what keeps an async
// re-render from showing a half-loaded buffer.
self.beginRender()
if prefix != "" {
writeToView([]byte(prefix))
}
@ -353,23 +461,68 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
loadingMutex.Unlock()
if !ok {
// if we're here then there's nothing left to scan from the source
// so we're at the EOF and can flush the stale content.
// onEndOfInput reads the view's dimensions (to decide
// whether to scroll) and sets the origin, both of which
// are UI-thread-only, so run it there.
_ = self.onUIThread(self.onEndOfInput)
// lineChan is closed. At a genuine end of input we swap in what we
// read and finalize. But lineChan is also closed when this task has
// been stopped to make way for a newer one: stopping closes
// opts.Stop, and the scanner goroutine then closes lineChan, so the
// select above can land here instead of on the opts.Stop case. A
// stopped task is being replaced and must leave the view to the
// incoming task — swapping in its half-read buffer, clamping the
// origin, or clearing `loading` would all corrupt what that task is
// about to render. So bail out here, the same as the explicit stop
// case above.
select {
case <-opts.Stop:
callThen()
break outer
default:
}
// Genuine end of input: do the first paint now if it hasn't happened
// yet (the content was shorter than a screenful, so we never reached
// the point below), and flush the stale content. onEndOfInput reads
// the view's dimensions (to decide whether to scroll) and sets the
// origin, both of which are UI-thread-only, so run it there — as is
// firstPaint, which also writes the origin.
_ = self.onUIThread(func() {
firstPaint()
self.onEndOfInput()
})
// The content is fully loaded now, so it's safe again for the
// layout to clamp the scroll position to it. We deliberately
// don't clear this when stopped (rather than EOF'd), because that
// means a newer task is taking over and is still loading.
self.loading.Store(false)
callThen()
// Any read requests that were queued while we were reading are
// now trivially satisfied, since we've read everything. Fire
// their callbacks instead of dropping them when we break out of
// the loop below (and nil out readLines).
drain:
for {
select {
case queued := <-readLines:
if queued.Then != nil {
queued.Then()
}
default:
break drain
}
}
break outer
}
writeToView(append(line, '\n'))
lineWrittenChan <- struct{}{}
linesRead++
if slowRenderPerLine > 0 {
time.Sleep(slowRenderPerLine)
}
if linesRead == linesToRead.InitialRefreshAfter {
// We have read enough lines to fill the view, so do a first refresh
// here to show what we have. Continue reading and refresh again at
// the end to make sure the scrollbar has the right size.
// We have read enough lines to fill the view, so do the first paint
// and refresh to show it. Continue reading and refresh again at the
// end to make sure the scrollbar has the right size.
_ = self.onUIThread(firstPaint)
refreshViewIfStale()
}
}
@ -488,20 +641,19 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
return
}
resetOrigin := self.GetTaskKey() != key && self.onNewKey != nil
// Note we don't reset the origin here even when the command key changed:
// that's deferred to the first paint that reveals the new content (see
// newContentPending), so the previous content — left displayed until the
// swap — doesn't visibly jump to the top before the new content appears.
// Read taskKey directly: we already hold the mutex that guards it, and
// GetTaskKey would take it again.
if self.taskKey != key && self.resetOrigin != nil {
self.newContentPending.Store(true)
}
self.taskKey = key
self.taskIDMutex.Unlock()
if resetOrigin {
// onNewKey resets the view's scroll origin, which is view state the
// UI thread reads while laying out and drawing, so do it there. This
// must happen after releasing taskIDMutex: it blocks until the UI
// thread runs it, and a NewTask call on the UI thread takes
// taskIDMutex, so holding it here would deadlock.
_ = self.onUIThread(self.onNewKey)
}
self.waitingMutex.Lock()
// Re-check staleness after acquiring waitingMutex: a newer task

View file

@ -7,11 +7,13 @@ import (
"reflect"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/jesseduffield/lazygit/pkg/gocui"
"github.com/jesseduffield/lazygit/pkg/utils"
"github.com/stretchr/testify/assert"
)
func getCounter() (func(), func() int) {
@ -24,7 +26,9 @@ func TestNewCmdTaskInstantStop(t *testing.T) {
beforeStart, getBeforeStartCallCount := getCounter()
refreshView, getRefreshViewCallCount := getCounter()
onEndOfInput, getOnEndOfInputCallCount := getCounter()
onNewKey, getOnNewKeyCallCount := getCounter()
resetOrigin, getResetOriginCallCount := getCounter()
beginRender, getBeginRenderCallCount := getCounter()
swapInRender, getSwapInRenderCallCount := getCounter()
onDone, getOnDoneCallCount := getCounter()
task := gocui.NewFakeTask()
newTask := func() gocui.Task {
@ -37,7 +41,9 @@ func TestNewCmdTaskInstantStop(t *testing.T) {
beforeStart,
refreshView,
onEndOfInput,
onNewKey,
resetOrigin,
beginRender,
swapInRender,
newTask,
// no UI thread in the test; run the view mutations inline
func(f func()) error { f(); return nil },
@ -66,7 +72,9 @@ func TestNewCmdTaskInstantStop(t *testing.T) {
{0, getBeforeStartCallCount(), "beforeStart"},
{1, getRefreshViewCallCount(), "refreshView"},
{0, getOnEndOfInputCallCount(), "onEndOfInput"},
{0, getOnNewKeyCallCount(), "onNewKey"},
{0, getResetOriginCallCount(), "resetOrigin"},
{0, getBeginRenderCallCount(), "beginRender"},
{0, getSwapInRenderCallCount(), "swapInRender"},
{1, getOnDoneCallCount(), "onDone"},
}
for _, expectation := range callCountExpectations {
@ -91,7 +99,9 @@ func TestNewCmdTask(t *testing.T) {
beforeStart, getBeforeStartCallCount := getCounter()
refreshView, getRefreshViewCallCount := getCounter()
onEndOfInput, getOnEndOfInputCallCount := getCounter()
onNewKey, getOnNewKeyCallCount := getCounter()
resetOrigin, getResetOriginCallCount := getCounter()
beginRender, getBeginRenderCallCount := getCounter()
swapInRender, getSwapInRenderCallCount := getCounter()
onDone, getOnDoneCallCount := getCounter()
task := gocui.NewFakeTask()
newTask := func() gocui.Task {
@ -104,7 +114,9 @@ func TestNewCmdTask(t *testing.T) {
beforeStart,
refreshView,
onEndOfInput,
onNewKey,
resetOrigin,
beginRender,
swapInRender,
newTask,
// no UI thread in the test; run the view mutations inline
func(f func()) error { f(); return nil },
@ -134,10 +146,12 @@ func TestNewCmdTask(t *testing.T) {
actual int
name string
}{
{1, getBeforeStartCallCount(), "beforeStart"},
{0, getBeforeStartCallCount(), "beforeStart"},
{1, getRefreshViewCallCount(), "refreshView"},
{1, getOnEndOfInputCallCount(), "onEndOfInput"},
{0, getOnNewKeyCallCount(), "onNewKey"},
{0, getResetOriginCallCount(), "resetOrigin"},
{1, getBeginRenderCallCount(), "beginRender"},
{1, getSwapInRenderCallCount(), "swapInRender"},
{1, getOnDoneCallCount(), "onDone"},
}
for _, expectation := range callCountExpectations {
@ -174,6 +188,206 @@ func (d *BlankLineReader) Read(p []byte) (n int, err error) {
return 1, nil
}
// A dummy reader that yields the given number of blank lines and then blocks
// until unblock is closed, at which point it reports EOF. This lets a test hold
// a task in its "still loading" state for as long as it needs to.
type BlockingLineReader struct {
linesToYield int
linesYielded int
reachedEnd bool
blocked chan struct{}
unblock chan struct{}
}
func (d *BlockingLineReader) Read(p []byte) (n int, err error) {
if d.linesYielded == d.linesToYield {
if !d.reachedEnd {
d.reachedEnd = true
close(d.blocked)
}
<-d.unblock
return 0, io.EOF
}
d.linesYielded++
p[0] = '\n'
return 1, nil
}
func TestNewCmdTaskQueuedReadAtEndOfInput(t *testing.T) {
writer := bytes.NewBuffer(nil)
task := gocui.NewFakeTask()
manager := NewViewBufferManager(
utils.NewDummyLog(),
writer,
func() {},
func() {},
func() {},
func() {},
func() {},
func() {},
func() gocui.Task { return task },
// no UI thread in the test; run the view mutations inline
func(f func()) error { f(); return nil },
)
reader := BlockingLineReader{
linesToYield: 5,
blocked: make(chan struct{}),
unblock: make(chan struct{}),
}
start := func() (Cmd, io.Reader) {
// not actually starting this because it's not necessary
return ExecCmd{Cmd: exec.Command("blah")}, &reader
}
// The initial request asks for far more lines than the reader has, so the
// task reaches EOF while that request is still the one being served.
fn := manager.NewCmdTask(start, "", LinesToRead{100, -1, nil}, func() {})
thenCalled := false
wg := sync.WaitGroup{}
wg.Go(func() {
_ = fn(TaskOpts{Stop: make(chan struct{}), InitialContentLoaded: func() { task.Done() }})
})
<-reader.blocked
manager.ReadToEnd(func() { thenCalled = true })
// ReadToEnd queues its request from a goroutine; wait for it to land so that
// it is definitely outstanding by the time we let the task reach EOF.
for len(*manager.readLines.Load()) == 0 {
time.Sleep(time.Millisecond)
}
close(reader.unblock)
wg.Wait()
assert.True(t, thenCalled)
}
// A task rendering content the view wasn't already showing resets the scroll
// position to the top, at its first paint. If it is stopped and replaced before
// it ever paints — a background refresh landing just after the user clicked a
// different item, say — the replacement renders the same content and so decides
// on no reset of its own; it has to perform the one the stopped task was owed,
// or the view keeps the scroll position of the content it showed before.
func TestResetOriginSurvivesTaskReplacement(t *testing.T) {
resetOrigin, getResetOriginCallCount := getCounter()
manager := NewViewBufferManager(
utils.NewDummyLog(),
bytes.NewBuffer(nil),
func() {},
func() {},
func() {},
resetOrigin,
func() {},
func() {},
func() gocui.Task { return gocui.NewFakeTask() },
// no UI thread in the test; run the view mutations inline
func(f func()) error { f(); return nil },
)
startTask := func(key string, reader io.Reader, onDone func()) {
start := func() (Cmd, io.Reader) {
// not actually starting this because it's not necessary
return ExecCmd{Cmd: exec.Command("blah")}, reader
}
// The first-paint point is far beyond what any of these readers yield, so
// only reaching EOF paints.
_ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, onDone), key)
}
runTaskToCompletion := func(key string) {
done := make(chan struct{})
startTask(key, &BlankLineReader{totalLinesToYield: 3}, func() { close(done) })
<-done
}
// A render of content the view wasn't showing resets the scroll position.
runTaskToCompletion("cmd1")
assert.Equal(t, 1, getResetOriginCallCount())
// Different content again, but this task stalls before it can paint.
stalled := BlockingLineReader{
linesToYield: 3,
blocked: make(chan struct{}),
unblock: make(chan struct{}),
}
defer close(stalled.unblock)
startTask("cmd2", &stalled, nil)
<-stalled.blocked
// The replacement shows the same content as the stalled task, so it has no
// reset of its own to do — but it must still do that task's.
runTaskToCompletion("cmd2")
assert.Equal(t, 2, getResetOriginCallCount())
}
// A render that takes long enough to start takes the view over to say
// "loading...", which means blanking whatever it was showing. That is only worth
// doing when the content coming is different from what's on screen: re-rendering
// the same content (a background refresh, say) would otherwise blank the view and
// paint the same thing back, a visible flicker for nothing.
func TestLoadingIndicatorOnlyTakesOverForNewContent(t *testing.T) {
var beforeStartCount atomic.Int32
manager := NewViewBufferManager(
utils.NewDummyLog(),
io.Discard,
func() { beforeStartCount.Add(1) },
func() {},
func() {},
func() {},
func() {},
func() {},
func() gocui.Task { return gocui.NewFakeTask() },
// no UI thread in the test; run the view mutations inline
func(f func()) error { f(); return nil },
)
startTask := func(key string, reader io.Reader, onDone func()) {
start := func() (Cmd, io.Reader) {
// not actually starting this because it's not necessary
return ExecCmd{Cmd: exec.Command("blah")}, reader
}
_ = manager.NewTask(manager.NewCmdTask(start, "", LinesToRead{100, 50, nil}, onDone), key)
}
// Starts a task whose command produces nothing at all, so that it is still
// waiting for its first line when the loading indicator falls due. Returns
// the reader so the caller can let it finish.
startStalledTask := func(key string) *BlockingLineReader {
reader := &BlockingLineReader{
blocked: make(chan struct{}),
unblock: make(chan struct{}),
}
startTask(key, reader, nil)
<-reader.blocked
return reader
}
// Get some content on screen first: the indicator is only due when a render
// is slow, and this one isn't.
done := make(chan struct{})
startTask("cmd1", &BlankLineReader{totalLinesToYield: 3}, func() { close(done) })
<-done
assert.EqualValues(t, 0, beforeStartCount.Load())
// A slow re-render of that same content must leave the view alone however
// long it takes. The indicator is due 200ms in, so give it well past that.
sameContent := startStalledTask("cmd1")
defer close(sameContent.unblock)
time.Sleep(500 * time.Millisecond)
assert.EqualValues(t, 0, beforeStartCount.Load())
// Different content, though, is worth taking the view over for.
newContent := startStalledTask("cmd2")
defer close(newContent.unblock)
assert.Eventually(t,
func() bool { return beforeStartCount.Load() == 1 },
2*time.Second, 10*time.Millisecond)
}
func TestNewCmdTaskRefresh(t *testing.T) {
type scenario struct {
name string
@ -240,6 +454,8 @@ func TestNewCmdTaskRefresh(t *testing.T) {
refreshView,
func() {},
func() {},
func() {},
func() {},
newTask,
// no UI thread in the test; run the view mutations inline
func(f func()) error { f(); return nil },