From 22fac904e97d68d97631e6bd3e46c5cbf3ce4f2b Mon Sep 17 00:00:00 2001 From: Matt White Date: Mon, 4 May 2026 19:39:08 -0600 Subject: [PATCH 1/2] =?UTF-8?q?fix(setup,sync):=201.0=20polish=20round=202?= =?UTF-8?q?=20=E2=80=94=20layout,=20completion=20handoff,=20cancel=20audit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundled fixes for issues uncovered when the user actually walked through `ckipper setup` end-to-end. Three parallel agents investigated cancel- propagation across the codebase, layout redesign, and wizard completeness; this commit synthesizes their findings. Detected configuration layout (Issue 1) Replaced the alternating wide-row + indented-description rendering with a card-style stack: each setting renders as ` ` on one line, with the schema description in dim color on the next, separated by blank lines. Removes the column-overflow problem (long values used to push the source marker out of alignment) and gives the eye a clear stopping point per setting. Empty values now render as `(empty)` instead of blank space. Wizard completion handoff (Issue 2) After a 5-minute docker build the old "Setup complete" header was buried in scrollback and the wizard exited silently. Now: - `_ckipper_setup_offer_image_build` records ok/failed/skipped and `_ckipper_setup_print_completion_summary` renders a coloured banner so a failed build is impossible to miss. - The summary lists `ckipper worktree rebuild-image` and `ckipper account sync` alongside the existing get-started commands so users can find them later without re-running the full wizard. - A "Press ENTER to finish setup" prompt anchors the screen so users know setup is over (skipped on non-TTY for CI). Per-account aliases auto-source (Issue 1b) `install.sh` appends the per-account aliases source line to ~/.zshrc, but a setup-only re-run never offered to. Added `_ckipper_setup_offer_aliases_source` with an idempotency check so re-runs don't duplicate the line. Without this, `claude-` launchers silently didn't work for users who only ran `ckipper setup`. Sync-after-2nd-add (Issue 3) Verified by trace: `_ckipper_setup_offer_initial_sync` correctly counts accounts AFTER the add and fires when count >= 2. No code change needed; the existing test covers this path. Rebuild-image discoverability (Issue 4a) `ckipper worktree rebuild-image` is the dedicated CLI; now mentioned in the completion summary's Maintenance block. Help text updated to list every wizard step including this one. Cancel propagation audit (Issue 5) Agent 1 found two unguarded sites in the sync interactive fallback (CKIPPER_NO_GUM=1 path): - `_ckipper_account_sync_pick_targets_fallback` - `_ckipper_account_sync_pick_types` (read fallback branch) Both now `|| return $?` after `_core_prompt_input` to propagate cancel. Other sites flagged by the audit (gum --no-limit pickers) are already protected by the dispatcher's empty-array length check + SPACE/ENTER hint added in PR #41. Tests - 520/520 shell tests pass (+10 over previous baseline). - New regressions: image build status (ok/failed/skipped), summary mentions rebuild-image + sync + failed-build banner, aliases-source skip/append/decline, sync interactive fallback cancel propagation. - Lint clean. --- lib/account/sync/interactive.zsh | 8 +- lib/account/sync/interactive_test.bats | 29 +++++++ lib/setup/dispatcher.zsh | 109 ++++++++++++++++++++----- lib/setup/dispatcher_test.bats | 85 +++++++++++++++++++ lib/setup/prompts.zsh | 52 +++++++----- 5 files changed, 238 insertions(+), 45 deletions(-) diff --git a/lib/account/sync/interactive.zsh b/lib/account/sync/interactive.zsh index adb69b5..1ab72c3 100644 --- a/lib/account/sync/interactive.zsh +++ b/lib/account/sync/interactive.zsh @@ -63,7 +63,10 @@ _ckipper_account_sync_pick_targets() { _ckipper_account_sync_pick_targets_fallback() { echo "Available targets: $*" >&2 local input - input=$(_core_prompt_input "Enter comma-separated targets" "") + # Propagate cancel (Esc/Ctrl-C/EOF) so the caller's empty-array check + # sees no targets and prints the SPACE/ENTER hint, instead of treating + # whatever shell garbage `$input` happens to contain as the list. + input=$(_core_prompt_input "Enter comma-separated targets" "") || return $? local name for name in ${(s:,:)input}; do echo "$name" @@ -87,7 +90,8 @@ _ckipper_account_sync_pick_types() { fi echo "Type tokens: ${(@k)_CKIPPER_SYNC_TYPE_LABEL}" >&2 local input - input=$(_core_prompt_input "Enter comma-separated types" "") + # Propagate cancel; same reasoning as pick_targets_fallback above. + input=$(_core_prompt_input "Enter comma-separated types" "") || return $? local name for name in ${(s:,:)input}; do echo "$name" diff --git a/lib/account/sync/interactive_test.bats b/lib/account/sync/interactive_test.bats index 1146059..1b57086 100644 --- a/lib/account/sync/interactive_test.bats +++ b/lib/account/sync/interactive_test.bats @@ -34,3 +34,32 @@ run_in_zsh() { [[ "$output" == *"client1,work,"* ]] [[ "$output" != *"personal"* ]] } + +# Regression: cancel from the comma-separated input prompt used to pass +# through as a 0-rc empty-output result, which the dispatcher then split +# into an empty array — masking cancel as "user submitted no targets". +# Now propagates the prompt's non-zero rc so callers can distinguish. +@test "_pick_targets_fallback returns non-zero on EOF (cancel)" { + run env HOME="$HOME" CKIPPER_DIR="$CKIPPER_DIR" CKIPPER_REGISTRY="$CKIPPER_REGISTRY" \ + CKIPPER_NO_GUM=1 TMP_HOME="$TMP_HOME" PATH="$PATH" \ + zsh -c "source \"$REPO_ROOT/lib/core/registry.zsh\"; \ + source \"$REPO_ROOT/lib/core/prompt.zsh\"; \ + source \"$REPO_ROOT/lib/account/sync/interactive.zsh\"; \ + _ckipper_account_sync_pick_targets_fallback work 2>/dev/null" /dev/null" Bundle worktree + Claude in one step" + echo " ck Interactive menu" + echo " claude- Per-account launcher (e.g. claude-personal)" echo "" - echo "Launch Claude in a worktree (host or Docker):" - echo " ckipper run # bundles worktree + Claude in one step" + echo "Maintenance:" + echo " ckipper config list Review every setting" + echo " ckipper doctor Diagnose installation issues" + echo " ckipper worktree rebuild-image Rebuild ckipper-dev Docker image" + echo " ckipper account sync Copy settings between accounts" echo "" - echo "Launch Claude directly with an account context:" - echo " claude- # auto-generated launcher" - echo " # bare-name shortcut, when free" +} + +# Render a single banner line about the docker image build outcome. Helps +# the user notice a build failure that would otherwise scroll past with +# the rest of `docker build` output. +# +# Args: $1 — `ok` | `failed` | `skipped`. +# Returns: 0 always. +_ckipper_setup_render_image_status() { + case "$1" in + ok) _core_style_color green "Docker image: built successfully." ;; + failed) _core_style_color red "Docker image: build FAILED — re-run with: ckipper worktree rebuild-image" ;; + skipped) _core_style_color dim "Docker image: skipped — build later with: ckipper worktree rebuild-image" ;; + esac echo "" - echo "Or just run 'ck' for the interactive menu." +} + +# Pause until the user presses Enter, so the "Setup complete" banner does +# not disappear off-screen behind the next shell prompt — particularly +# important when the docker build output preceded it. Skipped on +# non-interactive stdin (CI, piped installers). +# +# Returns: 0 always. +_ckipper_setup_wait_for_acknowledgement() { + [[ -t 0 ]] || return 0 + local _ack="" + read -r "_ack?Press ENTER to finish setup. " } # Print top-level setup help. @@ -90,7 +124,10 @@ _ckipper_setup_help() { " 1. Verifies prereqs (gum, jq, docker) and offers to brew-install missing." \ " 2. Shows your current global config and lets you customize any subset." \ " 3. Offers to register a Claude account and configure its preferences." \ - " 4. Offers to build the ckipper-dev Docker image." \ + " 4. Offers to sync settings between two existing accounts (≥ 2 accounts)." \ + " 5. Offers to wire per-account launchers (claude-) into ~/.zshrc." \ + " 6. Offers to build the ckipper-dev Docker image." \ + " 7. Prints a Setup Complete summary; press ENTER to finish." \ "" \ "Usage:" \ " ckipper setup Run the wizard." \ @@ -213,16 +250,46 @@ _ckipper_setup_collect_account_prefs() { "Forward host ~/.ssh into '$account' containers?" } -# Offer to build/rebuild the ckipper-dev Docker image now. +# Offer to build/rebuild the ckipper-dev Docker image now. Records the +# outcome in _CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS so the completion +# summary can render a banner — without that signal, a failed build is +# easy to miss in the 5+ minutes of streaming docker output and the user +# would only discover it later when `--docker` runs hit "image not found." # -# We invoke the build helper directly rather than wrapping it in a spinner. -# `gum spin -- ` execs its argv as a binary, so passing a shell function -# fails with "executable file not found in $PATH". The build also streams -# its own progress over ~5 min, which the user wants to see. +# Sets _CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS to one of: ok, failed, skipped. +# Returns: 0 always (failures are surfaced via the status global, not rc, +# so the wizard always finishes the post-build flow). +_ckipper_setup_offer_image_build() { + typeset -g _CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS="skipped" + if ! _core_prompt_confirm "Build the Docker image now? (slow; ~5 min)"; then + return 0 + fi + if _ckipper_worktree_build_image; then + _CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS="ok" + else + _CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS="failed" + fi +} + +# Offer to add the per-account aliases source line to ~/.zshrc. The +# launchers (`claude-`, bare ``) only exist when the +# user's shell sources `~/.ckipper/aliases.zsh`. install.sh prints the +# suggestion but never appends it; setup-only re-runs (post-install) +# never see the suggestion at all. This step closes that loop, with an +# idempotency check so re-runs don't duplicate the line. # # Returns: 0 always. -_ckipper_setup_offer_image_build() { - if _core_prompt_confirm "Build the Docker image now? (slow; ~5 min)"; then - _ckipper_worktree_build_image +_ckipper_setup_offer_aliases_source() { + local zshrc="$HOME/.zshrc" + [[ -f "$zshrc" ]] || return 0 + grep -q 'ckipper/aliases\.zsh' "$zshrc" 2>/dev/null && return 0 + if ! _core_prompt_confirm "Add per-account launchers (claude-) to ~/.zshrc?"; then + return 0 fi + { + echo "" + echo "# Ckipper — per-account launchers (claude-, bare )" + echo '[[ -f ~/.ckipper/aliases.zsh ]] && source ~/.ckipper/aliases.zsh' + } >> "$zshrc" + echo "Added the source line. Open a new shell (or run 'source ~/.zshrc')." } diff --git a/lib/setup/dispatcher_test.bats b/lib/setup/dispatcher_test.bats index bce5545..e507bac 100644 --- a/lib/setup/dispatcher_test.bats +++ b/lib/setup/dispatcher_test.bats @@ -117,6 +117,91 @@ JSON [[ "$output" == *"STUB-BUILD"* ]] } +# Regression: a failed docker build was easy to miss because 5 minutes of +# streaming output buried the completion message. The wizard now records +# build outcome (ok / failed / skipped) and the completion summary +# renders a colored banner that's findable at a glance. +@test "_ckipper_setup_offer_image_build records ok when build succeeds" { + _run_setup $'y\n' ' + _ckipper_worktree_build_image() { return 0; } + _ckipper_setup_offer_image_build + echo "status=$_CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS"' + + [[ "$output" == *"status=ok"* ]] +} + +@test "_ckipper_setup_offer_image_build records failed when build returns non-zero" { + _run_setup $'y\n' ' + _ckipper_worktree_build_image() { return 1; } + _ckipper_setup_offer_image_build + echo "status=$_CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS"' + + [[ "$output" == *"status=failed"* ]] +} + +@test "_ckipper_setup_offer_image_build records skipped when user declines" { + _run_setup $'n\n' ' + _ckipper_worktree_build_image() { echo SHOULD-NOT-RUN; } + _ckipper_setup_offer_image_build + echo "status=$_CKIPPER_SETUP_LAST_IMAGE_BUILD_STATUS"' + + [[ "$output" == *"status=skipped"* ]] + [[ "$output" != *"SHOULD-NOT-RUN"* ]] +} + +# Regression: completion summary previously listed only the basics; now +# it also points at `ckipper worktree rebuild-image` and `ckipper account +# sync` so users can find them without re-running the full wizard. +@test "_ckipper_setup_print_completion_summary mentions rebuild-image and sync" { + _run_setup "" "_ckipper_setup_print_completion_summary ok" + + [ "$status" -eq 0 ] + [[ "$output" == *"ckipper worktree rebuild-image"* ]] + [[ "$output" == *"ckipper account sync"* ]] +} + +# Regression: a build failure used to be invisible in the completion +# screen. Now the banner explicitly calls it out and points at the +# rebuild command. +@test "_ckipper_setup_print_completion_summary surfaces a failed build banner" { + _run_setup "" "_ckipper_setup_print_completion_summary failed" + + [ "$status" -eq 0 ] + [[ "$output" == *"FAILED"* ]] + [[ "$output" == *"rebuild-image"* ]] +} + +# Regression: setup never offered to wire the per-account aliases source +# line into ~/.zshrc. Users who installed via install.sh got it appended +# (line 158-160 of install.sh); users who only ever ran `ckipper setup` +# missed it and `claude-` launchers silently didn't work. +@test "_ckipper_setup_offer_aliases_source skips when ~/.zshrc already sources it" { + echo 'source ~/.ckipper/aliases.zsh' > "$TMP_HOME/.zshrc" + + _run_setup "" "_ckipper_setup_offer_aliases_source 2>&1" + + [ "$status" -eq 0 ] + [[ "$output" != *"Add per-account launchers"* ]] +} + +@test "_ckipper_setup_offer_aliases_source appends source line on accept" { + : > "$TMP_HOME/.zshrc" + + _run_setup $'y\n' "_ckipper_setup_offer_aliases_source 2>&1" + + [ "$status" -eq 0 ] + grep -q 'ckipper/aliases\.zsh' "$TMP_HOME/.zshrc" +} + +@test "_ckipper_setup_offer_aliases_source declines do not write to ~/.zshrc" { + : > "$TMP_HOME/.zshrc" + + _run_setup $'n\n' "_ckipper_setup_offer_aliases_source 2>&1" + + [ "$status" -eq 0 ] + ! grep -q 'ckipper/aliases\.zsh' "$TMP_HOME/.zshrc" +} + # Regression: setup previously offered cross-account sync only after the # user added a NEW account in the wizard. A user with 2+ existing accounts # who declined "Add another?" never saw the sync feature surfaced. The diff --git a/lib/setup/prompts.zsh b/lib/setup/prompts.zsh index f988f3a..4d11366 100644 --- a/lib/setup/prompts.zsh +++ b/lib/setup/prompts.zsh @@ -36,15 +36,20 @@ readonly _CKIPPER_SETUP_PROMPTS_HEADER="Detected configuration" # row and silently advance with no overrides. readonly _CKIPPER_SETUP_PROMPTS_PICKER_HEADER="Pick keys to customize (SPACE to mark, ENTER to confirm)" -# Pipe-separated row builder for the summary table. Resolves the effective -# value via _core_config_get and the source marker via _core_config_read_global -# (empty return ⇒ default; otherwise ⇒ user override). +# Width of the SETTING column in the card-style summary. 22 chars covers +# every key in the current schema (longest is `aliases_auto_source` at 19) +# with a 3-char gutter before the value. +readonly _CKIPPER_SETUP_PROMPTS_KEY_WIDTH=22 + +# Render one schema key as a two-line "card": ` ` +# on line 1 and the dim-colored description indented on line 2. Account +# keys are filtered out by the caller, so we don't have to handle scope here. # # Args: $1 — schema key. -# Returns: 0 always; prints "||" to stdout. -_ckipper_setup_prompts_summary_row() { +# Returns: 0 always; writes two lines (no trailing blank) to stdout. +_ckipper_setup_prompts_summary_card() { local key="$1" - local value source raw + local value source raw description display_value value=$(_core_config_get "$key") raw=$(_core_config_read_global "$key") if [[ -z "$raw" ]]; then @@ -52,7 +57,14 @@ _ckipper_setup_prompts_summary_row() { else source="$_CKIPPER_SETUP_PROMPTS_SOURCE_USER" fi - printf '%s|%s|%s\n' "$key" "$value" "$source" + # Empty values render as a discoverable placeholder rather than blank + # space, which otherwise reads like "the field is broken." + display_value="$value" + [[ -z "$display_value" ]] && display_value="(empty)" + description="${_CKIPPER_SCHEMA_DESCRIPTION[$key]}" + printf '%-*s %s %s\n' \ + "$_CKIPPER_SETUP_PROMPTS_KEY_WIDTH" "$key" "$display_value" "$source" + [[ -n "$description" ]] && _core_style_color dim " $description" } # Print every global-scoped key one per line in lexical order. Used by the @@ -67,26 +79,22 @@ _ckipper_setup_prompts_global_keys() { done } -# Render the "detected configuration" summary table. Emits a styled header, -# followed by a SETTING | VALUE | SOURCE row per global-scoped key. Account -# keys are skipped because their effective value depends on which account the -# wizard is about to configure. +# Render the "detected configuration" summary as a stack of cards: each +# global key gets a ` ` line followed by a dim +# description, separated by blank lines. Replaces an earlier table-based +# rendering whose alternating wide-row + indented-description rhythm read +# as visually noisy and where long values overflowed the column padding. +# Account-scoped keys are intentionally skipped — their effective value +# depends on which account the wizard is about to configure. # # Returns: 0 always. _ckipper_setup_prompts_summary() { _core_style_header "$_CKIPPER_SETUP_PROMPTS_HEADER" - _core_style_table_print_row "SETTING|VALUE|SOURCE" - # Per-key block: aligned three-column row + indented description on the - # next line. We render this manually rather than feeding a 4-column row - # to `_core_style_table` because the schema descriptions can run 90+ - # characters and the fixed-width column padding (22 chars) would leave - # them overflowing across the screen and breaking column alignment for - # every other column. - local key description + local key first=1 while IFS= read -r key; do - _core_style_table_print_row "$(_ckipper_setup_prompts_summary_row "$key")" - description="${_CKIPPER_SCHEMA_DESCRIPTION[$key]}" - [[ -n "$description" ]] && echo " $description" + (( first )) || echo + first=0 + _ckipper_setup_prompts_summary_card "$key" done < <(_ckipper_setup_prompts_global_keys) _core_style_divider } From 6cda4b7dd65e0e7b07aff1b7beda34bf7fb5a66e Mon Sep 17 00:00:00 2001 From: Matt White Date: Mon, 4 May 2026 19:55:29 -0600 Subject: [PATCH 2/2] fix(setup): style detected-config + completion screen with gum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 polish in response to user feedback that the previous card layout "still looks bad" and that the completion screen "doesn't match the format as the rest of the setup with the color and stuff." Detected configuration Renders through `gum table -p` with a rounded border tinted to gum's prompt-accent pink (212), so the block visually belongs to the same wizard as the Yes/No prompt below it. Auto-sizes columns to longest value; over-long values (e.g. 50+ char filesystem paths) are truncated with `…` to keep the table inside narrow terminals. Descriptions intentionally drop out of the summary — they appear as labels on the pick-keys-to-customize picker (added in PR #43), where they matter most. A dim-text tip below the table points users there. Falls back to a plain key/value/source list under CKIPPER_NO_GUM (tests, CI, hosts without gum). Same data, no border. Loop locals hoisted out of the body to dodge zsh's `local var` echo-on-redeclare quirk. Completion screen Wrapped in `gum style --border rounded --padding "1 2"` with a colored build-status row (✓ green / ✗ red / ○ dim) and bold section headers inside. Plain fallback path preserved for non-gum environments. Two sections — Getting started, Maintenance — each list 3-4 commands so users can find `ckipper worktree rebuild-image` and `ckipper account sync` without re-running the wizard. Tests - 521/521 pass. - Replaced the "renders descriptions inline" regression with two new pinning the new contract: descriptions DON'T appear in the summary body, and the summary points users at the picker for them. --- lib/setup/dispatcher.zsh | 98 +++++++++++++++++++++++++++--- lib/setup/prompts.zsh | 115 ++++++++++++++++++++++++------------ lib/setup/prompts_test.bats | 24 +++++--- 3 files changed, 183 insertions(+), 54 deletions(-) diff --git a/lib/setup/dispatcher.zsh b/lib/setup/dispatcher.zsh index 21d8b36..ce91d87 100644 --- a/lib/setup/dispatcher.zsh +++ b/lib/setup/dispatcher.zsh @@ -59,15 +59,82 @@ _ckipper_setup_offer_existing_sync() { _ckipper_account_sync_dispatch } -# Print the post-setup hint block: build-status banner, review/diagnose -# commands, two ways to launch Claude (per-account aliases or `ckipper -# run`), and the bare-`ck` menu. The build-status arg lets the user spot -# a failed image build at a glance — it's easy to miss otherwise because -# 5 minutes of streaming docker output buries the completion message. +# Print the post-setup completion screen: bordered gum-styled card with a +# build-status line and two columns of commands (getting-started and +# maintenance). The whole thing is one `gum style` block so it visually +# belongs to the same wizard as the gum-rendered prompts above it. A +# build failure is easy to miss after 5 minutes of streaming docker +# output; the colored status line is the primary signal. +# +# Falls back to plain ANSI rendering when CKIPPER_NO_GUM is set. # # Args: $1 — image build status: `ok` | `failed` | `skipped`. # Returns: 0 always. _ckipper_setup_print_completion_summary() { + local image_status="$1" + if _ckipper_setup_completion_use_gum; then + _ckipper_setup_render_completion_gum "$image_status" + else + _ckipper_setup_render_completion_plain "$image_status" + fi +} + +# Mirror of `_core_prompt_use_gum` — kept private so the completion path +# does not pull `_core_prompt_*` into its dependency surface. +# +# Returns: 0 if gum should drive rendering; 1 for the plain fallback. +_ckipper_setup_completion_use_gum() { + [[ "$CKIPPER_NO_GUM" == "1" ]] && return 1 + command -v gum >/dev/null 2>&1 +} + +# Render the completion screen via `gum style`. Pre-builds the inner +# content as a multi-line string so the border wraps the whole block. +# +# Args: $1 — image status (`ok` | `failed` | `skipped`). +# Returns: 0 always. +_ckipper_setup_render_completion_gum() { + local image_status="$1" + local content + content=$(_ckipper_setup_completion_inner "$image_status") + gum style \ + --border rounded \ + --padding "1 2" \ + --border-foreground "$_CKIPPER_SETUP_PROMPTS_BORDER_FG" \ + "$content" +} + +# Build the multi-line text content that goes inside the bordered card. +# The image-status line uses gum's foreground colors directly so the +# border block stays a single styled call. Sections are separated by +# blank lines for visual rhythm inside the card. +# +# Args: $1 — image status. +# Returns: 0 always; prints the multi-line content to stdout. +_ckipper_setup_completion_inner() { + local image_status="$1" + gum style --bold --foreground "$_CKIPPER_SETUP_PROMPTS_BORDER_FG" "Setup complete" + echo + _ckipper_setup_render_image_status_gum "$image_status" + echo + gum style --bold "Getting started:" + echo " ckipper run Bundle worktree + Claude" + echo " ck Interactive menu" + echo " claude- Per-account launcher (e.g. claude-personal)" + echo + gum style --bold "Maintenance:" + echo " ckipper config list Review every setting" + echo " ckipper doctor Diagnose installation issues" + echo " ckipper worktree rebuild-image Rebuild ckipper-dev Docker image" + echo " ckipper account sync Copy settings between accounts" +} + +# Plain-text completion screen for non-gum environments (CI, tests). Same +# information, no border or color. +# +# Args: $1 — image status. +# Returns: 0 always. +_ckipper_setup_render_completion_plain() { local image_status="$1" _core_style_header "Setup complete" _ckipper_setup_render_image_status "$image_status" @@ -84,9 +151,24 @@ _ckipper_setup_print_completion_summary() { echo "" } -# Render a single banner line about the docker image build outcome. Helps -# the user notice a build failure that would otherwise scroll past with -# the rest of `docker build` output. +# Render the docker-build-status line for the gum path using gum's +# foreground color codes (gum-color 46 = bright green, 196 = red, 244 = +# dim gray) so it nests cleanly inside the surrounding `gum style` block. +# +# Args: $1 — `ok` | `failed` | `skipped`. +# Returns: 0 always. +_ckipper_setup_render_image_status_gum() { + case "$1" in + ok) gum style --foreground 46 "✓ Docker image: built successfully." ;; + failed) gum style --foreground 196 "✗ Docker image: build FAILED — re-run: ckipper worktree rebuild-image" ;; + skipped) gum style --foreground 244 "○ Docker image: skipped — build later: ckipper worktree rebuild-image" ;; + esac +} + +# Plain-text image-status line (no gum). Uses the existing _core_style +# color palette so terminals that support ANSI still get a coloured +# banner; the no-color path falls through to plain text via +# _core_style_color's enablement check. # # Args: $1 — `ok` | `failed` | `skipped`. # Returns: 0 always. diff --git a/lib/setup/prompts.zsh b/lib/setup/prompts.zsh index 4d11366..1c07dbe 100644 --- a/lib/setup/prompts.zsh +++ b/lib/setup/prompts.zsh @@ -36,35 +36,50 @@ readonly _CKIPPER_SETUP_PROMPTS_HEADER="Detected configuration" # row and silently advance with no overrides. readonly _CKIPPER_SETUP_PROMPTS_PICKER_HEADER="Pick keys to customize (SPACE to mark, ENTER to confirm)" -# Width of the SETTING column in the card-style summary. 22 chars covers +# Border-foreground color for gum-styled summary blocks. 212 is gum's +# default pink — matches the prompt accent (Yes/No buttons) so the +# detected-config block visually belongs to the same wizard. +readonly _CKIPPER_SETUP_PROMPTS_BORDER_FG=212 + +# SETTING column width for the no-gum fallback summary. 22 chars covers # every key in the current schema (longest is `aliases_auto_source` at 19) # with a 3-char gutter before the value. readonly _CKIPPER_SETUP_PROMPTS_KEY_WIDTH=22 -# Render one schema key as a two-line "card": ` ` -# on line 1 and the dim-colored description indented on line 2. Account -# keys are filtered out by the caller, so we don't have to handle scope here. +# Maximum width of the VALUE cell before truncation. Caps the table width +# so it stays readable in narrow terminals and during shared-screen demos +# without hiding the at-a-glance setting/source signal — the full value +# is one `ckipper config get ` away. +readonly _CKIPPER_SETUP_PROMPTS_VALUE_MAX_WIDTH=40 + +# Build the pipe-separated row data for the summary table — one header +# row plus one row per global-scoped key. Emits to stdout for callers to +# pipe into `gum table -p` or to consume directly in fallback rendering. # -# Args: $1 — schema key. -# Returns: 0 always; writes two lines (no trailing blank) to stdout. -_ckipper_setup_prompts_summary_card() { - local key="$1" - local value source raw description display_value - value=$(_core_config_get "$key") - raw=$(_core_config_read_global "$key") - if [[ -z "$raw" ]]; then - source="$_CKIPPER_SETUP_PROMPTS_SOURCE_DEFAULT" - else - source="$_CKIPPER_SETUP_PROMPTS_SOURCE_USER" - fi - # Empty values render as a discoverable placeholder rather than blank - # space, which otherwise reads like "the field is broken." - display_value="$value" - [[ -z "$display_value" ]] && display_value="(empty)" - description="${_CKIPPER_SCHEMA_DESCRIPTION[$key]}" - printf '%-*s %s %s\n' \ - "$_CKIPPER_SETUP_PROMPTS_KEY_WIDTH" "$key" "$display_value" "$source" - [[ -n "$description" ]] && _core_style_color dim " $description" +# Returns: 0 always; prints `||` rows (header first). +_ckipper_setup_prompts_summary_rows() { + printf 'SETTING|VALUE|SOURCE\n' + # Hoist loop locals — see fallback rendering for the reason. + local key="" value="" source="" raw="" display_value="" + while IFS= read -r key; do + value=$(_core_config_get "$key") + raw=$(_core_config_read_global "$key") + if [[ -z "$raw" ]]; then + source="$_CKIPPER_SETUP_PROMPTS_SOURCE_DEFAULT" + else + source="$_CKIPPER_SETUP_PROMPTS_SOURCE_USER" + fi + # Empty values render as a discoverable placeholder rather than + # blank space, which otherwise reads like "the field is broken." + display_value="$value" + [[ -z "$display_value" ]] && display_value="(empty)" + # Truncate over-long values so the table doesn't blow past + # narrow terminals. Trailing `…` signals truncation. + if (( ${#display_value} > _CKIPPER_SETUP_PROMPTS_VALUE_MAX_WIDTH )); then + display_value="${display_value[1,_CKIPPER_SETUP_PROMPTS_VALUE_MAX_WIDTH-1]}…" + fi + printf '%s|%s|%s\n' "$key" "$display_value" "$source" + done < <(_ckipper_setup_prompts_global_keys) } # Print every global-scoped key one per line in lexical order. Used by the @@ -79,24 +94,48 @@ _ckipper_setup_prompts_global_keys() { done } -# Render the "detected configuration" summary as a stack of cards: each -# global key gets a ` ` line followed by a dim -# description, separated by blank lines. Replaces an earlier table-based -# rendering whose alternating wide-row + indented-description rhythm read -# as visually noisy and where long values overflowed the column padding. -# Account-scoped keys are intentionally skipped — their effective value -# depends on which account the wizard is about to configure. +# Render the detected-configuration summary as a `gum table -p` styled +# table — auto-sizes columns to the longest value in each column and +# adapts to the terminal width. Schema descriptions are intentionally +# omitted from the summary (they were a 4th-column overflow problem in +# both prior layouts); descriptions surface as labels in the +# pick-keys-to-customize picker (` — `), so the user +# sees them at the moment they're deciding what to change. +# +# Falls back to a plain-text two-column layout under CKIPPER_NO_GUM (tests, +# non-TTY callers, and runners without gum installed). # # Returns: 0 always. _ckipper_setup_prompts_summary() { _core_style_header "$_CKIPPER_SETUP_PROMPTS_HEADER" - local key first=1 - while IFS= read -r key; do - (( first )) || echo - first=0 - _ckipper_setup_prompts_summary_card "$key" - done < <(_ckipper_setup_prompts_global_keys) - _core_style_divider + if _ckipper_setup_prompts_use_gum; then + _ckipper_setup_prompts_summary_rows \ + | gum table -p -s '|' --border rounded \ + --border.foreground "$_CKIPPER_SETUP_PROMPTS_BORDER_FG" + else + _ckipper_setup_prompts_summary_fallback + fi + _core_style_color dim \ + "Tip: pick a setting below to see its description and edit it." +} + +# Plain-text fallback when gum is unavailable. Renders the same data as +# ` ` — no borders, no colors, but parsable for +# tests and non-TTY callers. +# +# Returns: 0 always. +_ckipper_setup_prompts_summary_fallback() { + # Hoist loop locals outside the body — re-declaring `local var` (no + # =value) on iteration N>1 makes zsh print `var='prior_value'` since + # the variable carries a value from the previous iteration. Same idiom + # used in lib/worktree/worktree.zsh's list helper for the same reason. + local row="" key="" value="" source="" first=1 + while IFS= read -r row; do + (( first )) && { first=0; continue; } # skip header row + IFS='|' read -r key value source <<<"$row" + printf '%-*s %-28s %s\n' \ + "$_CKIPPER_SETUP_PROMPTS_KEY_WIDTH" "$key" "$value" "$source" + done < <(_ckipper_setup_prompts_summary_rows) } # Decide whether to use gum for the picker. Mirrors `_core_prompt_use_gum` but diff --git a/lib/setup/prompts_test.bats b/lib/setup/prompts_test.bats index 6ac4f8e..f8aa0ca 100644 --- a/lib/setup/prompts_test.bats +++ b/lib/setup/prompts_test.bats @@ -67,17 +67,25 @@ _run_prompts() { [[ "$output" != *"ssh_forward"* ]] } -# Regression: descriptions used to live in a 4th column, which overflowed -# the fixed-width table because zsh's `printf '%-22s'` does not truncate. -# They now render on the line below each row, indented two spaces. -@test "_ckipper_setup_prompts_summary renders each schema description below its row" { +# Regression: descriptions intentionally do NOT appear in the summary +# anymore — they live in the pick-keys-to-customize picker labels (added +# in PR #43) so the user sees them at the moment they decide what to +# change. The summary stays compact and renders cleanly through gum's +# styled table. This test pins the new contract: descriptions in picker, +# not summary. +@test "_ckipper_setup_prompts_summary does not embed descriptions inline" { _run_prompts "" "_ckipper_setup_prompts_summary" [ "$status" -eq 0 ] - # Each schema description should appear verbatim somewhere in the output. - [[ "$output" == *"Bool. true = installer auto-adds the per-account aliases source line"* ]] - [[ "$output" == *"Path. Base directory containing your git projects."* ]] - [[ "$output" == *"Comma-separated int list. Container ports to forward to the host."* ]] + [[ "$output" != *"Bool. true = installer auto-adds the per-account aliases source line"* ]] + [[ "$output" != *"Comma-separated int list. Container ports to forward to the host."* ]] +} + +@test "_ckipper_setup_prompts_summary points at the picker for descriptions" { + _run_prompts "" "_ckipper_setup_prompts_summary" + + [ "$status" -eq 0 ] + [[ "$output" == *"description"* ]] || [[ "$output" == *"Tip:"* ]] } @test "_ckipper_setup_prompts_summary marks set values as (your config) and unset as (default)" {