feat(cli): visual and UX polish across all commands - #15
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This comment has been minimized.
This comment has been minimized.
226711f to
4e8bcee
Compare
4e8bcee to
ecf6e93
Compare
ecf6e93 to
ae3aba2
Compare
Codecov Report❌ Patch coverage is ❌ 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:
|
ae3aba2 to
02020e1
Compare
02020e1 to
9f606c6
Compare
e334c68 to
3c7004b
Compare
3c7004b to
13b6dca
Compare
…on, cancel and exit-code handling
…, drop redundant banners
…ummary, checksum panic fix
…t, single summary
…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.
…lidated validators
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.
13b6dca to
81a01c8
Compare
Summary
✓summary box per outcome,→hints, consistent empty states, dates, plurals, punctuation), grouped and styled--help, a native styledpstable, and TTY-gated spinners/progress so piped output is ANSI-free andNO_COLORis respected.✗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 redundantError:trailer.[8/7]drift),xf debugdropping all arguments after the first, an unchecked checksum slice panic indownload, 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.| cat -v,NO_COLOR=1) and on a TTY.doctor,auth status(all token states),cache list/purge/path,worktree list/prune, lifecycle (up/ps/down), error and exit-code paths.self-updateinstall and a fullupgraderun (no target install available); verified by reading and unit tests.Visual evidence
Errors, before → after:
auth statuswith an expired token, before → after:--helpis now grouped with styled headings instead of one flat alphabetical list of 22 commands, before → after:Notes
mainonce feat(worktree): create and manage development worktrees #14 merges.cache purgewithout flags is now a usage error (exit 1);doctorfailure exits 1 without a duplicate error line.Checklist
Note
Polish visual output and UX across all CLI commands
cmd/xf/root.go.SuccessBox, empty states usePrintEmpty, hints usePrintHint, and dates useFormatDate/FormatDateTime.uiutilities ininternal/ui/ui.go:Plural,FormatDate,FormatDateTime,IsTerminal,ShortHome,PrintHint,PrintEmpty,ClearScreen, andProgressBar.Abandon; removesBox,Subheader,Italic,Underline, andStatusTableStyle.passthroughErrorandexitCodeErrorso CLI exits mirror subprocess status; SIGTERM is now handled alongside SIGINT with signal-derived exit codes (128+signum).runner.PSInfotointernal/dockercompose/runner.gofor structured container status, and rewritesxf psto render a formatted table with colored state columns.xf doctornow exits non-zero when checks fail and prints an aggregated summary;xf cache purgenow errors when called without--allor--license.xf doctornow exits 1 on failures, child-command wrappers now forward subprocess exit codes instead of always exiting 1, andErrCancellednow exits 0.Macroscope summarized 3c7004b.