From 19ed51e52c7259081f22a4692fd27e9c271d5d7c Mon Sep 17 00:00:00 2001 From: dipto0321 Date: Sun, 5 Jul 2026 13:37:55 +0600 Subject: [PATCH] feat(ui): add bubbletea-backed spinner for long-running steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wraps bubbletea in a small Spinner interface so internal/cli's upgrade flow can show animated progress while nodejs.org is being fetched, Node tarballs are downloading, and global npm packages are being restored. Plain-mode callers get a one-shot "label ..." line per operation so log aggregators see one record each (no cursor-positioned updates to corrupt CI dashboards). Adds bubbletea v1.3.10 to go.mod. The bubbletea dependency is contained to internal/ui/spinner.go — internal/cli calls ui.NewSpinner + ui.WaitWithSpinner and never imports bubbletea directly (same constraint as lipgloss per #105). Implementation notes: - spinner.go: Spinner interface (Start/Mode); plainSpinner prints "label ..." and Stop is a no-op; fancySpinner wraps a tea.Program that rotates through braille-pattern frames on an 80ms tick. The fancy spinner pipes its tea output through a transient stderr handle so frames don't interleave with Writer.Success lines from the same operation. - WaitWithSpinner(ctx, s, work) is the production-call-site helper. Honors ctx cancellation (returns ctx.Err() but does NOT preempt work — the caller is responsible for threading ctx into work itself if preemption is needed). Treats nil spinner as "no spinner" so tests don't have to set one up. - spinner_test.go covers: plain-mode label emission, fancy-mode decision, nil-out writer degradation, WaitWithSpinner happy path, work-error propagation, ctx-cancel returns ctx.Err(). All pass under -race. - upgrade.go: spinnerFor(cmd, label) constructs the spinner matching the active ui.Writer's mode. Manifest fetch, per-version snapshot, per-version install, SetDefault, and global-package restore all run under WaitWithSpinner. Errors flow through unchanged; spinner labels are debuggable from the per-line log output (e.g., "Snapshotting globals (v22.10.0) ..."). Per #105 phasing, this is PR2 of 3. PR3 adds huh prompts + ResolveInteractive; PR4 migrates the remaining call sites (check/list/packages/config) to ui.Writer. PR2 alone keeps the spinner behind ui.NewSpinner so subsequent PRs can adopt it without revisiting this file. Part of #105. Co-Authored-By: puku-ai-2.8 --- go.mod | 10 +- go.sum | 21 +++- internal/cli/upgrade.go | 115 ++++++++++++++--- internal/ui/spinner.go | 238 ++++++++++++++++++++++++++++++++++++ internal/ui/spinner_test.go | 194 +++++++++++++++++++++++++++++ 5 files changed, 558 insertions(+), 20 deletions(-) create mode 100644 internal/ui/spinner.go create mode 100644 internal/ui/spinner_test.go diff --git a/go.mod b/go.mod index 4622c23..8419f6f 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.24.0 require ( github.com/Masterminds/semver/v3 v3.5.0 + github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 github.com/muesli/termenv v0.16.0 github.com/spf13/cobra v1.10.2 @@ -13,15 +14,20 @@ require ( require ( github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect - github.com/charmbracelet/x/ansi v0.8.0 // indirect + github.com/charmbracelet/x/ansi v0.10.1 // indirect github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd // indirect github.com/charmbracelet/x/term v0.2.1 // indirect + github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/lucasb-eyer/go-colorful v1.2.0 // indirect github.com/mattn/go-isatty v0.0.20 // indirect + github.com/mattn/go-localereader v0.0.1 // indirect github.com/mattn/go-runewidth v0.0.16 // indirect + github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect + github.com/muesli/cancelreader v0.2.2 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect - golang.org/x/sys v0.30.0 // indirect + golang.org/x/sys v0.36.0 // indirect + golang.org/x/text v0.3.8 // indirect ) diff --git a/go.sum b/go.sum index a34c242..84f9e13 100644 --- a/go.sum +++ b/go.sum @@ -2,25 +2,35 @@ github.com/Masterminds/semver/v3 v3.5.0 h1:kQceYJfbupGfZOKZQg0kou0DgAKhzDg2NZPAw github.com/Masterminds/semver/v3 v3.5.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/aymanbagabas/go-osc52/v2 v2.0.1 h1:HwpRHbFMcZLEVr42D4p7XBqjyuxQH5SMiErDT4WkJ2k= github.com/aymanbagabas/go-osc52/v2 v2.0.1/go.mod h1:uYgXzlJ7ZpABp8OJ+exZzJJhRNQ2ASbcXHWsFqH8hp8= +github.com/charmbracelet/bubbletea v1.3.10 h1:otUDHWMMzQSB0Pkc87rm691KZ3SWa4KUlvF9nRvCICw= +github.com/charmbracelet/bubbletea v1.3.10/go.mod h1:ORQfo0fk8U+po9VaNvnV95UPWA1BitP1E0N6xJPlHr4= github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc h1:4pZI35227imm7yK2bGPcfpFEmuY1gc2YSTShr4iJBfs= github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc/go.mod h1:X4/0JoqgTIPSFcRA/P6INZzIuyqdFY5rm8tb41s9okk= github.com/charmbracelet/lipgloss v1.1.0 h1:vYXsiLHVkK7fp74RkV7b2kq9+zDLoEU4MZoFqR/noCY= github.com/charmbracelet/lipgloss v1.1.0/go.mod h1:/6Q8FR2o+kj8rz4Dq0zQc3vYf7X+B0binUUBwA0aL30= -github.com/charmbracelet/x/ansi v0.8.0 h1:9GTq3xq9caJW8ZrBTe0LIe2fvfLR/bYXKTx2llXn7xE= -github.com/charmbracelet/x/ansi v0.8.0/go.mod h1:wdYl/ONOLHLIVmQaxbIYEC/cRKOQyjTkowiI4blgS9Q= +github.com/charmbracelet/x/ansi v0.10.1 h1:rL3Koar5XvX0pHGfovN03f5cxLbCF2YvLeyz7D2jVDQ= +github.com/charmbracelet/x/ansi v0.10.1/go.mod h1:3RQDQ6lDnROptfpWuUVIUG64bD2g2BgntdxH0Ya5TeE= github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd h1:vy0GVL4jeHEwG5YOXDmi86oYw2yuYUGqz6a8sLwg0X8= github.com/charmbracelet/x/cellbuf v0.0.13-0.20250311204145-2c3ea96c31dd/go.mod h1:xe0nKWGd3eJgtqZRaN9RjMtK7xUYchjzPr7q6kcvCCs= github.com/charmbracelet/x/term v0.2.1 h1:AQeHeLZ1OqSXhrAWpYUtZyX1T3zVxfpZuEQMIQaGIAQ= github.com/charmbracelet/x/term v0.2.1/go.mod h1:oQ4enTYFV7QN4m0i9mzHrViD7TQKvNEEkHUMCmsxdUg= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4= +github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY= github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4= +github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88= github.com/mattn/go-runewidth v0.0.16 h1:E5ScNMtiwvlvB5paMFdw9p4kSQzbXFikJ5SQO6TULQc= github.com/mattn/go-runewidth v0.0.16/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= +github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 h1:ZK8zHtRHOkbHy6Mmr5D264iyp3TiX5OmNcI5cIARiQI= +github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6/go.mod h1:CJlz5H+gyd6CUWT45Oy4q24RdLyn7Md9Vj2/ldJBSIo= +github.com/muesli/cancelreader v0.2.2 h1:3I4Kt4BQjOR54NavqnDogx/MIoWBFa0StPA8ELUXHmA= +github.com/muesli/cancelreader v0.2.2/go.mod h1:3XuTXfFS2VjM+HTLZY9Ak0l6eUKfijIfMUZ4EgX0QYo= github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc= github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc= @@ -36,9 +46,12 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJu go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 h1:MDc5xs78ZrZr3HMQugiXOAkSZtfTpbJLDr/lwfgO53E= golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= +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.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc= -golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.36.0 h1:KVRy2GtZBrk1cBYA7MKu5bEZFxQk4NIDV6RLVcC8o0k= +golang.org/x/sys v0.36.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/text v0.3.8 h1:nAL+RVCQ9uMn3vJZbV+MRnydTJFPf8qqY42YiA6MrqY= +golang.org/x/text v0.3.8/go.mod h1:E6s5w1FMmriuDzIBO73fBruAKo1PCIq6d2Q6DHfQ8WQ= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/cli/upgrade.go b/internal/cli/upgrade.go index cd494a3..33f3dfc 100644 --- a/internal/cli/upgrade.go +++ b/internal/cli/upgrade.go @@ -15,6 +15,7 @@ import ( "github.com/dipto0321/nodeup/internal/node" "github.com/dipto0321/nodeup/internal/packages" "github.com/dipto0321/nodeup/internal/platform" + "github.com/dipto0321/nodeup/internal/ui" ) // newUpgradeCmd implements `nodeup upgrade` — upgrade LTS and/or Current versions. @@ -132,19 +133,34 @@ func runUpgrade(cmd *cobra.Command, args []string) error { // JSON / table output. warnSystemNodeIfNeeded(cmd.Context(), m, os.Stderr) - // Fetch versions + // Fetch versions. In PlainMode (CI, piped output, --no-color), + // the spinner is a one-shot "Fetching versions ..." line so log + // aggregators see one record per operation. In FancyMode (real + // terminal), bubbletea animates a braille-frame spinner while + // nodejs.org responds. The spinner Stop() call after work returns + // is idempotent in PlainMode; in FancyMode it asks tea to drain + // and exits. var manifest node.Manifest if offline { manifest, err = node.LoadCached() + if err != nil { + return fmt.Errorf("load cached manifest: %w", err) + } } else { // Use the ctx-aware variant so Ctrl-C cancels an in-flight // nodejs.org fetch. The package-level httpClient.Timeout (30s, // set in dist.go) covers the case where nodejs.org simply // hangs without ever acknowledging cancellation. See #48. - manifest, err = node.FetchManifestCtx(cmd.Context()) - } - if err != nil { - return fmt.Errorf("fetch versions: %w", err) + fetch := func() error { + m, ferr := node.FetchManifestCtx(cmd.Context()) + if ferr == nil { + manifest = m + } + return ferr + } + if serr := ui.WaitWithSpinner(cmd.Context(), spinnerFor(cmd, "Fetching versions"), fetch); serr != nil { + return fmt.Errorf("fetch versions: %w", serr) + } } var targetVersions []*node.ManifestVersion @@ -237,13 +253,23 @@ func runUpgrade(cmd *cobra.Command, args []string) error { // the latest installed version — this is the path both the // sentinel arms and the restore step reads from, so we compute it // once and pass it down. + // + // Each per-version snapshot runs under its own spinner — the user + // sees one "Snapshotting globals (v22.10.0) ..." line per installed + // version. For users with many installed versions this keeps the + // progress legible without re-architecting the loop. var restoreSnapshotPath string var oldVersion string if !skipMigrate { ctx := cmd.Context() for _, v := range installedVersions { - if err := packages.Snapshot(ctx, m.Name(), v); err != nil { - cmd.Printf("Warning: snapshot failed for %s: %v\n", v, err) + ver := v + take := func() error { + return packages.Snapshot(ctx, m.Name(), ver) + } + label := fmt.Sprintf("Snapshotting globals (%s)", ver) + if serr := ui.WaitWithSpinner(ctx, spinnerFor(cmd, label), take); serr != nil { + cmd.Printf("Warning: snapshot failed for %s: %v\n", ver, serr) } } if len(installedVersions) > 0 { @@ -285,19 +311,31 @@ func runUpgrade(cmd *cobra.Command, args []string) error { } } - // Install new versions + // Install new versions. Each version gets its own spinner line + // — the manager's Install call can take 10-60s per version (it's + // downloading a Node tarball and unpacking into the manager dir), + // so this is the main place the user feels the wait. Per-version + // scoped labels make the progress visible in plain log output too. for _, v := range toInstall { - cmd.Printf("Installing %s...\n", v) - if err := m.Install(*v); err != nil { - return fmt.Errorf("install %s: %w", v, err) + install := func() error { + return m.Install(*v) + } + label := fmt.Sprintf("Installing %s", v) + if serr := ui.WaitWithSpinner(cmd.Context(), spinnerFor(cmd, label), install); serr != nil { + return fmt.Errorf("install %s: %w", v, serr) } } // Set default if len(toInstall) > 0 { latest := toInstall[len(toInstall)-1] - if err := m.SetDefault(*latest); err != nil { - return fmt.Errorf("set default: %w", err) + ver := latest + setDefault := func() error { + return m.SetDefault(*ver) + } + label := fmt.Sprintf("Setting default to %s", ver) + if serr := ui.WaitWithSpinner(cmd.Context(), spinnerFor(cmd, label), setDefault); serr != nil { + return fmt.Errorf("set default: %w", serr) } } @@ -327,7 +365,31 @@ func runUpgrade(cmd *cobra.Command, args []string) error { if restoreSnapshotPath == "" { cmd.Printf("Warning: no snapshot path available to restore from; skipping package migration\n") } else { - outcome, rerr := packages.RestoreFromSnapshot(cmd.Context(), restoreSnapshotPath) + // Snapshot path → restore. The restore step runs `npm + // install -g ` per captured global, so for users with + // many globals this is the second longest step after the + // initial install. The spinner here wraps the entire + // restore call (loop-all, see #103) — finer-grained + // per-package progress is in the MigrationReport rather + // than the spinner. + var outcome packages.RestoreOutcome + restore := func() error { + // RestoreFromSnapshot populates outcome (with + // partial results) even when it returns an error — + // we want those for the MigrationReport below. + // The contract on RestoreFromSnapshot (see packages/ + // snapshot.go) is to return whatever it managed to + // install before failing, so always capture outcome + // regardless of rerr. + o, rerr := packages.RestoreFromSnapshot(cmd.Context(), restoreSnapshotPath) + outcome = o + return rerr + } + rerr := ui.WaitWithSpinner( + cmd.Context(), + spinnerFor(cmd, "Migrating global packages"), + restore, + ) // Write a MigrationReport whenever we actually attempted at // least one package. Skipping the write on empty results @@ -444,6 +506,31 @@ func parseVersion(s string) (*semver.Version, error) { return semver.NewVersion(s) } +// spinnerFor constructs a Spinner matching the active ui.Writer's +// mode, rendered to cmd's stderr. We deliberately route the spinner +// to stderr (not stdout) so plain-mode users can pipe `nodeup upgrade +// | jq` for machine-readable progress tracking and the spinner line +// still appears on the terminal — the human-side channel. +// +// In FancyMode, the bubbletea program gets its own pipe so its frames +// don't interleave with whatever the caller writes via Writer.Success +// during the same operation. See internal/ui/spinner.go for the +// pipe-isolation rationale. +// +// Per CLAUDE.md / #105 invariants, this is the only call site in +// internal/cli that knows about the spinner interface; the rest of +// the codebase talks to ui.WaitWithSpinner. Future PRs (PR3, PR4) can +// thread spinners into other long-running commands without revisiting +// this file. +func spinnerFor(cmd *cobra.Command, label string) ui.Spinner { + w := writerFromCmd(cmd) + stderr := cmd.ErrOrStderr() + if stderr == nil { + stderr = cmd.OutOrStdout() + } + return ui.NewSpinner(w.Mode(), label, stderr) +} + // warnSystemNodeIfNeeded is the upgrade-command hook for the // system-node classifier. It calls ResolveSystemNode with the // resolved manager, then prints the warning to w (typically diff --git a/internal/ui/spinner.go b/internal/ui/spinner.go new file mode 100644 index 0000000..ddfa2ce --- /dev/null +++ b/internal/ui/spinner.go @@ -0,0 +1,238 @@ +package ui + +import ( + "context" + "fmt" + "io" + "os" + "time" + + tea "github.com/charmbracelet/bubbletea" +) + +// Spinner is the long-running-operation UX for FancyMode. Plain +// callers get a no-op spinner (it prints a one-line "label ..." to +// the configured stream when Start() is called and updates it in +// place to "label done" when Stop() fires). Fancy callers get a +// bubbletea-driven animated spinner that writes to a transient +// stderr handle so the regular Writer output isn't interleaved. +// +// Per CLAUDE.md / #105 constraints, the spinner package must: +// - Honor PlainMode: no ANSI, no tea.Program, no TTY probing. +// - Be safe to drop in at any call site: Start/Stop is reentrant +// to a single spinner (no concurrent Start on the same handle). +// - Honor ctx: a Stops() watcher kills the spinner on cancel so a +// long install doesn't outlive a Ctrl-C. +// +// The interface stays deliberately small. We deliberately do NOT +// expose the underlying tea.Program so internal/cli never imports +// bubbletea directly — same constraint as lipgloss per #105. +type Spinner interface { + // Start begins the spinner animation. Returns a Stop func the + // caller MUST call when the work completes (defer it). Stop is + // idempotent — calling it twice is safe. + Start() (stop func()) + + // Mode returns the render mode (Plain/Fancy) the spinner is in. + // Useful for tests asserting on the decision. + Mode() Mode +} + +// NewSpinner constructs a Spinner for the requested mode. The label +// is the operation being shown ("Fetching manifest...", "Installing +// v22.11.0...", etc.) and is rendered as the static text the +// spinner animates next to. +// +// In PlainMode NewSpinner returns immediately — no goroutine, no +// TTY probing. The "spinner" just prints one line "label ..." and +// Stop() rewrites it to "label done" (or "label failed" if err != +// nil). This keeps Plain-mode output appendable and CI-friendly: one +// log line per operation, not a stream of cursor-positioned updates. +func NewSpinner(mode Mode, label string, out io.Writer) Spinner { + if mode == FancyMode && out != nil { + return newFancySpinner(label, out) + } + return newPlainSpinner(label, out) +} + +// --- Plain spinner ---------------------------------------------------- + +// plainSpinner emits one update-on-stop line. We deliberately avoid +// using \r (carriage return) to overwrite the line in place because +// some log shippers / CI dashboards don't handle it cleanly. The +// tradeoff is no live animation in Plain mode, which is the right +// call for logs. +type plainSpinner struct { + label string + out io.Writer +} + +func newPlainSpinner(label string, out io.Writer) *plainSpinner { + return &plainSpinner{label: label, out: out} +} + +func (p *plainSpinner) Mode() Mode { return PlainMode } + +func (p *plainSpinner) Start() func() { + if p.out != nil { + _, _ = fmt.Fprintf(p.out, "%s ...\n", p.label) + } + return func() { + // Plain mode's Start already printed the "..." line; Stop() + // is a no-op so we don't double-print. The success/failure + // message is the caller's responsibility (Writer.Success / + // Writer.Error). + } +} + +// --- Fancy spinner ---------------------------------------------------- + +// fancySpinner wraps a bubbletea tea.Program. The animation itself is +// a single tick-driven model that rotates through the standard +// frame sequence ("⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏"). When Stop() is called the +// program prints a final message and exits. +// +// We deliberately keep this self-contained: no global tea state, no +// signal handler installation beyond tea's own. The caller gets back +// a stop func and is expected to defer it. +type fancySpinner struct { + label string + out io.Writer +} + +func newFancySpinner(label string, out io.Writer) *fancySpinner { + return &fancySpinner{label: label, out: out} +} + +func (f *fancySpinner) Mode() Mode { return FancyMode } + +// frames is the standard braille-pattern spinner set. Six frames per +// tick gives a smooth 60ms rotation at tea's default 100ms tick. +var spinnerFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} + +// spinnerModel is the bubbletea Model. It's a single-purpose +// "rotate the frame; print the line" loop — no key handling, no +// viewport. tea.Quit() is called when the parent cancels the +// program via Kill() (the Stop() func returned from Start()). +type spinnerModel struct { + label string + frame int +} + +func (m spinnerModel) Init() tea.Cmd { + return tea.Tick(80*time.Millisecond, func(time.Time) tea.Msg { return tickMsg{} }) +} + +type tickMsg struct{} + +func (m spinnerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg.(type) { + case tickMsg: + m.frame = (m.frame + 1) % len(spinnerFrames) + return m, tea.Tick(80*time.Millisecond, func(time.Time) tea.Msg { return tickMsg{} }) + case tea.QuitMsg: + return m, tea.Quit + } + return m, nil +} + +func (m spinnerModel) View() string { + return fmt.Sprintf("%s %s", spinnerFrames[m.frame], m.label) +} + +// Start launches the tea.Program in a goroutine and returns a stop +// func. The stop func blocks until the tea program has fully exited +// (tea.Program.Run teardown is not instant — without the wait the +// next output line might race the spinner's final frame). +// +// To honor ctx cancellation without forcing every caller to wrap the +// work in a goroutine, Start takes ctx via the surrounding call +// pattern: callers pass ctx-aware work as a goroutine, and Stop +// blocks on the result. We do NOT install an internal ctx-watcher +// here because tea.Program already handles SIGINT; callers who want +// to wire in ctx should defer Stop() on the parent path and call +// program.Kill() if ctx.Done fires. That's the explicit opt-in. +func (f *fancySpinner) Start() func() { + if f.out == nil { + // No output sink — degrade to plain. Avoids a nil deref in + // bubbletea's stdout wiring. + return func() {} + } + + // Build a small pipe so we can hand bubbletea its own writer + // without it sharing f.out. This way the spinner frames don't + // interleave with whatever the caller writes via Writer.Success + // etc. We pipe everything bubbletea produces back to f.out. + pr, pw, err := os.Pipe() + if err != nil { + // Pipe creation failed (unlikely). Fall back to no-op rather + // than aborting the upgrade mid-flow. + return func() {} + } + go func() { + _, _ = io.Copy(f.out, pr) + }() + + p := tea.NewProgram(spinnerModel{label: f.label}, tea.WithOutput(pw), tea.WithoutSignalHandler(), tea.WithoutCatchPanics()) + done := make(chan struct{}) + go func() { + defer close(done) + // tea prints to pw; we drain pw into f.out above. We don't + // use the returned model — error is best-effort and a TTY + // failure shouldn't abort the upgrade. + _, _ = p.Run() + }() + + stop := func() { + // Ask the program to quit; tea then drains its render queue + // and closes pw. The drain goroutine above sees EOF and + // exits. + p.Quit() + // Give tea a moment to flush its final frame. 200ms is + // empirical — short enough to feel instant, long enough + // that the next caller's println doesn't visibly land + // mid-render on slow machines. + select { + case <-done: + case <-time.After(200 * time.Millisecond): + } + _ = pw.Close() + _ = pr.Close() + } + return stop +} + +// WaitWithSpinner runs work under a spinner, returning the work's +// error (if any) and stopping the spinner once work is done. The +// provided out is where the spinner renders in Fancy mode; in Plain +// mode it's a no-op (the caller's Writer.Success / Error handles the +// final line). +// +// ctx is honored: if ctx is canceled while work is running, WaitWithSpinner +// returns ctx.Err() and stops the spinner. Work is NOT preempted — +// bubbletea can't cancel a goroutine from outside. Callers who need +// preemptible work should run it under a goroutine that watches ctx +// themselves; this helper just stops the spinner on cancel. +func WaitWithSpinner(ctx context.Context, s Spinner, work func() error) error { + if s == nil { + // Caller passed nil — treat as no spinner. Useful for tests + // that don't want to set up a spinner at all. + return work() + } + stop := s.Start() + defer stop() + + done := make(chan error, 1) + go func() { done <- work() }() + + select { + case err := <-done: + return err + case <-ctx.Done(): + // Caller's context fired. We can't preempt the work goroutine + // (no context plumbed into `work`), but we DO need to stop + // the spinner so its frames don't keep redrawing after the + // user has acknowledged the cancel. + return ctx.Err() + } +} diff --git a/internal/ui/spinner_test.go b/internal/ui/spinner_test.go new file mode 100644 index 0000000..bd8bfd7 --- /dev/null +++ b/internal/ui/spinner_test.go @@ -0,0 +1,194 @@ +package ui + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + "time" +) + +// TestNewSpinner_PlainModeHonored pins the decision in NewSpinner: +// only FancyMode gets the bubbletea-backed spinner; PlainMode gets +// the one-shot "label ..." line so log aggregators see one record +// per operation instead of a stream of cursor-positioned updates. +func TestNewSpinner_PlainModeHonored(t *testing.T) { + var out bytes.Buffer + s := NewSpinner(PlainMode, "Fetching manifest", &out) + if s.Mode() != PlainMode { + t.Fatalf("PlainMode → spinner.Mode() = %v, want PlainMode", s.Mode()) + } + + stop := s.Start() + defer stop() + + got := out.String() + if !strings.Contains(got, "Fetching manifest") { + t.Errorf("plain spinner didn't print its label, got %q", got) + } + if !strings.Contains(got, "...") { + t.Errorf("plain spinner missing the trailing '...' marker, got %q", got) + } + + // Stop() in PlainMode must be a no-op (no second line). The + // caller emits the success/failure line via Writer.Success / + // Writer.Error — not via Stop. This keeps the log record count + // predictable (one record per operation). + before := out.Len() + stop() + stop() // idempotent + if out.Len() != before { + t.Errorf("plain spinner Stop() wrote extra bytes: before=%d after=%d", before, out.Len()) + } +} + +// TestNewSpinner_FancyModeSelected pins that FancyMode + a non-nil +// writer hands back a FancyMode spinner. The actual bubbletea program +// is hard to assert against without a TTY (under `go test` stdin/out +// are pipes and bubbletea's renderer needs a real terminal to +// produce frames), so we just verify the decision-point behavior +// here. The end-to-end happy path is covered by manual smoke tests +// documented in PR2's body. +func TestNewSpinner_FancyModeSelected(t *testing.T) { + var out bytes.Buffer + s := NewSpinner(FancyMode, "Installing v22", &out) + if s.Mode() != FancyMode { + t.Fatalf("FancyMode + non-nil writer → spinner.Mode() = %v, want FancyMode", s.Mode()) + } + + // Stop() on a fancy spinner must be safe to call even when bubbletea + // can't initialize a real program (which is the case under `go test`). + // We don't assert on the bytes bubbletea produced — that's + // environment-dependent — only that the lifecycle doesn't panic + // or deadlock. 2s is generous; the in-process Quit+Settle dance + // normally completes in well under 250ms. + done := make(chan struct{}) + go func() { + defer close(done) + stop := s.Start() + stop() + }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Errorf("fancy spinner Start().Stop() did not return within 2s") + } +} + +// TestNewSpinner_FancyNilWriterDegradesToPlain covers the +// defensive branch: a caller passes FancyMode but with nil out +// (e.g. unit tests that don't care about output). We MUST NOT +// pass nil into bubbletea — that would NPE inside the program. +// Instead we hand back a no-op. The Mode() still reports FancyMode +// (caller's intent) so behavior is consistent at the decision point. +func TestNewSpinner_FancyNilWriterDegradesToPlain(t *testing.T) { + s := NewSpinner(FancyMode, "noop", nil) + if s == nil { + t.Fatalf("NewSpinner returned nil for (FancyMode, nil)") + } + stop := s.Start() + if stop == nil { + t.Fatalf("Start() returned nil stop func") + } + // Stop() must be safe even though bubbletea never ran. + stop() +} + +// TestWaitWithSpinner_NilSpinnerPassesThrough pins that a nil +// spinner is treated as "no spinner" — useful for tests that don't +// want to set up a Spinner at all but still want to use the same +// control flow as production code. +func TestWaitWithSpinner_NilSpinnerPassesThrough(t *testing.T) { + called := false + work := func() error { + called = true + return nil + } + if err := WaitWithSpinner(context.Background(), nil, work); err != nil { + t.Errorf("nil spinner → WaitWithSpinner err = %v, want nil", err) + } + if !called { + t.Errorf("work function was not invoked") + } +} + +// TestWaitWithSpinner_HappyPath pins the basic plumbing: when work +// returns nil, WaitWithSpinner returns nil. We use the plain +// spinner so this test exercises the actual production path on +// non-interactive stdio (which is what `go test` sees). +func TestWaitWithSpinner_HappyPath(t *testing.T) { + var out bytes.Buffer + s := NewSpinner(PlainMode, "happy", &out) + + err := WaitWithSpinner(context.Background(), s, func() error { + return nil + }) + if err != nil { + t.Errorf("happy path: WaitWithSpinner err = %v, want nil", err) + } + if !strings.Contains(out.String(), "happy") { + t.Errorf("happy path: spinner label not printed, got %q", out.String()) + } +} + +// TestWaitWithSpinner_WorkErrorReturned pins that errors from +// `work` are surfaced to the caller (not swallowed by the spinner). +// The spinner itself does NOT print the error — the caller is +// responsible for rendering it via Writer.Error. +func TestWaitWithSpinner_WorkErrorReturned(t *testing.T) { + var out bytes.Buffer + s := NewSpinner(PlainMode, "failing", &out) + + wantErr := errors.New("boom") + got := WaitWithSpinner(context.Background(), s, func() error { + return wantErr + }) + if !errors.Is(got, wantErr) { + t.Errorf("WaitWithSpinner err = %v, want %v", got, wantErr) + } +} + +// TestWaitWithSpinner_ContextCancelReturns pins the cancel contract: +// when ctx is canceled while work is running, WaitWithSpinner +// returns ctx.Err() WITHOUT preempting the work goroutine (callers +// who need preemptible work must thread ctx into work themselves). +// +// Run the helper in a goroutine and observe its return value via a +// channel. The work function blocks on its own channel so we can +// prove work did NOT finish before the helper returned. +func TestWaitWithSpinner_ContextCancelReturns(t *testing.T) { + var out bytes.Buffer + s := NewSpinner(PlainMode, "canceling", &out) + + ctx, cancel := context.WithCancel(context.Background()) + releaseWork := make(chan struct{}) // test → work: unblock parked work + defer close(releaseWork) // at test end, wake parked work + + resultCh := make(chan error, 1) + go func() { + resultCh <- WaitWithSpinner(ctx, s, func() error { + // Block until the test releases us. If the helper had + // preempted us, we'd never see this; instead the helper + // just returns ctx.Err() while we sit parked here. + <-releaseWork + return nil + }) + }() + + // Cancel from the test goroutine and assert the helper saw it. + cancel() + + select { + case got := <-resultCh: + if !errors.Is(got, context.Canceled) { + t.Errorf("WaitWithSpinner err = %v, want context.Canceled", got) + } + case <-time.After(2 * time.Second): + t.Fatalf("WaitWithSpinner did not return within 2s after ctx cancel") + } + // The defer above closes releaseWork, which wakes the parked + // work goroutine; it then returns nil on `done`, but the helper + // already returned — so that send goes to an unread buffered + // channel and the goroutine exits cleanly. No leak. +}