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

Incorrect line skipping in renderer flush #1232

Closed
semihbkgr opened this issue Nov 11, 2024 · 1 comment · Fixed by #1233
Closed

Incorrect line skipping in renderer flush #1232

semihbkgr opened this issue Nov 11, 2024 · 1 comment · Fixed by #1233

Comments

@semihbkgr
Copy link
Contributor

Describe the bug

In the renderer flush, lines are skipped if they match the previous line. However, the comparison between the current and previous lines is not done properly.

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

After that, the newLines are trimmed based on the terminal height to fit the screen:

if r.height > 0 && len(newLines) > r.height {
    newLines = newLines[len(newLines)-r.height:]
}

This trimming causes the index to be lost. The first item in oldLines may not correspond to the first item in newLines if the number of lines in the current buffer exceeds the terminal height.

The issue occurs in the following comparison:

canSkip := !flushQueuedMessages && 
    len(oldLines) > i && oldLines[i] == newLines[i]

In some cases, this leads to skipping line updates even when two lines are not the same (and vice versa). This happens because oldLines always starts from the top of the previous buffer, while newLines may have been truncated.

The solution is to compare the most recently rendered lines with the new lines, instead of comparing the lines from the beginning of the previous buffer.

Setup
Please complete the following information along with version numbers, if applicable.

  • OS: Debian
  • Shell: zsh
  • Terminal Emulator: kitty
  • Terminal Multiplexer: tmux

To Reproduce

It can be reproduced with the following steps:

  1. Press j to increase the number of lines until it exceeds the terminal height.
  2. Press k to decrease the number of lines until it is smaller than the terminal height.
  3. You will observe that the lines are misaligned, with each line appearing one row higher than its correct position on the screen, and the first line will be missing.
package main

import (
	"fmt"
	"os"
	"strings"

	tea "github.com/charmbracelet/bubbletea"
)

type model struct {
	n int
}

func (m model) Init() tea.Cmd {
	return nil
}

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	switch msg := msg.(type) {
	case tea.KeyMsg:
		switch msg.String() {
		case "q", "ctrl+c", "esc":
			return m, tea.Quit
		case "j":
			m.n += 1
		case "k":
			m.n -= 1
		}
	}
	return m, nil
}

func (m model) View() string {
	b := strings.Builder{}
	for i := range m.n {
		b.WriteString(fmt.Sprintf("%d. line\n", i))
	}
	return fmt.Sprintf("FirstLine\n%sLastLine", b.String())
}

func main() {
	if _, err := tea.NewProgram(model{n: 9}).Run(); err != nil {
		fmt.Println("Error running program:", err)
		os.Exit(1)
	}
}

Expected behavior

Lines should be skipped correctly only when the current line is identical to the previously rendered line

Screenshots
Screenshot from 2024-11-11 23-03-03
Adding lines...
Screenshot from 2024-11-11 23-03-20
Deleting lines...
Screenshot from 2024-11-11 23-03-34

@aymanbagabas
Copy link
Member

This is now fixed in v1.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants