Repository: charmbracelet/bubbletea
Pull request #1251 (2024-11-22 19:59:44)

fix: renderer: keep a separate count of lines rendered in the alt screen 
We need to keep a separate count of lines rendered in the alt screen and
inline mode. This is because when we enter alt screen mode, the cursor
position is saved, and the alt screen is cleared. Now when we exit the
alt screen mode, the cursor position is restored so we don't need to
move the cursor to the beginning of the section that we rendered.

To fix this, we keep a separate count of lines rendered in the alt screen
mode and inline mode.

Related: https://github.com/charmbracelet/bubbletea/issues/1013
Fixes: https://github.com/charmbracelet/bubbletea/pull/1241
Fixes: https://github.com/charmbracelet/bubbletea/issues/1248


CC/ @unitsvc @semihbkgr 

unitsvc: Thank you for the fix, it works perfectly.

unitsvc: @meowgorithm CC
semihbkgr: @aymanbagabas The bug described in #1248 is not a regression in `v1.2.3`. It is a type of bug I intended to fix in my PR, but the scenario explained in the issue (where the program starts in `altscreen` mode) is not covered by my fix.

semihbkgr: I did some debugging to understand why this case is not covered. It turns out that when the renderer is initialized, `renderedLines` is set to 0 (the default value for an integer) before calling flush. In this scenario, the program enters alt screen mode before `flush` is called for the first time, so `linesRenderedBeforeAltScreen` remains 0. When this value is 0, we avoid moving the cursor in the `exitAltScreen` function.

```go
if r.linesRendered < r.linesRenderedBeforeAltScreen {
...
} else if r.linesRendered > r.linesRenderedBeforeAltScreen && r.linesRenderedBeforeAltScreen > 0 {
...
}
```

So, the default value of `renderedLines` should be set to 1 before calling flush. This is because, when the program starts, the cursor position will be on the first line.
semihbkgr: When testing this PR, I realized that aligning the cursor position by moving it up and down in the `exitAltScreen` function was not a good idea, as it could cause undesired behavior in some cases. For example:
When the program is terminated while in alt screen mode, it calls the `restoreTerminalState` function, which subsequently calls `exitAltScreen`. During this process, the cursor could potentially move down unnecessarily, adding extra unwanted spaces that won’t be used by the next flush, before the program terminates.
The second problem was that moving the cursor down did not work when it reached the bottom of the terminal. The `flush` function can handle this more properly by writing `\r\n`.
These are the issues that will be resolved after this PR.
unitsvc: @aymanbagabas Can this pr be merged and a new version released

Patch:
diff --git a/standard_renderer.go b/standard_renderer.go
index aa8524b..369f27f 100644
--- a/standard_renderer.go
+++ b/standard_renderer.go
@@ -36,6 +36,7 @@ type standardRenderer struct {
 	lastRender         string
 	lastRenderedLines  []string
 	linesRendered      int
+	altLinesRendered   int
 	useANSICompressor  bool
 	once               sync.Once
 
@@ -57,9 +58,6 @@ type standardRenderer struct {
 
 	// lines explicitly set not to render
 	ignoreLines map[int]struct{}
-
-	// lines rendered before entering alt screen mode
-	linesRenderedBeforeAltScreen int
 }
 
 // newRenderer creates a new renderer. Normally you'll want to initialize it
@@ -173,7 +171,9 @@ func (r *standardRenderer) flush() {
 	buf := &bytes.Buffer{}
 
 	// Moving to the beginning of the section, that we rendered.
-	if r.linesRendered > 1 {
+	if r.altScreenActive {
+		buf.WriteString(ansi.HomeCursorPosition)
+	} else if r.linesRendered > 1 {
 		buf.WriteString(ansi.CursorUp(r.linesRendered - 1))
 	}
 
@@ -258,12 +258,21 @@ func (r *standardRenderer) flush() {
 		}
 	}
 
+	lastLinesRendered := r.linesRendered
+	if r.altScreenActive {
+		lastLinesRendered = r.altLinesRendered
+	}
+
 	// Clearing left over content from last render.
-	if r.linesRendered > len(newLines) {
+	if lastLinesRendered > len(newLines) {
 		buf.WriteString(ansi.EraseScreenBelow)
 	}
 
-	r.linesRendered = len(newLines)
+	if r.altScreenActive {
+		r.altLinesRendered = len(newLines)
+	} else {
+		r.linesRendered = len(newLines)
+	}
 
 	// Make sure the cursor is at the start of the last line to keep rendering
 	// behavior consistent.
@@ -271,7 +280,7 @@ func (r *standardRenderer) flush() {
 		// This case fixes a bug in macOS terminal. In other terminals the
 		// other case seems to do the job regardless of whether or not we're
 		// using the full terminal window.
-		buf.WriteString(ansi.SetCursorPosition(0, r.linesRendered))
+		buf.WriteString(ansi.SetCursorPosition(0, len(newLines)))
 	} else {
 		buf.WriteString(ansi.CursorLeft(r.width))
 	}
@@ -337,10 +346,6 @@ func (r *standardRenderer) enterAltScreen() {
 	r.altScreenActive = true
 	r.execute(ansi.EnableAltScreenBuffer)
 
-	// Save the current line count before entering the alternate screen mode.
-	// This allows us to compare and adjust the cursor position when exiting the alternate screen.
-	r.linesRenderedBeforeAltScreen = r.linesRendered
-
 	// Ensure that the terminal is cleared, even when it doesn't support
 	// alt screen (or alt screen support is disabled, like GNU screen by
 	// default).
@@ -359,6 +364,9 @@ func (r *standardRenderer) enterAltScreen() {
 		r.execute(ansi.ShowCursor)
 	}
 
+	// Entering the alt screen resets the lines rendered count.
+	r.altLinesRendered = 0
+
 	r.repaint()
 }
 
@@ -373,18 +381,6 @@ func (r *standardRenderer) exitAltScreen() {
 	r.altScreenActive = false
 	r.execute(ansi.DisableAltScreenBuffer)
 
-	// Adjust cursor and screen
-	if r.linesRendered < r.linesRenderedBeforeAltScreen {
-		// If fewer lines were rendered in the alternate screen, move the cursor up
-		// to align with the previous normal screen position and clear any remaining lines.
-		r.execute(ansi.CursorUp(r.linesRenderedBeforeAltScreen - r.linesRendered))
-		r.execute(ansi.EraseScreenBelow)
-	} else if r.linesRendered > r.linesRenderedBeforeAltScreen && r.linesRenderedBeforeAltScreen > 0 {
-		// If more lines were rendered in the alternate screen, move the cursor down
-		// to align with the new position.
-		r.execute(ansi.CursorDown(r.linesRendered - r.linesRenderedBeforeAltScreen))
-	}
-
 	// cmd.exe and other terminals keep separate cursor states for the AltScreen
 	// and the main buffer. We have to explicitly reset the cursor visibility
 	// whenever we exit AltScreen.


Test patch:
diff --git a/screen_test.go b/screen_test.go
index 0a7facb..6011d51 100644
--- a/screen_test.go
+++ b/screen_test.go
@@ -24,7 +24,7 @@ func TestClearMsg(t *testing.T) {
 		{
 			name:     "altscreen_autoexit",
 			cmds:     []Cmd{EnterAltScreen},
-			expected: "\x1b[?25l\x1b[?2004h\x1b[?1049h\x1b[2J\x1b[H\x1b[?25l\rsuccess\x1b[0K\r\n\x1b[0K\x1b[2;0H\x1b[2K\r\x1b[?2004l\x1b[?25h\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1049l\x1b[?25h",
+			expected: "\x1b[?25l\x1b[?2004h\x1b[?1049h\x1b[2J\x1b[H\x1b[?25l\x1b[H\rsuccess\x1b[0K\r\n\x1b[0K\x1b[2;0H\x1b[2K\r\x1b[?2004l\x1b[?25h\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1049l\x1b[?25h",
 		},
 		{
 			name:     "mouse_cellmotion",