diff --git a/modules/bonfire/README.md b/modules/bonfire/README.md index af043f02..ee43685b 100644 --- a/modules/bonfire/README.md +++ b/modules/bonfire/README.md @@ -27,7 +27,7 @@ Use cases: - Usage strings and argument-count checks derived from the declared arguments (`` for required, `[name]` for optional). - Scalar flags, bare flags (`bonfire.Flag.Bare`, so `--force` alone stores `true`) and ordered repeatable flags (`bonfire.Flag.Repeatable`, read back through `bonfire.Input.Flags`). - Styled status lines: `bonfire.Output.Info`, `bonfire.Output.Success`, `bonfire.Output.Warning` and `bonfire.Output.Error` (the last one writes to the error stream). -- Widgets on the charm stack, ported from SSU's terminal UI: box-drawn tables (`bonfire.Output.Table`, lipgloss), a spinner around a function (`bonfire.Output.Spinner`), a gradient progress bar (`bonfire.Output.Progress`, whose `bonfire.Progress` also implements `bonfire.ItemProgress`) and a multi-row process view for bulk loops (`bonfire.Process`), each a bubbletea program that keeps stdin and signals to the command. Log lines and output printed while a view runs appear above it (`bonfire.PrintAbove`). +- Widgets on the charm stack, ported from SSU's terminal UI: box-drawn tables (`bonfire.Output.Table`, lipgloss), a spinner around a function (`bonfire.Output.Spinner`), a gradient progress bar (`bonfire.Output.Progress`, whose `bonfire.Progress` also implements `bonfire.ItemProgress`) and a multi-row process view for bulk loops (`bonfire.Process`), each a bubbletea program that keeps stdin and signals to the command. Log lines, and output the command prints through the console that started the view, appear above a running view (`bonfire.PrintAbove`); other consoles (a `bonfire.Call` buffer, a console on a file) keep writing to their own writers. - Prompts: `bonfire.Output.Ask`, `bonfire.Output.Confirm` and `bonfire.Output.Secret` use huh fields on an interactive terminal, and `bonfire.Output.Choice` an arrow-key selector with a confirmation screen. Off a terminal they read lines from stdin as before (a piped password still works), return their defaults when input ends, and `bonfire.Output.Confirm` returns its default without asking. A cancelled prompt returns `bonfire.ErrAborted`. `bonfire.ConfirmProceed` asks before a destructive step only on an interactive terminal. - Degradation: every widget follows the terminal and the global options, and the `bonfire.Output` interface is unchanged for plugins: diff --git a/modules/bonfire/helpers.go b/modules/bonfire/helpers.go index f6c8885b..17fb0546 100644 --- a/modules/bonfire/helpers.go +++ b/modules/bonfire/helpers.go @@ -149,7 +149,7 @@ func Process(ctx context.Context, out Output, operation string, items []string, } if c, ok := out.(*console); ok && c.live() && len(items) > 0 { var errs []error - _ = runLive(ctx, c.out, c, newProcessModel(c.palette, operation, items), func(send func(tea.Msg)) error { + started, _ := runLive(ctx, c.out, c, newProcessModel(c.palette, operation, items), func(send func(tea.Msg)) error { for i, item := range items { if err := ctx.Err(); err != nil { errs = append(errs, err) @@ -165,7 +165,9 @@ func Process(ctx context.Context, out Output, operation string, items []string, } return nil }) - return errors.Join(errs...) + if started { + return errors.Join(errs...) + } } var errs []error for _, item := range items { diff --git a/modules/bonfire/live.go b/modules/bonfire/live.go index f69bfe65..9eec5684 100644 --- a/modules/bonfire/live.go +++ b/modules/bonfire/live.go @@ -8,6 +8,7 @@ package bonfire import ( "context" + "errors" "fmt" "io" "strings" @@ -314,26 +315,33 @@ func (m processModel) View() tea.View { return tea.NewView(b.String()) } -// The running live program, for PrintAbove. Only one live view runs at a -// time; widgets started while one runs print plain lines. -var ( - liveMu sync.Mutex - activeProg *tea.Program -) - -// liveActive reports whether a live view is running. -func liveActive() bool { - liveMu.Lock() - defer liveMu.Unlock() - return activeProg != nil +// liveView is the running live program and the console that started it. +type liveView struct { + owner *console + prog *tea.Program } +// The running live view, for PrintAbove and the owning console's output. +// Only one live view runs at a time; a widget that cannot claim the slot +// (a nested widget, or a second console at the same moment) prints plain +// lines. +var ( + liveMu sync.Mutex + activeView *liveView +) + +// errLivePanic is the result the final frame shows when work panicked. +var errLivePanic = errors.New("bonfire: live view work panicked") + // runLive runs model as a bubbletea program on out while work runs on the -// calling goroutine. work gets a send function for progress messages. When -// work returns, the program is unregistered (so PrintAbove stops sending to -// it), told to finish and waited for, all under liveMu; it never receives -// lines after that. -func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, work func(send func(tea.Msg)) error) error { +// calling goroutine, with c as the owner of the view. It first claims the +// single live slot under liveMu; when another view holds it, runLive +// returns started false without running work, and the caller prints plain +// lines instead. work gets a send function for progress messages. When +// work returns or panics, the view is unregistered (so PrintAbove stops +// sending to it), told to finish and waited for, all under liveMu; it +// never receives lines after that, and a panic goes on after the cleanup. +func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, work func(send func(tea.Msg)) error) (started bool, err error) { if ctx == nil { ctx = context.Background() } @@ -344,38 +352,61 @@ func runLive(ctx context.Context, out io.Writer, c *console, model tea.Model, wo tea.WithContext(ctx), tea.WithColorProfile(c.profile()), ) + view := &liveView{owner: c, prog: prog} + liveMu.Lock() + if activeView != nil { + liveMu.Unlock() + return false, nil + } + activeView = view + liveMu.Unlock() + finished := make(chan struct{}) go func() { defer close(finished) _, _ = prog.Run() }() - liveMu.Lock() - activeProg = prog - liveMu.Unlock() - err := work(prog.Send) - - // Unregister, finish and wait as one step: a printer that arrives - // meanwhile waits for the lock and then writes after the final frame, - // so no line reaches a stopped program or races the last frame. - liveMu.Lock() - activeProg = nil - prog.Send(finishMsg{err: err}) - <-finished - liveMu.Unlock() - return err + completed := false + defer func() { + result := err + if !completed { + result = errLivePanic + } + // Unregister, finish and wait as one step: a printer that arrives + // meanwhile waits for the lock and then writes after the final + // frame, so no line reaches a stopped program or races the last + // frame. + liveMu.Lock() + if activeView == view { + activeView = nil + } + prog.Send(finishMsg{err: result}) + <-finished + liveMu.Unlock() + }() + err = work(prog.Send) + completed = true + return true, err } -// printAboveLines prints s, one or more lines, above the running live view. -// It reports false when no view runs. +// printAboveLines prints s, one or more lines, above the running live view, +// whichever console started it (bonfire.PrintAbove). It reports false when +// no view runs. func printAboveLines(s string) bool { + return printAbove(nil, s) +} + +// printAbove prints s above the running live view when owner is nil or is +// the console that started the view, and reports whether it did. +func printAbove(owner *console, s string) bool { liveMu.Lock() defer liveMu.Unlock() - if activeProg == nil { + if activeView == nil || (owner != nil && activeView.owner != owner) { return false } // One message for the whole block, sent while holding liveMu: blocks // from different goroutines keep the order of their calls. - activeProg.Send(printLine(strings.TrimSuffix(s, "\n"))) + activeView.prog.Send(printLine(strings.TrimSuffix(s, "\n"))) return true } diff --git a/modules/bonfire/live_test.go b/modules/bonfire/live_test.go index 8ede2c09..f0f9df90 100644 --- a/modules/bonfire/live_test.go +++ b/modules/bonfire/live_test.go @@ -427,3 +427,106 @@ func TestPrintAboveOrderedNoLoss(t *testing.T) { } }) } + +// TestPrintlnStaysWithItsConsole (WR-02): while a view runs on console A, +// another console's Println, Printf and Info go to its own writer; A's own +// Println still prints above its view. +func TestPrintlnStaysWithItsConsole(t *testing.T) { + bufA, bufB := &lockedBuffer{}, &lockedBuffer{} + a := newConsole(strings.NewReader(""), bufA, io.Discard, noEnv, boolPtr(true), boolPtr(true)) + b := newConsole(strings.NewReader(""), bufB, bufB, noEnv, boolPtr(false), boolPtr(false)) + err := a.Spinner("working", func() error { + b.Println("b-println") + b.Printf("b-printf %d\n", 1) + b.Info("b-info") + a.Println("a-println") + return nil + }) + if err != nil { + t.Fatal(err) + } + gotA, gotB := ansi.Strip(bufA.String()), ansi.Strip(bufB.String()) + for _, want := range []string{"b-println", "b-printf 1", "b-info"} { + if !strings.Contains(gotB, want) { + t.Errorf("console B's writer lacks %q:\n%s", want, gotB) + } + if strings.Contains(gotA, want) { + t.Errorf("console B's %q printed above console A's view:\n%s", want, gotA) + } + } + if !strings.Contains(gotA, "a-println") { + t.Errorf("console A's own line missing above its view:\n%s", gotA) + } +} + +// TestLiveViewSingleOwner (WR-02): two consoles starting a spinner at the +// same moment never both run a live view; the one that loses prints its +// plain line to its own writer. +func TestLiveViewSingleOwner(t *testing.T) { + const rounds = 200 + for round := range rounds { + bufs := [2]*lockedBuffer{{}, {}} + var start, entered sync.WaitGroup + start.Add(1) + entered.Add(2) + var wg sync.WaitGroup + for i := range 2 { + wg.Add(1) + go func() { + defer wg.Done() + c := newConsole(strings.NewReader(""), bufs[i], io.Discard, noEnv, boolPtr(true), boolPtr(true)) + start.Wait() + _ = c.Spinner(fmt.Sprintf("label-%d", i), func() error { + entered.Done() + entered.Wait() // both work functions run at once + return nil + }) + }() + } + start.Done() + wg.Wait() + plain := 0 + for i, b := range bufs { + if strings.Contains(ansi.Strip(b.String()), fmt.Sprintf("[...] label-%d", i)) { + plain++ + } + } + if plain != 1 { + t.Fatalf("round %d: %d consoles printed their plain line to their own writer, want 1:\nA: %q\nB: %q", + round, plain, ansi.Strip(bufs[0].String()), ansi.Strip(bufs[1].String())) + } + if PrintAbove("after the round") { + t.Fatalf("round %d: a view is still registered", round) + } + } +} + +// TestLiveViewPanicUnregisters (WR-02): a panic in the work function still +// ends and unregisters the view; the next widget runs as a live view. +func TestLiveViewPanicUnregisters(t *testing.T) { + out := &lockedBuffer{} + c := newConsole(strings.NewReader(""), out, io.Discard, noEnv, boolPtr(true), boolPtr(true)) + func() { + defer func() { + if r := recover(); r != "boom" { + t.Fatalf("recovered %v, want the work function's panic", r) + } + }() + _ = c.Spinner("panicking", func() error { panic("boom") }) + }() + if PrintAbove("after the panic") { + t.Fatal("the panicked view is still registered") + } + next := &lockedBuffer{} + c2 := newConsole(strings.NewReader(""), next, io.Discard, noEnv, boolPtr(true), boolPtr(true)) + var during bool + if err := c2.Spinner("next", func() error { + during = PrintAbove("above the next view") + return nil + }); err != nil { + t.Fatal(err) + } + if !during || strings.Contains(ansi.Strip(next.String()), "[...] next") { + t.Errorf("the next spinner did not run as a live view:\n%s", ansi.Strip(next.String())) + } +} diff --git a/modules/bonfire/output.go b/modules/bonfire/output.go index 5b098876..8845b545 100644 --- a/modules/bonfire/output.go +++ b/modules/bonfire/output.go @@ -128,10 +128,11 @@ func (c *console) setGlobals(g Globals) { // live reports whether widgets animate: stdout is a terminal and none of // -q, --no-ansi and -n is set (D-03). A live view that is already running -// makes nested widgets print plain lines. +// makes nested widgets print plain lines: runLive then reports that it did +// not start. func (c *console) live() bool { g := c.globals - return c.tty && !g.Quiet && g.Color != ColorNever && !g.NoInteraction && !liveActive() + return c.tty && !g.Quiet && g.Color != ColorNever && !g.NoInteraction } // canPrompt reports whether prompts may wait for input from a person: @@ -170,13 +171,14 @@ func (c *console) styled(w io.Writer) io.Writer { return &colorprofile.Writer{Forward: w, Profile: c.profile()} } -// emit writes s, a styled line or block, to w. While a live view runs on -// stdout, lines for stdout print above it instead of tearing it. +// emit writes s, a styled line or block, to w. While a live view this +// console started runs on stdout, lines for stdout print above it instead +// of tearing it. func (c *console) emit(w io.Writer, s string) { if w == nil { return } - if w == c.out && printAboveLines(s) { + if w == c.out && printAbove(c, s) { return } _, _ = io.WriteString(c.styled(w), s) @@ -194,7 +196,7 @@ func (c *console) Println(a ...any) { return } s := fmt.Sprintln(a...) - if printAboveLines(s) { + if printAbove(c, s) { return } _, _ = io.WriteString(c.out, s) @@ -205,7 +207,7 @@ func (c *console) Printf(format string, a ...any) { return } s := fmt.Sprintf(format, a...) - if strings.HasSuffix(s, "\n") && printAboveLines(s) { + if strings.HasSuffix(s, "\n") && printAbove(c, s) { return } _, _ = io.WriteString(c.out, s) diff --git a/modules/bonfire/widgets.go b/modules/bonfire/widgets.go index 54455e9c..293267ee 100644 --- a/modules/bonfire/widgets.go +++ b/modules/bonfire/widgets.go @@ -26,13 +26,16 @@ func (c *console) Spinner(message string, fn func() error) error { if c.globals.Quiet { return fn() } - if !c.live() { - c.Printf("[...] %s\n", message) - return fn() + if c.live() { + started, err := runLive(context.Background(), c.out, c, newSpinnerModel(c.palette, message), func(func(tea.Msg)) error { + return fn() + }) + if started { + return err + } } - return runLive(context.Background(), c.out, c, newSpinnerModel(c.palette, message), func(func(tea.Msg)) error { - return fn() - }) + c.Printf("[...] %s\n", message) + return fn() } func (c *console) Progress(total int, fn func(Progress) error) error { @@ -45,12 +48,15 @@ func (c *console) Progress(total int, fn func(Progress) error) error { if c.globals.Quiet { return fn(&quietProgress{c: c}) } - if !c.live() { - return fn(&progressBar{c: c, total: total, lastPct: -1}) + if c.live() { + started, err := runLive(context.Background(), c.out, c, newProgressModel(c.palette, total), func(send func(tea.Msg)) error { + return fn(&liveProgress{send: send, total: total}) + }) + if started { + return err + } } - return runLive(context.Background(), c.out, c, newProgressModel(c.palette, total), func(send func(tea.Msg)) error { - return fn(&liveProgress{send: send, total: total}) - }) + return fn(&progressBar{c: c, total: total, lastPct: -1}) } // progressBar prints a line at every tenth of the work when widgets do not