Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 7, 2026

Replaced Existing Spinner with Simplified Implementation

Summary

Merged the SpinnerV2 improvements directly into spinner.go, replacing the previous implementation while maintaining 100% API compatibility.

Changes Made

  • Updated pkg/console/spinner.go with simplified implementation (124 lines vs 184)
  • Removed pkg/console/spinner_v2.go (no longer needed)
  • Removed pkg/console/spinner_v2_test.go (no longer needed)
  • Maintained SpinnerWrapper type name and NewSpinner() function
  • All existing tests pass without modification

Key Improvements

Code Reduction: 32.6%

  • Before: 184 lines
  • After: 124 lines
  • Reduction: 60 lines

Simplified State Management

  • Removed redundant running boolean field
  • Use program != nil check instead
  • Fields reduced from 3 to 2

Cleaner Logic

// Before
if !s.enabled || s.running { return }
s.running = true

// After
if s.enabled && s.program != nil { ... }

Compact Syntax

  • One-liner methods for simple functions (Init, View, IsEnabled)
  • Combined operations where logical
  • Removed obvious comments

API Compatibility

100% Compatible - No breaking changes:

  • NewSpinner(message string) *SpinnerWrapper
  • Start(), Stop(), StopWithMessage(), UpdateMessage(), IsEnabled()

All existing code using NewSpinner() continues to work without modification.

Validation

  • ✅ All 11 spinner tests pass
  • ✅ All console package tests pass
  • make fmt: PASS
  • make lint: PASS

Addresses feedback: @pelikhan requested to replace the existing spinner implementation and ensure code is formatted and linted.

Original prompt

This section details on the original issue you should resolve

<issue_title>[plan] Implement new idiomatic spinner using Bubble Tea</issue_title>
<issue_description>## Objective

Implement a new spinner using the idiomatic Bubble Tea pattern identified in the research phase.

Context

Based on the design document from the research task, implement a new spinner that follows Bubble Tea best practices while maintaining existing functionality.

Approach

  1. Create new implementation in pkg/console/spinner_v2.go (keep old for now)
  2. Use recommended Bubble Tea pattern (e.g., tea.WithoutRenderer() or direct component)
  3. Maintain existing API: Start(), Stop(), SetMessage()
  4. Preserve TTY detection and accessibility support
  5. Eliminate manual goroutine management and mutexes

Files to Modify

  • Create: pkg/console/spinner_v2.go (new implementation)
  • Reference: pkg/console/spinner.go (for API compatibility)

Implementation Checklist

  • Implement SpinnerV2 struct with same public API
  • Use Bubble Tea program or direct component (no custom goroutines)
  • TTY detection works (disable in pipes/CI)
  • Accessibility support (ACCESSIBLE env var)
  • MiniDot animation style maintained
  • Clean start/stop semantics
  • No mutex required

Acceptance Criteria

AI generated by Plan Command for discussion #9188

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits January 7, 2026 17:06
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
- Create spinner_v2.go with 43.7% code reduction (104 vs 185 lines)
- Eliminate 'running' state field - use program != nil check
- Maintain full API compatibility with SpinnerWrapper
- Add comprehensive test suite with 14 tests
- All tests pass, code formatted and linted

Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Implement new idiomatic spinner using Bubble Tea Implement SpinnerV2 with simplified state management and 43% code reduction Jan 7, 2026
Copilot AI requested a review from mnkiefer January 7, 2026 17:21
@pelikhan
Copy link
Contributor

pelikhan commented Jan 7, 2026

@copilot replace existing spinner

- Merge SpinnerV2 improvements into main spinner.go
- Remove spinner_v2.go and spinner_v2_test.go files
- Maintain SpinnerWrapper type and NewSpinner() function
- Eliminate redundant 'running' state field
- Reduce code by 32.6% (184 → 124 lines)
- All tests pass, 100% API compatible

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 7, 2026

@copilot replace existing spinner

Done. Replaced the existing spinner implementation with the simplified version. The improvements are now in spinner.go (124 lines vs 184, 32.6% reduction) with the same SpinnerWrapper type and NewSpinner() function. Removed spinner_v2.go and spinner_v2_test.go. All tests pass. (commit 0bffb2e)

Copilot AI changed the title Implement SpinnerV2 with simplified state management and 43% code reduction Replace existing spinner with simplified state management and 33% code reduction Jan 7, 2026
Copilot AI requested a review from pelikhan January 7, 2026 19:20
@pelikhan pelikhan marked this pull request as ready for review January 7, 2026 19:37
@pelikhan
Copy link
Contributor

pelikhan commented Jan 7, 2026

@copilot format, lint go

The file was reintroduced during merge with main but is no longer
needed since SpinnerV2 implementation was merged into main spinner.
Removing it fixes typecheck errors about missing 'running' field.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 7, 2026

@copilot format, lint go

Done. Ran make fmt and make lint - both pass. Removed spinner_v2_test.go that was reintroduced during merge with main, which was causing lint errors about the missing running field. All tests pass. (commit bf01836)

@pelikhan pelikhan merged commit 800e166 into main Jan 7, 2026
48 checks passed
@pelikhan pelikhan deleted the copilot/implement-bubble-tea-spinner branch January 7, 2026 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[plan] Implement new idiomatic spinner using Bubble Tea

3 participants