diff --git a/pkg/tui/components/spinner/spinner.go b/pkg/tui/components/spinner/spinner.go index ee24dcb43..81c68436a 100644 --- a/pkg/tui/components/spinner/spinner.go +++ b/pkg/tui/components/spinner/spinner.go @@ -19,8 +19,13 @@ const ( ModeSpinnerOnly ) -type Spinner struct { - animSub animation.Subscription // manages animation tick subscription +type Spinner interface { + layout.Model + Reset() Spinner + Stop() +} +type spinner struct { + animSub *animation.Subscription // manages animation tick subscription dotsStyle lipgloss.Style styledSpinnerFrames []string // pre-rendered spinner frames mode Mode @@ -69,7 +74,9 @@ func New(mode Mode, dotsStyle lipgloss.Style) Spinner { styledFrames[i] = dotsStyle.Render(char) } - return Spinner{ + sub := &animation.Subscription{} + return &spinner{ + animSub: sub, dotsStyle: dotsStyle, styledSpinnerFrames: styledFrames, mode: mode, @@ -79,11 +86,11 @@ func New(mode Mode, dotsStyle lipgloss.Style) Spinner { } } -func (s Spinner) Reset() Spinner { +func (s *spinner) Reset() Spinner { return New(s.mode, s.dotsStyle) } -func (s Spinner) Update(message tea.Msg) (layout.Model, tea.Cmd) { +func (s *spinner) Update(message tea.Msg) (layout.Model, tea.Cmd) { if msg, ok := message.(animation.TickMsg); ok { // Respond to global animation tick (all spinners advance together) s.frame = msg.Frame @@ -107,7 +114,7 @@ func (s Spinner) Update(message tea.Msg) (layout.Model, tea.Cmd) { return s, nil } -func (s Spinner) View() string { +func (s *spinner) View() string { spinner := s.styledSpinnerFrames[s.frame%len(s.styledSpinnerFrames)] if s.mode == ModeSpinnerOnly { return spinner @@ -115,17 +122,17 @@ func (s Spinner) View() string { return spinner + " " + s.renderMessage() } -func (s Spinner) SetSize(_, _ int) tea.Cmd { return nil } +func (s *spinner) SetSize(_, _ int) tea.Cmd { return nil } // Init registers the spinner with the animation coordinator. // If this is the first active animation, it starts the global tick. -func (s Spinner) Init() tea.Cmd { +func (s *spinner) Init() tea.Cmd { return s.animSub.Start() } // Stop unregisters the spinner from the animation coordinator. // Call this when the spinner is no longer active/visible. -func (s Spinner) Stop() { +func (s *spinner) Stop() { s.animSub.Stop() } @@ -139,7 +146,7 @@ var lightStyles = []lipgloss.Style{ styles.SpinnerTextDimmestStyle, } -func (s Spinner) renderMessage() string { +func (s *spinner) renderMessage() string { var out strings.Builder for i, char := range s.currentMessage { dist := min(max(i-s.lightPosition, s.lightPosition-i), len(lightStyles)-1) diff --git a/pkg/tui/components/spinner/spinner_test.go b/pkg/tui/components/spinner/spinner_test.go index f5f9bb4fa..59977b4c0 100644 --- a/pkg/tui/components/spinner/spinner_test.go +++ b/pkg/tui/components/spinner/spinner_test.go @@ -4,8 +4,23 @@ import ( "testing" "charm.land/lipgloss/v2" + "github.com/stretchr/testify/require" + + "github.com/docker/cagent/pkg/tui/animation" ) +func TestSpinnerCopyDoesNotLeakAnimationSubscription(t *testing.T) { + s1 := New(ModeSpinnerOnly, lipgloss.NewStyle()) + cmd := s1.Init() + require.NotNil(t, cmd) + require.True(t, animation.HasActive()) + + // Copy the spinner value and stop via the copy; should still stop the shared subscription. + s2 := s1 + s2.Stop() + require.False(t, animation.HasActive()) +} + func BenchmarkSpinner_ModeSpinnerOnly(b *testing.B) { s := New(ModeSpinnerOnly, lipgloss.NewStyle()) for b.Loop() {