diff --git a/AGENTS.md b/AGENTS.md index 1cfb3ec..ca8effd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,7 +79,7 @@ CI runs on **ubuntu-latest** and **macos-latest** (GitHub Actions, `.github/work ## Implementation Conventions ### Shell Functions -- **Logging**: `__profile_log_{error,warn,info,success}` from `scripts/lib.zsh`; start each step of a setup script with `__profile_log_section ` (bold, blue background) +- **Logging**: `__profile_log_{error,warn,info,success}` from `scripts/lib.zsh`; start each step of a setup script with `__profile_log_section <title>` (bold, blue background). `error` and `warn` write to stderr, the others to stdout; never add `>&2` at a call site - **Tree search**: `__profile_search_ancestor_tree <filename>` walks up from `$PWD` to `/` looking for a file - **Private functions**: Prefix with `__profile_` to indicate internal use - **Public functions** in `include/functions` are user-facing shell commands (e.g., `start`, `jest`, `alias_last`, `wt`) diff --git a/include/functions b/include/functions index fb9a062..b001803 100644 --- a/include/functions +++ b/include/functions @@ -136,7 +136,7 @@ function __profile_git_repo_root() { local git_common_dir git_common_dir=$(git rev-parse --git-common-dir 2>/dev/null) if [[ $? -ne 0 ]]; then - __profile_log_error "not inside a git repository" >&2 + __profile_log_error "not inside a git repository" return 1 fi # Resolve to absolute path (relative when in main worktree, absolute in secondary) @@ -148,7 +148,7 @@ function __profile_git_repo_root() { # main and master are the fallback for repos that were created locally. function __profile_git_default_branch() { git rev-parse --git-dir >/dev/null 2>&1 || { - __profile_log_error "not inside a git repository" >&2 + __profile_log_error "not inside a git repository" return 1 } @@ -166,7 +166,7 @@ function __profile_git_default_branch() { fi done - __profile_log_error "cannot find the default branch (set it with: git remote set-head origin --auto)" >&2 + __profile_log_error "cannot find the default branch (set it with: git remote set-head origin --auto)" return 1 } diff --git a/scripts/lib.zsh b/scripts/lib.zsh index 0689716..00d2160 100644 --- a/scripts/lib.zsh +++ b/scripts/lib.zsh @@ -2,11 +2,11 @@ autoload -U colors && colors function __profile_log_error() { - __profile_log_log "red" "ERROR" $1 + __profile_log_log "red" "ERROR" $1 >&2 } function __profile_log_warn() { - __profile_log_log "yellow" "WARNING" $1 + __profile_log_log "yellow" "WARNING" $1 >&2 } function __profile_log_info() { diff --git a/scripts/profile b/scripts/profile index 92eeac4..45f9202 100755 --- a/scripts/profile +++ b/scripts/profile @@ -51,11 +51,11 @@ function __profile_cmd_update() { __profile_log_section "Repository" git -C "$repo" pull origin "$(git -C "$repo" branch --show-current)" || - { __profile_log_error "git pull failed" >&2; return 1; } + { __profile_log_error "git pull failed"; return 1; } __profile_log_section "Submodules" git -C "$repo" submodule update --init --recursive || - { __profile_log_error "git submodule update failed" >&2; return 1; } + { __profile_log_error "git submodule update failed"; return 1; } __profile_log_section "Orphaned submodules" __profile_cmd_remove_orphans "$repo" @@ -74,7 +74,7 @@ function __profile_cmd_listed_submodules() { code=$? # Exit code 1 means no match. if (( code > 1 )); then - __profile_log_error "could not read $repo/.gitmodules (exit code $code)" >&2 + __profile_log_error "could not read $repo/.gitmodules (exit code $code)" return 1 fi for key in "${(@f)output}"; do @@ -93,7 +93,7 @@ function __profile_cmd_submodule_sections() { output="$(git -C "$repo" config --local --name-only --get-regexp '^submodule\..+\.[^.]+$')" code=$? if (( code > 1 )); then - __profile_log_error "could not read the submodule sections of $repo/.git/config (exit code $code)" >&2 + __profile_log_error "could not read the submodule sections of $repo/.git/config (exit code $code)" return 1 fi for key in "${(@f)output}"; do @@ -114,7 +114,7 @@ function __profile_cmd_submodule_gitdirs() { # The common dir, not <repo>/.git, so that a linked worktree finds the shared modules. common="$(git -C "$repo" rev-parse --git-common-dir)" || - { __profile_log_error "could not find the git directory of $repo" >&2; return 1; } + { __profile_log_error "could not find the git directory of $repo"; return 1; } if [[ -n "$common" ]]; then [[ "$common" == /* ]] || common="$repo/$common" REPLY="${common:A}/modules" @@ -220,7 +220,7 @@ function __profile_cmd_remove_orphan() { worktree="$(__profile_cmd_submodule_worktree "$repo" "$name" "$gitdir")" if [[ -n "$gitdir" && "${gitdir:A}" != "$modules"/* ]] || [[ "$worktree" == "$repo" || "$worktree" != "$repo"/* ]]; then - __profile_log_warn "kept orphaned submodule '$name': a path outside the repo" >&2 + __profile_log_warn "kept orphaned submodule '$name': a path outside the repo" return 0 fi # Other content can occupy the path, for example files that the repo now tracks there. @@ -229,7 +229,7 @@ function __profile_cmd_remove_orphan() { local REPLY="" __profile_cmd_orphan_keep_reason "$gitdir" "$live_worktree" if [[ -n "$REPLY" ]]; then - __profile_log_warn "kept orphaned submodule '$name': $REPLY" >&2 + __profile_log_warn "kept orphaned submodule '$name': $REPLY" return 0 fi @@ -267,7 +267,7 @@ function __profile_cmd_remove_orphans() { for name in "${candidates[@]}"; do (( ${listed[(Ie)$name]} )) && continue if ! __profile_cmd_remove_orphan "$repo" "$name" "$modules" "${sections[(Ie)$name]}"; then - __profile_log_error "failed to remove orphaned submodule '$name'" >&2 + __profile_log_error "failed to remove orphaned submodule '$name'" failed=1 fi done @@ -292,7 +292,7 @@ function __profile_cmd_known_profiles() { done fi if (( ! ${#reply} )); then - __profile_log_error "no 'known' profile list in $brewfile" >&2 + __profile_log_error "no 'known' profile list in $brewfile" return 1 fi } @@ -327,7 +327,7 @@ function __profile_cmd_install() { return 2 fi if (( ! ${known_profiles[(Ie)$name]} )); then - __profile_log_error "unknown machine profile: '$name'" >&2 + __profile_log_error "unknown machine profile: '$name'" return 2 fi profiles+=("$name") @@ -338,9 +338,9 @@ function __profile_cmd_install() { if (( $+commands[brew] )); then HOMEBREW_PROFILE_INSTALL_PROFILES="${(j: :)profiles}" \ brew bundle --no-upgrade --file "$repo/brew/Brewfile" </dev/null || - { __profile_log_error "brew bundle failed" >&2; return 1; } + { __profile_log_error "brew bundle failed"; return 1; } else - __profile_log_warn "brew is not on PATH. Skipping brew bundle..." >&2 + __profile_log_warn "brew is not on PATH. Skipping brew bundle..." fi # The nvim plugin sync and the mise runtimes belong to the dev setup; essentials runs stock nvim. @@ -358,18 +358,18 @@ function __profile_cmd_install() { nvim --headless \ --cmd 'lua vim.g.profile_sync = true' \ "+lua local ok, err = pcall(vim.cmd, 'ProfileSync') if not ok then io.stderr:write(tostring(err) .. '\n') vim.cmd('cquit 1') end" \ - </dev/null || { __profile_log_error "nvim plugin sync failed" >&2; return 1; } + </dev/null || { __profile_log_error "nvim plugin sync failed"; return 1; } else - __profile_log_warn "nvim is not on PATH. Skipping the nvim plugin sync..." >&2 + __profile_log_warn "nvim is not on PATH. Skipping the nvim plugin sync..." fi __profile_log_section "mise runtimes" if (( $+commands[mise] )); then # Project config in the caller's directory (mise.toml, .tool-versions, and the idiomatic # files enabled by config/mise/profile.toml) would otherwise replace the machine-profile runtimes. - (cd / && mise install </dev/null) || { __profile_log_error "mise install failed" >&2; return 1; } + (cd / && mise install </dev/null) || { __profile_log_error "mise install failed"; return 1; } else - __profile_log_warn "mise is not on PATH. Skipping mise install..." >&2 + __profile_log_warn "mise is not on PATH. Skipping mise install..." fi } @@ -472,7 +472,7 @@ function __profile_cmd_unlisted_candidates() { shift 2 entries="$(__profile_cmd_profile_entries "$profiles" "$brewfile")" || - { __profile_log_error "the profile entries could not be listed" >&2; return 1; } + { __profile_log_error "the profile entries could not be listed"; return 1; } for word in "${(@f)entries}"; do [[ -n "$word" ]] || continue type="${word%%:*}" name="${word#*:}" @@ -489,12 +489,12 @@ function __profile_cmd_unlisted_candidates() { for name in "${(@f)$(__profile_cmd_names_of_type "$type" "$@")}"; do [[ -n "$name" ]] || continue if ! names=("${(@f)$(__profile_cmd_package_names "$type" "$name")}"); then - __profile_log_warn "kept $name: its names could not be read" >&2 + __profile_log_warn "kept $name: its names could not be read" continue fi names=("${(@f)$(__profile_cmd_with_short_names "${names[@]}")}") if (( ${#${(@)names:*entry_names}} )); then - __profile_log_warn "kept $name: a profile lists it" >&2 + __profile_log_warn "kept $name: a profile lists it" continue fi print -r -- "${type}:${name}" @@ -503,7 +503,7 @@ function __profile_cmd_unlisted_candidates() { for name in "${(@f)$(__profile_cmd_names_of_type tap "$@")}"; do [[ -n "$name" ]] || continue if (( ${entry_taps[(Ie)$name]} )); then - __profile_log_warn "kept $name: a profile lists it" >&2 + __profile_log_warn "kept $name: a profile lists it" continue fi print -r -- "tap:${name}" @@ -562,14 +562,14 @@ function __profile_cmd_cleanup_brew() { brew bundle cleanup --formula --cask --tap --file "$brewfile" </dev/null)" code=$? if (( code > 1 )); then - __profile_log_error "the package list failed with exit code $code" >&2 + __profile_log_error "the package list failed with exit code $code" return 1 fi candidates=("${(@f)$(__profile_cmd_cleanup_candidates "$output")}") candidates=("${(@)candidates:#}") # A list-only run that exits with 1 always prints a list, so its absence means Homebrew failed. if (( code == 1 && ! ${#candidates} )); then - __profile_log_error "Homebrew failed with no package list (exit code $code)" >&2 + __profile_log_error "Homebrew failed with no package list (exit code $code)" return 1 fi if (( ${#candidates} )); then @@ -592,7 +592,7 @@ function __profile_cmd_cleanup_brew() { # The list-only run is not repeated with --force: Homebrew would compute its own list # again, without the guard above. __profile_cmd_remove_packages "${candidates[@]}" || - { __profile_log_error "the package removal failed" >&2; return 1; } + { __profile_log_error "the package removal failed"; return 1; } } function __profile_cmd_cleanup_mise() { @@ -602,14 +602,14 @@ function __profile_cmd_cleanup_mise() { # mise writes the `prune --dry-run` list to stderr, and `--dry-run-code` exits with 1 # both when versions are prunable and on a config error, so only this JSON is reliable. json="$(cd / && mise ls --prunable --json </dev/null)" || - { __profile_log_error "the runtime list failed with exit code $?" >&2; return 1; } + { __profile_log_error "the runtime list failed with exit code $?"; return 1; } if (( ! $+commands[jq] )); then - __profile_log_error "jq is not on PATH, so the runtime list cannot be read" >&2 + __profile_log_error "jq is not on PATH, so the runtime list cannot be read" return 1 fi list="$(print -r -- "$json" | jq -r 'to_entries[] | .key as $tool | .value[] | "\($tool)@\(.version)"')" || - { __profile_log_error "the runtime list could not be read" >&2; return 1; } + { __profile_log_error "the runtime list could not be read"; return 1; } local -a versions=("${(@f)list}") versions=("${(@)versions:#}") @@ -623,7 +623,7 @@ function __profile_cmd_cleanup_mise() { return 0 fi (cd / && mise prune --tools --yes </dev/null) || - { __profile_log_error "the runtime removal failed" >&2; return 1; } + { __profile_log_error "the runtime removal failed"; return 1; } } function __profile_cmd_cleanup() { @@ -642,14 +642,14 @@ function __profile_cmd_cleanup() { # A subset would make the packages of the other profiles look unlisted. __profile_cmd_cleanup_brew "${(j: :)reply}" || return 1 else - __profile_log_warn "brew is not on PATH. Skipping brew bundle cleanup..." >&2 + __profile_log_warn "brew is not on PATH. Skipping brew bundle cleanup..." fi __profile_log_section "mise prune" if (( $+commands[mise] )); then __profile_cmd_cleanup_mise || return 1 else - __profile_log_warn "mise is not on PATH. Skipping mise prune..." >&2 + __profile_log_warn "mise is not on PATH. Skipping mise prune..." fi } diff --git a/tests/log_streams.test.sh b/tests/log_streams.test.sh new file mode 100755 index 0000000..3800a2c --- /dev/null +++ b/tests/log_streams.test.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env zsh + +source "$SHA1N_PROFILE_TESTS_HOME/sandbox.zsh" + +log_stdout() { + env -i HOME="$HOME" PATH="$PATH" TERM=dumb zsh -fc \ + 'source "$1/scripts/lib.zsh" && "$2" message' zsh "$profile_home" "$1" 2>/dev/null +} + +log_stderr() { + env -i HOME="$HOME" PATH="$PATH" TERM=dumb zsh -fc \ + 'source "$1/scripts/lib.zsh" && "$2" message' zsh "$profile_home" "$1" 2>&1 >/dev/null +} + +function test_diagnostics_go_to_stderr() { + test_case_title + + local fn + for fn in __profile_log_error __profile_log_warn; do + assert_empty "$(log_stdout "$fn")" + assert_contains "$(log_stderr "$fn")" "message" + done +} + +function test_progress_goes_to_stdout() { + test_case_title + + local fn + for fn in __profile_log_info __profile_log_success __profile_log_section; do + assert_contains "$(log_stdout "$fn")" "message" + assert_empty "$(log_stderr "$fn")" + done +} + +setup +run_test test_diagnostics_go_to_stderr +run_test test_progress_goes_to_stdout +cleanup +finish_tests