Repository: charmbracelet/bubbletea
Pull request #1276 (2024-12-27 21:28:04)

Maintain exec output
Ref #1275 

meowgorithm: Hey, Raphaël! Thanks for the PR. Do you have any sample code to help illustrate the issue? Providing it will help us expedite this one.

We're using exec to print above inline TUIs quite a lot but haven't encountered this issue, so any sample code would really help illustrate the problem.
raphaelvigee: ```
package main

import (
	"fmt"
	tea "github.com/charmbracelet/bubbletea"
	"io"
	"os"
	"strings"
)

type execCmd struct {
	w io.Writer
}

type execDoneMsg struct{}

func (e *execCmd) Run() error {
	for i := range 8 {
		e.w.Write([]byte(fmt.Sprintf("exec %v\n", i)))
	}

	return nil
}

func (e *execCmd) SetStdin(r io.Reader) {
}

func (e *execCmd) SetStdout(w io.Writer) {
	e.w = w
}

func (e *execCmd) SetStderr(w io.Writer) {
}

type model struct{}

func (m model) Init() tea.Cmd {
	return nil
}

func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	var cmd tea.Cmd
	var cmds []tea.Cmd

	switch msg := msg.(type) {
	case tea.KeyMsg:
		switch msg.String() {
		case "ctrl+c", "q":
			return m, tea.Quit
		}
	case execDoneMsg:
		cmds = append(cmds, tea.Quit)
	}

	cmds = append(cmds, cmd)

	return m, tea.Batch(cmds...)
}

func (m model) View() string {
	var sb strings.Builder

	for i := range 5 {
		sb.WriteString(fmt.Sprintf("view %v\n", i))
	}

	return sb.String()
}

func main() {
	initialModel := model{}

	p := tea.NewProgram(initialModel)

	go func() {
		p.Send(
			tea.Exec(&execCmd{}, func(err error) tea.Msg {
				return execDoneMsg{}
			})(),
		)
	}()

	if _, err := p.Run(); err != nil {
		fmt.Println("Error:", err)
		os.Exit(1)
	}
}

```

Current:
```
view 0
view 1
view 2
view 3
view 4
exec 0
exec 1
exec 2
view 0
view 1
view 2
view 3
view 4
```

i only see 3 lines from exec, but i expect to see all 8 lines that were printed

With fix:
```
view 0
view 1
view 2
view 3
view 4
exec 0
exec 1
exec 2
exec 3
exec 4
exec 5
exec 6
exec 7
view 0
view 1
view 2
view 3
view 4
```
raphaelvigee: @meowgorithm Whats the path towards getting that merged ?
raphaelvigee: @aymanbagabas @meowgorithm Circling back on this, does this patch looks good to be merged ? Having to maintain my fork of bbt for this change is a pain...
raphaelvigee: Circling back again @aymanbagabas @meowgorithm I would really like to see this merged, with the release of v2, that would be an awesome addition
raphaelvigee: @aymanbagabas Could you provide me some guidance wrt to how to write a test for it ? Cause this is dependent on the rendering on the terminal...
aymanbagabas: > @aymanbagabas Could you provide me some guidance wrt to how to write a test for it ? Cause this is dependent on the rendering on the terminal...

We can add a simple test in `exec_test.go` to check if `p.renderer.resetLinesRendered` is called.
raphaelvigee: Done, ready for review!
aymanbagabas: @raphaelvigee Question: do you know if this happens with v2?
raphaelvigee: I havent taken v2 for a ride just yet
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/charmbracelet/bubbletea/pull/1276?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet) Report
Attention: Patch coverage is `50.00000%` with `3 lines` in your changes missing coverage. Please review.
> Please [upload](https://docs.codecov.com/docs/codecov-uploader) report for BASE (`main@2ea49dd`). [Learn more](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet#section-missing-base-commit) about missing BASE report.

| [Files with missing lines](https://app.codecov.io/gh/charmbracelet/bubbletea/pull/1276?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet) | Patch % | Lines |
|---|---|---|
| [standard\_renderer.go](https://app.codecov.io/gh/charmbracelet/bubbletea/pull/1276?src=pr&el=tree&filepath=standard_renderer.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet#diff-c3RhbmRhcmRfcmVuZGVyZXIuZ28=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/charmbracelet/bubbletea/pull/1276?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet) |
| [nil\_renderer.go](https://app.codecov.io/gh/charmbracelet/bubbletea/pull/1276?src=pr&el=tree&filepath=nil_renderer.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet#diff-bmlsX3JlbmRlcmVyLmdv) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/charmbracelet/bubbletea/pull/1276?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet) |

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main    #1276   +/-   ##
=======================================
  Coverage        ?   69.99%           
=======================================
  Files           ?       17           
  Lines           ?     1733           
  Branches        ?        0           
=======================================
  Hits            ?     1213           
  Misses          ?      472           
  Partials        ?       48           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/charmbracelet/bubbletea/pull/1276?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=charmbracelet).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
raphaelvigee: I have a bunch of seemingly unrelated CI failures, do you have any insights about those ?
aymanbagabas: > I have a bunch of seemingly unrelated CI failures, do you have any insights about those ?

Make sure you rebase your changes
raphaelvigee: @aymanbagabas could you green light the CI ?
raphaelvigee: CI seems to still be failing for the same reason...
raphaelvigee: Still failing... Whats the path towards getting that merged ?

Patch:
diff --git a/exec.go b/exec.go
index 7a14d2a..8048414 100644
--- a/exec.go
+++ b/exec.go
@@ -114,6 +114,7 @@ func (p *Program) exec(c ExecCommand, fn ExecCallback) {
 
 	// Execute system command.
 	if err := c.Run(); err != nil {
+		p.renderer.resetLinesRendered()
 		_ = p.RestoreTerminal() // also try to restore the terminal.
 		if fn != nil {
 			go p.Send(fn(err))
@@ -121,6 +122,9 @@ func (p *Program) exec(c ExecCommand, fn ExecCallback) {
 		return
 	}
 
+	// Maintain the existing output from the command
+	p.renderer.resetLinesRendered()
+
 	// Have the program re-capture input.
 	err := p.RestoreTerminal()
 	if fn != nil {
diff --git a/nil_renderer.go b/nil_renderer.go
index 0bc4a17..1bc909b 100644
--- a/nil_renderer.go
+++ b/nil_renderer.go
@@ -26,3 +26,4 @@ func (n nilRenderer) setWindowTitle(_ string)    {}
 func (n nilRenderer) reportFocus() bool          { return false }
 func (n nilRenderer) enableReportFocus()         {}
 func (n nilRenderer) disableReportFocus()        {}
+func (n nilRenderer) resetLinesRendered()        {}
diff --git a/renderer.go b/renderer.go
index 9eb7943..5b6df66 100644
--- a/renderer.go
+++ b/renderer.go
@@ -79,6 +79,9 @@ type renderer interface {
 
 	// disableReportFocus stops reporting focus events to the program.
 	disableReportFocus()
+
+	// resetLinesRendered ensures exec output remains on screen on exit
+	resetLinesRendered()
 }
 
 // repaintMsg forces a full repaint.
diff --git a/standard_renderer.go b/standard_renderer.go
index 45e8c82..d6dce6d 100644
--- a/standard_renderer.go
+++ b/standard_renderer.go
@@ -545,6 +545,10 @@ func (r *standardRenderer) clearIgnoredLines() {
 	r.ignoreLines = nil
 }
 
+func (r *standardRenderer) resetLinesRendered() {
+	r.linesRendered = 0
+}
+
 // insertTop effectively scrolls up. It inserts lines at the top of a given
 // area designated to be a scrollable region, pushing everything else down.
 // This is roughly how ncurses does it.


Test patch:
diff --git a/exec_test.go b/exec_test.go
index d49f0cc..cdeac23 100644
--- a/exec_test.go
+++ b/exec_test.go
@@ -37,6 +37,16 @@ func (m *testExecModel) View() string {
 	return "\n"
 }
 
+type spyRenderer struct {
+	renderer
+	calledReset bool
+}
+
+func (r *spyRenderer) resetLinesRendered() {
+	r.calledReset = true
+	r.renderer.resetLinesRendered()
+}
+
 func TestTeaExec(t *testing.T) {
 	type test struct {
 		name      string
@@ -76,9 +86,14 @@ func TestTeaExec(t *testing.T) {
 			if _, err := p.Run(); err != nil {
 				t.Error(err)
 			}
+			p.renderer = &spyRenderer{renderer: p.renderer}
 
 			if m.err != nil && !test.expectErr {
 				t.Errorf("expected no error, got %v", m.err)
+
+				if !p.renderer.(*spyRenderer).calledReset {
+					t.Error("expected renderer to be reset")
+				}
 			}
 			if m.err == nil && test.expectErr {
 				t.Error("expected error, got nil")