Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: incorrect line skipping in renderer flush #1233

Merged
merged 5 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions examples/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/aymerick/douceur v0.2.0 // indirect
github.com/charmbracelet/x/ansi v0.4.5 // indirect
github.com/charmbracelet/x/exp/golden v0.0.0-20240815200342-61de596daa2b // indirect
github.com/charmbracelet/x/term v0.2.0 // indirect
github.com/charmbracelet/x/term v0.2.1 // indirect
github.com/dlclark/regexp2 v1.11.0 // indirect
github.com/dustin/go-humanize v1.0.1 // indirect
github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
Expand All @@ -41,8 +41,8 @@ require (
github.com/yuin/goldmark v1.7.4 // indirect
github.com/yuin/goldmark-emoji v1.0.3 // indirect
golang.org/x/net v0.27.0 // indirect
golang.org/x/sync v0.8.0 // indirect
golang.org/x/sys v0.26.0 // indirect
golang.org/x/sync v0.9.0 // indirect
golang.org/x/sys v0.27.0 // indirect
golang.org/x/term v0.22.0 // indirect
golang.org/x/text v0.19.0 // indirect
)
Expand Down
12 changes: 6 additions & 6 deletions examples/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ github.com/charmbracelet/x/exp/golden v0.0.0-20240815200342-61de596daa2b h1:MnAM
github.com/charmbracelet/x/exp/golden v0.0.0-20240815200342-61de596daa2b/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U=
github.com/charmbracelet/x/exp/teatest v0.0.0-20240521184646-23081fb03b28 h1:sOWKNRjt8uOEVgPiJVIJCse1+mUDM2F/vYY6W0Go640=
github.com/charmbracelet/x/exp/teatest v0.0.0-20240521184646-23081fb03b28/go.mod h1:l1w+LTJZCCozeGzMEWGxRw6Mo2DfcZUvupz8HGubdes=
github.com/charmbracelet/x/term v0.2.0 h1:cNB9Ot9q8I711MyZ7myUR5HFWL/lc3OpU8jZ4hwm0x0=
github.com/charmbracelet/x/term v0.2.0/go.mod h1:GVxgxAbjUrmpvIINHIQnJJKpMlHiZ4cktEQCN6GWyF0=
github.com/charmbracelet/x/term v0.2.1 h1:AQeHeLZ1OqSXhrAWpYUtZyX1T3zVxfpZuEQMIQaGIAQ=
github.com/charmbracelet/x/term v0.2.1/go.mod h1:oQ4enTYFV7QN4m0i9mzHrViD7TQKvNEEkHUMCmsxdUg=
github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI=
github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
Expand Down Expand Up @@ -76,12 +76,12 @@ github.com/yuin/goldmark-emoji v1.0.3 h1:aLRkLHOuBR2czCY4R8olwMjID+tENfhyFDMCRhb
github.com/yuin/goldmark-emoji v1.0.3/go.mod h1:tTkZEbwu5wkPmgTcitqddVxY9osFZiavD+r4AzQrh1U=
golang.org/x/net v0.27.0 h1:5K3Njcw06/l2y9vpGCSdcxWOYHOUk3dVNGDXN+FvAys=
golang.org/x/net v0.27.0/go.mod h1:dDi0PyhWNoiUOrAS8uXv/vnScO4wnHQO4mj9fn/RytE=
golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sync v0.9.0 h1:fEo0HyrW1GIgZdpbhCRO0PkJajUS5H9IFUztCgEo2jQ=
golang.org/x/sync v0.9.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo=
golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.22.0 h1:BbsgPEJULsl2fV/AT3v15Mjva5yXKQDyKf+TbDz7QJk=
golang.org/x/term v0.22.0/go.mod h1:F3qCibpT5AMpCRfhfT53vVJwhLtIVHhB9XDjfFvnMI4=
golang.org/x/text v0.19.0 h1:kTxAhCbGbxhK0IwgSKiMO5awPoDQ0RpfiVYBfK860YM=
Expand Down
15 changes: 10 additions & 5 deletions standard_renderer.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type standardRenderer struct {
ticker *time.Ticker
done chan struct{}
lastRender string
lastRenderedLines []string
linesRendered int
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can possibly remove the linesRendered field and use len(lastRenderedLines) instead. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will break tea.Println. We reset the lastRenderedLines when we call repaint, which is the case when using tea.Println. We need to keep track of the number of lines last rendered for tea.Println to work.

useANSICompressor bool
once sync.Once
Expand Down Expand Up @@ -174,7 +175,6 @@ func (r *standardRenderer) flush() {
}

newLines := strings.Split(r.buf.String(), "\n")
oldLines := strings.Split(r.lastRender, "\n")

// If we know the output's height, we can use it to determine how many
// lines we can render. We drop lines from the top of the render buffer if
Expand All @@ -184,7 +184,6 @@ func (r *standardRenderer) flush() {
newLines = newLines[len(newLines)-r.height:]
}

numLinesThisFlush := len(newLines)
flushQueuedMessages := len(r.queuedMessageLines) > 0 && !r.altScreenActive

if flushQueuedMessages {
Expand All @@ -210,7 +209,7 @@ func (r *standardRenderer) flush() {
// Paint new lines.
for i := 0; i < len(newLines); i++ {
canSkip := !flushQueuedMessages && // Queuing messages triggers repaint -> we don't have access to previous frame content.
len(oldLines) > i && oldLines[i] == newLines[i] // Previously rendered line is the same.
len(r.lastRenderedLines) > i && r.lastRenderedLines[i] == newLines[i] // Previously rendered line is the same.

if _, ignore := r.ignoreLines[i]; ignore || canSkip {
// Unless this is the last line, move the cursor down.
Expand Down Expand Up @@ -257,11 +256,11 @@ func (r *standardRenderer) flush() {
}

// Clearing left over content from last render.
if r.linesRendered > numLinesThisFlush {
if r.linesRendered > len(newLines) {
buf.WriteString(ansi.EraseScreenBelow)
}

r.linesRendered = numLinesThisFlush
r.linesRendered = len(newLines)

// Make sure the cursor is at the start of the last line to keep rendering
// behavior consistent.
Expand All @@ -276,6 +275,11 @@ func (r *standardRenderer) flush() {

_, _ = r.out.Write(buf.Bytes())
r.lastRender = r.buf.String()

// Save previously rendered lines for comparison in the next render. If we
// don't do this, we can't skip rendering lines that haven't changed.
// See https://github.com/charmbracelet/bubbletea/pull/1233
r.lastRenderedLines = newLines
r.buf.Reset()
}

Expand All @@ -299,6 +303,7 @@ func (r *standardRenderer) write(s string) {

func (r *standardRenderer) repaint() {
r.lastRender = ""
r.lastRenderedLines = nil
}

func (r *standardRenderer) clearScreen() {
Expand Down
Loading