Skip to content

feat(cli): visual and UX polish across all commands - #15

Open
chrisdeeming wants to merge 22 commits into
dev/worktree-managementfrom
dev/ui-polish
Open

feat(cli): visual and UX polish across all commands#15
chrisdeeming wants to merge 22 commits into
dev/worktree-managementfrom
dev/ui-polish

Conversation

@chrisdeeming

@chrisdeeming chrisdeeming commented Aug 19, 2026

Copy link
Copy Markdown
Member

Summary

  • Full visual/UX pass over every command: one output language (single summary box per outcome, hints, consistent empty states, dates, plurals, punctuation), grouped and styled --help, a native styled ps table, and TTY-gated spinners/progress so piped output is ANSI-free and NO_COLOR is respected.
  • New error model: styled renderer with remediation hints, internal classification sentinels never leak into messages, prompt/menu cancellation exits 0 silently, and passthrough commands (php, composer, compose, exec, direct XF commands) propagate the child's exit code with no redundant Error: trailer.
  • Fixes five real bugs found during the audit: init step counting ([8/7] drift), xf debug dropping all arguments after the first, an unchecked checksum slice panic in download, review-screen validation warnings being wiped before they could be read, and the keychain missing-entry sentinel.

Testing

  • go build ./... && go vet ./... && gofmt -l . && go test ./... — all green.
  • Full output audit of every command against the style rules, piped (| cat -v, NO_COLOR=1) and on a TTY.
  • Manual runs: doctor, auth status (all token states), cache list/purge/path, worktree list/prune, lifecycle (up/ps/down), error and exit-code paths.
  • Not exercised live: self-update install and a full upgrade run (no target install available); verified by reading and unit tests.

Visual evidence

Errors, before → after:

CleanShot 2026-08-19 at 13 05 03@2x  CleanShot 2026-08-19 at 13 05 26@2x

auth status with an expired token, before → after:

CleanShot 2026-08-19 at 12 52 08@2x  CleanShot 2026-08-19 at 12 53 05@2x

--help is now grouped with styled headings instead of one flat alphabetical list of 22 commands, before → after:

CleanShot 2026-08-19 at 13 15 00@2x  CleanShot 2026-08-19 at 13 14 24@2x

Notes

Checklist

  • The change is focused and does not include unrelated work.
  • Relevant automated and/or manual testing has been performed.
  • UI changes include screenshots or video where appropriate.
  • Upgrade, compatibility, and performance implications have been considered.

Note

Polish visual output and UX across all CLI commands

  • Adds structured command groups ("Getting started", "Environment", "Run tools", "Maintenance") to help output with styled headings via a custom Cobra usage template in cmd/xf/root.go.
  • Standardises terminal output across all commands: spinners use a single shared instance per operation, success results use SuccessBox, empty states use PrintEmpty, hints use PrintHint, and dates use FormatDate/FormatDateTime.
  • Adds new ui utilities in internal/ui/ui.go: Plural, FormatDate, FormatDateTime, IsTerminal, ShortHome, PrintHint, PrintEmpty, ClearScreen, and ProgressBar.Abandon; removes Box, Subheader, Italic, Underline, and StatusTableStyle.
  • Propagates child process exit codes via passthroughError and exitCodeError so CLI exits mirror subprocess status; SIGTERM is now handled alongside SIGINT with signal-derived exit codes (128+signum).
  • Adds runner.PSInfo to internal/dockercompose/runner.go for structured container status, and rewrites xf ps to render a formatted table with colored state columns.
  • xf doctor now exits non-zero when checks fail and prints an aggregated summary; xf cache purge now errors when called without --all or --license.
  • Risk: several commands change exit codes — xf doctor now exits 1 on failures, child-command wrappers now forward subprocess exit codes instead of always exiting 1, and ErrCancelled now exits 0.

Macroscope summarized 3c7004b.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3910bb82-800e-4863-b1e0-08fbe9605f9b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@chrisdeeming chrisdeeming changed the title dev/ui polish feat(cli): visual and UX polish across all commands Aug 19, 2026
@blacksmith-sh

This comment has been minimized.

Comment thread cmd/xf/main.go Outdated
Comment thread cmd/xf/init_review.go Outdated
Comment thread internal/dockercompose/runner.go
Comment thread cmd/xf/exec.go
Comment thread cmd/xf/doctor.go
Comment thread cmd/xf/init_helpers.go
Comment thread cmd/xf/auth.go
Comment thread cmd/xf/selfupdate.go Outdated
Comment thread cmd/xf/upgrade.go Outdated
Comment thread cmd/xf/init_execute.go Outdated
Comment thread cmd/xf/auth.go
Comment thread cmd/xf/init.go Outdated
Comment thread cmd/xf/init_review.go Outdated
Comment thread cmd/xf/doctor.go Outdated
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 26.00382% with 774 lines in your changes missing coverage. Please review.
✅ Project coverage is 36.11%. Comparing base (fe1ffde) to head (81a01c8).

❌ Your patch check has failed because the patch coverage (26.00%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@                     Coverage Diff                     @@
##           dev/worktree-management      #15      +/-   ##
===========================================================
+ Coverage                    33.16%   36.11%   +2.94%     
===========================================================
  Files                           57       58       +1     
  Lines                         6310     6709     +399     
===========================================================
+ Hits                          2093     2423     +330     
- Misses                        3891     3943      +52     
- Partials                       326      343      +17     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread cmd/xf/upgrade.go
Comment thread cmd/xf/debug.go
Comment thread cmd/xf/init.go Outdated
Comment thread cmd/xf/init_execute.go
Comment thread cmd/xf/debug.go Outdated
@chrisdeeming
chrisdeeming force-pushed the dev/ui-polish branch 2 times, most recently from e334c68 to 3c7004b Compare August 19, 2026 21:08
@chrisdeeming
chrisdeeming marked this pull request as ready for review August 19, 2026 21:16
…reporting, unified hints

Step plan is now computed by plannedInitSteps instead of a hardcoded
total, so --skip-up/--skip-install/--skip-composer and missing
composer.json all yield an accurate [n/n]. Skipped steps occupy their
real slot via printSkippedStep instead of a bare "Skipped" substep.
initExisting now shares checkPrerequisites with the fresh path instead
of duplicating the Docker checks, and reports a not-running Docker
daemon with a remediation hint. Downloads and extraction collapse to
one spinner and one checkmark per product instead of a substep plus a
separate detail line, and a failed xf:install now returns the wrapped
error (no success box after failure) instead of only warning.
Cancellations across init and the review flow are marked with
ErrCancelled so Execute exits 0 silently instead of printing an error
tail. Useful-commands and license-label formatting are consolidated.
plannedInitSteps was seeded by checking the filesystem for
composer.json before extraction had run, while the later
composer-install gate re-checked the filesystem after extraction. For
a fresh repository-checkout install, that meant the plan said 7 steps
but 8 printed, ending in [8/7].

Add extract.ContainsUploadFile to peek a package's upload/ contents
without extracting, and detectComposerBeforeDownload to resolve
hasComposer once (reusing the cache-hit xenforo download) before any
step total is committed to output. Thread that single value into both
plannedInitSteps and the composer gate so the two decisions can no
longer disagree.
Every extraction error now names the file or directory involved. Init
and InitExisting return the paths of any .default files written
alongside user-modified configs, and the init flows print a hint per
path instead of writing silently.
- auth status/logout: LoadToken now wraps ErrAuthRequired for the
  missing-token case, so the clean "Not authenticated" message shows
  instead of a raw keychain error leaking through.
- doctor: drop the contradictory "All N checks passed" phrasing when
  some checks only warned.
- auth: drop the inconsistent ellipsis on the "Refreshing access
  token" status line.
Add an Unreleased entry covering styled errors with hints, grouped
help, consistent output, the native ps table, quiet cancellation,
passthrough exit codes, and the fixed init/debug/upgrade bugs.
Ctrl-C exits 130 (SIGINT), not 0 — it's prompt/menu cancellation (Esc,
Cancel) that exits quietly. Also fix the checksum-panic fix's scope,
which belongs to download, not upgrade.
…ingle error reports

Doctor's warned-branch summary now reports the actual passed count
instead of the total. Help text and error output no longer leak raw
ANSI when piped or under NO_COLOR: cobra's template funcs gate on a
new ui.Enabled(f) check since cobra's writer bypasses lipgloss, and
handleError now routes through lipgloss.Fprintf so both its own
styling and any pre-rendered hint text get downsampled per-stream.

init now validates the target directory before the (potentially slow,
cold-cache) composer detection download and before the step plan is
printed, instead of after. Install and upgrade failures print one
error report instead of two, by returning a silent exit-code error
after the local failure message instead of a wrapped error that
handleError prints again.

Also: FormatDateTime instead of RFC1123 in auth refresh output, a
proper PrintHint idiom for cache list -v, dropped raw %v in two
warnings, a spinner-safe browser-open warning, and firstErrorClause
now strips trailing plumbing clauses even when the first clause is
itself plumbing.
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.

1 participant