starred/App-Store-Connect-CLI

Fork 0

mirror of https://github.com/rudrankriyam/App-Store-Connect-CLI.git synced 2026-04-25 15:45:48 +03:00

[GH-ISSUE #594] feat: Add gh-style stderr spinner (no color) #168

New issue

Closed

opened 2026-02-26 21:33:52 +03:00 by kerem · 1 comment

kerem commented

2026-02-26 21:33:52 +03:00

Owner

Originally created by @rudrankriyam on GitHub (Feb 17, 2026).
Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/594

Summary

Add a GitHub CLI-style indeterminate spinner (the revolving dots/braille frames) for short-running operations.

Key goals:

No color (plain terminal default color)
stderr-only (never pollute stdout, which is often JSON)
TTY-gated (never show when output is non-interactive)
Opt-out via env var (like GH_SPINNER_DISABLED)
Implement as a shared primitive so any command can use it, but only adopt it in safe places initially (Option A).

This is explicitly the spinner style from gh:

frames: ⣾ ⣽ ⣻ ⢿ ⡿ ⣟ ⣯ ⣷ (spinner charset 11)

Why this matters

A subtle spinner makes the CLI feel “alive” while it’s:

fetching multiple resources
doing parallel API calls
waiting on a small network burst

It’s especially valuable for dashboard-style commands (like the planned asc status) where:

output is printed only after all fetches complete
the user otherwise stares at a blank terminal

Current state (verified)

The CLI already has the concept of “progress safe to emit” via shared.ProgressEnabled():
- progress must be stderr-only
- disabled when stderr is not a TTY
- there is internal plumbing (noProgress) but no consistent spinner primitive
Update downloads use a progress bar (schollz/progressbar) for byte-based downloads.
- This issue is NOT trying to replace that.

Proposed approach (Option A)

1) Build a shared spinner primitive

Add a small shared helper (stdlib only; no new deps) under internal/cli/shared/.

Proposed API (example; bikeshed ok):

// WithSpinner runs fn while a spinner is displayed on stderr.
// It is a no-op when the spinner is disabled.
func WithSpinner(label string, fn func() error) error

// Or an explicit type:
type Spinner struct {
  // ...
}
func NewSpinner(w io.Writer) *Spinner
func (s *Spinner) Start(label string)
func (s *Spinner) Stop()

Implementation rules:

Writes to os.Stderr only
Emits no ANSI styling (no colors)
Keep output minimal:
- use \r carriage return for in-place update
- on Stop(), clear the line so the next stderr write starts cleanly
Avoid cursor hide/show to keep it “normal”
Bounded tick rate (e.g. 120ms like gh)

2) Add env var kill switch

Add an env var similar to GitHub CLI:

ASC_SPINNER_DISABLED

Semantics (match gh style):

if env var is unset → spinner allowed
if set to a falsey value (0, false, no, or empty) → spinner allowed
if set to anything else (including invalid boolean) → spinner disabled

This “invalid => disabled” behavior is intentional: it makes it hard to accidentally enable spinners in CI.

3) TTY gating

Spinner should only render when BOTH:

stdout is a TTY
stderr is a TTY

Rationale: if stdout is being piped to a file/another process, we should be maximally quiet.

4) Adopt in safe places first

Do not blanket-wrap every command.

Initial adoption target:

asc status (issue #585)
- spinner during data fetch phase
- spinner stops before rendering the dashboard/table/JSON

Avoid using spinner in commands that:

stream logs to stderr (--debug, --api-debug, retry logs)
show a byte progress bar (downloads)
continuously poll and print status updates (already “chatty”)

Follow-ups (future):

gradually add to other dashboard-like commands that are quiet on stderr

UX details

No output changes

stdout output for commands must remain identical.
spinner is purely cosmetic on interactive terminals.

Optional label

Support a short label prefix like:

Fetching… or Working…

But keep the default label empty to match gh status behavior.

Implementation notes

You already have shared.ProgressEnabled(). Consider adding:
- shared.SpinnerEnabled() (checks ProgressEnabled + stdout TTY + env var)
Keep spinner logic isolated so it’s easy to turn off globally later.

Test plan (TDD-first)

Unit tests

SpinnerEnabled() behavior matrix:
- disabled when stdout not TTY
- disabled when stderr not TTY
- disabled when ASC_SPINNER_DISABLED=1|true|yes|garbage
- enabled when ASC_SPINNER_DISABLED=0|false|no|""

Cmdtests (once `asc status` exists)

asc status --output json stdout remains valid JSON
spinner never appears when stdout is piped
spinner never appears when ASC_SPINNER_DISABLED=1

Acceptance criteria

Shared spinner helper exists (stdlib only, no new dependencies).
Spinner uses the gh-style frames (charset 11) and no ANSI color.
Spinner is stderr-only and TTY-gated (stdout+stderr).
ASC_SPINNER_DISABLED disables it reliably.
First adoption is limited to asc status (and any other explicitly chosen, quiet commands).
make test passes.

Originally created by @rudrankriyam on GitHub (Feb 17, 2026). Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/594 ## Summary Add a GitHub CLI-style **indeterminate spinner** (the revolving dots/braille frames) for short-running operations. Key goals: - **No color** (plain terminal default color) - **stderr-only** (never pollute stdout, which is often JSON) - **TTY-gated** (never show when output is non-interactive) - **Opt-out via env var** (like `GH_SPINNER_DISABLED`) - Implement as a shared primitive so any command *can* use it, but **only adopt it in safe places initially** (Option A). This is explicitly the spinner style from `gh`: - frames: `⣾ ⣽ ⣻ ⢿ ⡿ ⣟ ⣯ ⣷` (spinner charset 11) ## Why this matters A subtle spinner makes the CLI feel “alive” while it’s: - fetching multiple resources - doing parallel API calls - waiting on a small network burst It’s especially valuable for dashboard-style commands (like the planned `asc status`) where: - output is printed only after all fetches complete - the user otherwise stares at a blank terminal ## Current state (verified) - The CLI already has the concept of “progress safe to emit” via `shared.ProgressEnabled()`: - progress must be **stderr-only** - disabled when stderr is not a TTY - there is internal plumbing (`noProgress`) but no consistent spinner primitive - Update downloads use a **progress bar** (`schollz/progressbar`) for byte-based downloads. - This issue is NOT trying to replace that. ## Proposed approach (Option A) ### 1) Build a shared spinner primitive Add a small shared helper (stdlib only; no new deps) under `internal/cli/shared/`. Proposed API (example; bikeshed ok): ```go // WithSpinner runs fn while a spinner is displayed on stderr. // It is a no-op when the spinner is disabled. func WithSpinner(label string, fn func() error) error // Or an explicit type: type Spinner struct { // ... } func NewSpinner(w io.Writer) *Spinner func (s *Spinner) Start(label string) func (s *Spinner) Stop() ``` Implementation rules: - Writes to `os.Stderr` only - Emits no ANSI styling (no colors) - Keep output minimal: - use `\r` carriage return for in-place update - on Stop(), clear the line so the next stderr write starts cleanly - Avoid cursor hide/show to keep it “normal” - Bounded tick rate (e.g. 120ms like `gh`) ### 2) Add env var kill switch Add an env var similar to GitHub CLI: - `ASC_SPINNER_DISABLED` Semantics (match `gh` style): - if env var is unset → spinner allowed - if set to a falsey value (`0`, `false`, `no`, or empty) → spinner allowed - if set to anything else (including invalid boolean) → spinner disabled This “invalid => disabled” behavior is intentional: it makes it hard to accidentally enable spinners in CI. ### 3) TTY gating Spinner should only render when BOTH: - stdout is a TTY - stderr is a TTY Rationale: if stdout is being piped to a file/another process, we should be maximally quiet. ### 4) Adopt in *safe* places first Do not blanket-wrap every command. Initial adoption target: - `asc status` (issue #585) - spinner during data fetch phase - spinner stops before rendering the dashboard/table/JSON Avoid using spinner in commands that: - stream logs to stderr (`--debug`, `--api-debug`, retry logs) - show a byte progress bar (downloads) - continuously poll and print status updates (already “chatty”) Follow-ups (future): - gradually add to other dashboard-like commands that are quiet on stderr ## UX details ### No output changes - stdout output for commands must remain identical. - spinner is purely cosmetic on interactive terminals. ### Optional label Support a short label prefix like: - `Fetching…` or `Working…` But keep the default label empty to match `gh status` behavior. ## Implementation notes - You already have `shared.ProgressEnabled()`. Consider adding: - `shared.SpinnerEnabled()` (checks `ProgressEnabled` + stdout TTY + env var) - Keep spinner logic isolated so it’s easy to turn off globally later. ## Test plan (TDD-first) ### Unit tests - [ ] `SpinnerEnabled()` behavior matrix: - [ ] disabled when stdout not TTY - [ ] disabled when stderr not TTY - [ ] disabled when `ASC_SPINNER_DISABLED=1|true|yes|garbage` - [ ] enabled when `ASC_SPINNER_DISABLED=0|false|no|""` ### Cmdtests (once `asc status` exists) - [ ] `asc status --output json` stdout remains valid JSON - [ ] spinner never appears when stdout is piped - [ ] spinner never appears when `ASC_SPINNER_DISABLED=1` ## Acceptance criteria - [ ] Shared spinner helper exists (stdlib only, no new dependencies). - [ ] Spinner uses the `gh`-style frames (charset 11) and no ANSI color. - [ ] Spinner is stderr-only and TTY-gated (stdout+stderr). - [ ] `ASC_SPINNER_DISABLED` disables it reliably. - [ ] First adoption is limited to `asc status` (and any other explicitly chosen, quiet commands). - [ ] `make test` passes.

kerem

2026-02-26 21:33:52 +03:00

closed this issue
added the
enhancement
label

kerem commented

2026-02-26 21:33:53 +03:00

Author

Owner

@rudrankriyam commented on GitHub (Feb 17, 2026):

Where to use this next (pagination-first)

Agree: the biggest perceived latency today is --paginate list flows (e.g. asc apps list --paginate). These commands often do many sequential HTTP requests and otherwise look “stuck” in the terminal.

Recommendation: after the shared spinner primitive lands, adopt it in every command path that:

accepts --paginate, and
calls asc.PaginateAll(...) (or otherwise loops over links.next), and
is not already streaming noisy stderr output (debug logs, byte progress bars, etc.)

Key constraint: spinner must remain stderr-only + TTY-gated (stdout+stderr) and be disabled when output is piped.

Concrete adoption list (high priority)

These are the places that already expose --paginate and are common/slow in real use:

asc apps list --paginate (internal/cli/apps/apps.go)
Builds:
- internal/cli/builds/builds_commands.go (builds list)
- internal/cli/builds/builds_related.go
- internal/cli/builds/builds_relationships.go
- internal/cli/builds/builds_individual_testers.go
- internal/cli/builds/build_test_notes.go
- internal/cli/builds/builds_uploads.go
TestFlight:
- internal/cli/testflight/testflight.go (apps/builds/groups lists)
- internal/cli/testflight/beta_testers.go
- internal/cli/testflight/beta_groups.go
- internal/cli/testflight/*relationships.go + *related.go
Analytics:
- internal/cli/analytics/analytics_instances.go
- internal/cli/analytics/analytics_reports.go
- internal/cli/analytics/analytics_requests.go (explicitly recommends --paginate)
Reviews:
- internal/cli/reviews/reviews.go
- internal/cli/reviews/review_submissions.go
- internal/cli/reviews/review_items.go
- internal/cli/reviews/review_attachments.go
- internal/cli/reviews/reviews_summarizations.go
Signing:
- internal/cli/certificates/certificates.go
- internal/cli/profiles/profiles.go
Xcode Cloud list endpoints (often paginated):
- internal/cli/xcodecloud/*.go (workflows/build-runs/artifacts/test-results/scm/issues/actions/extras)

Implementation note (keep it low-effort per command)

The mechanical pattern should be:

if --paginate is true, wrap the asc.PaginateAll(...) call in shared.WithSpinner("", fn)
otherwise do the single-page request as-is (no spinner)

This keeps the spinner:

off for fast single-page lists
on for the slow path where users actually wait

Add to acceptance criteria (for rollout beyond `asc status`)

For any command with --paginate, spinner runs only when --paginate is set.
Spinner never appears when stdout is piped (JSON contracts remain clean).
Spinner never interleaves with --api-debug / retry logs (disable spinner when those are enabled).

@cursor please implement pagination-first adoption after the shared spinner lands.

@rudrankriyam commented on GitHub (Feb 17, 2026): ### Where to use this next (pagination-first) Agree: the biggest perceived latency today is **`--paginate` list flows** (e.g. `asc apps list --paginate`). These commands often do many sequential HTTP requests and otherwise look “stuck” in the terminal. Recommendation: after the shared spinner primitive lands, adopt it in *every* command path that: - accepts `--paginate`, and - calls `asc.PaginateAll(...)` (or otherwise loops over `links.next`), and - is not already streaming noisy stderr output (debug logs, byte progress bars, etc.) Key constraint: **spinner must remain stderr-only + TTY-gated** (stdout+stderr) and be disabled when output is piped. #### Concrete adoption list (high priority) These are the places that already expose `--paginate` and are common/slow in real use: - `asc apps list --paginate` (`internal/cli/apps/apps.go`) - Builds: - `internal/cli/builds/builds_commands.go` (builds list) - `internal/cli/builds/builds_related.go` - `internal/cli/builds/builds_relationships.go` - `internal/cli/builds/builds_individual_testers.go` - `internal/cli/builds/build_test_notes.go` - `internal/cli/builds/builds_uploads.go` - TestFlight: - `internal/cli/testflight/testflight.go` (apps/builds/groups lists) - `internal/cli/testflight/beta_testers.go` - `internal/cli/testflight/beta_groups.go` - `internal/cli/testflight/*relationships.go` + `*related.go` - Analytics: - `internal/cli/analytics/analytics_instances.go` - `internal/cli/analytics/analytics_reports.go` - `internal/cli/analytics/analytics_requests.go` (explicitly recommends `--paginate`) - Reviews: - `internal/cli/reviews/reviews.go` - `internal/cli/reviews/review_submissions.go` - `internal/cli/reviews/review_items.go` - `internal/cli/reviews/review_attachments.go` - `internal/cli/reviews/reviews_summarizations.go` - Signing: - `internal/cli/certificates/certificates.go` - `internal/cli/profiles/profiles.go` - Xcode Cloud list endpoints (often paginated): - `internal/cli/xcodecloud/*.go` (workflows/build-runs/artifacts/test-results/scm/issues/actions/extras) #### Implementation note (keep it low-effort per command) The mechanical pattern should be: - if `--paginate` is true, wrap the `asc.PaginateAll(...)` call in `shared.WithSpinner("", fn)` - otherwise do the single-page request as-is (no spinner) This keeps the spinner: - off for fast single-page lists - on for the slow path where users actually wait #### Add to acceptance criteria (for rollout beyond `asc status`) - [ ] For any command with `--paginate`, spinner runs only when `--paginate` is set. - [ ] Spinner never appears when stdout is piped (JSON contracts remain clean). - [ ] Spinner never interleaves with `--api-debug` / retry logs (disable spinner when those are enabled). @cursor please implement pagination-first adoption after the shared spinner lands.

kerem referenced this issue

2026-02-26 21:34:38 +03:00

[PR #174] [MERGED] feat: add age rating declaration CLI #336

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/App-Store-Connect-CLI#168

No description provided.

Rows
Columns