Skip to content

fix: Fixes issue with script not running correctly in zsh enviornment. #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nicolethoen merged 1 commit into from
Apr 23, 2026
Merged

fix: Fixes issue with script not running correctly in zsh enviornment. #36

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 49 additions & 4 deletions scripts/install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@
# Install Node (via nvm + Node.js 24 when Node is missing), enable Corepack,
# install GitHub CLI when missing, and install @patternfly/patternfly-cli globally.

# Ensure this script runs in bash, even if invoked with zsh
if [ -z "${BASH_VERSION:-}" ]; then
exec bash "$0" "$@"
fi

set -euo pipefail

if [ -z "${HOME:-}" ]; then
Expand Down Expand Up @@ -48,6 +53,40 @@ ensure_nvm_loaded() {
return 1
}

setup_nvm_for_zsh() {
# Add nvm to .zshrc only if zsh is the user's default shell
if [ -z "${SHELL:-}" ]; then
return 0
fi

case "$SHELL" in
*/zsh)
;;
*)
return 0
;;
esac

local zshrc="${HOME}/.zshrc"
if [ ! -f "$zshrc" ]; then
touch "$zshrc"
fi

if grep -q 'NVM_DIR' "$zshrc" 2>/dev/null; then
info "nvm is already configured in ${zshrc}"
return 0
fi

info "Adding nvm to ${zshrc} for zsh compatibility."
cat >> "$zshrc" << 'EOF'

# Load nvm (Node Version Manager)
export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"
Comment on lines +70 to +86
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Persist the actual NVM_DIR instead of hardcoding ~/.nvm.

ensure_nvm_loaded supports custom NVM_DIR, but the zsh snippet always writes export NVM_DIR="$HOME/.nvm". Users installing nvm into a custom location will get a broken future zsh session.

Proposed fix 
-  local zshrc="${HOME}/.zshrc"
+  local zshrc="${HOME}/.zshrc"
+  local nvm_dir="${NVM_DIR:-$HOME/.nvm}"
@@
-  cat >> "$zshrc" << 'EOF'
+  cat >> "$zshrc" << EOF
 
 # Load nvm (Node Version Manager)
-export NVM_DIR="$HOME/.nvm"
-[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
-[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"
+export NVM_DIR="${nvm_dir}"
+[ -s "\$NVM_DIR/nvm.sh" ] && \. "\$NVM_DIR/nvm.sh"
+[ -s "\$NVM_DIR/bash_completion" ] && \. "\$NVM_DIR/bash_completion"
 EOF
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
local zshrc="${HOME}/.zshrc"
if [ ! -f "$zshrc" ]; then
touch "$zshrc"
fi
if grep -q 'NVM_DIR' "$zshrc" 2>/dev/null; then
info "nvm is already configured in ${zshrc}"
return 0
fi
info "Adding nvm to ${zshrc} for zsh compatibility."
cat >> "$zshrc" << 'EOF'
# Load nvm (Node Version Manager)
export NVM_DIR="$HOME/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
[ -s "$NVM_DIR/bash_completion" ] && \. "$NVM_DIR/bash_completion"
local zshrc="${HOME}/.zshrc"
local nvm_dir="${NVM_DIR:-$HOME/.nvm}"
if [ ! -f "$zshrc" ]; then
touch "$zshrc"
fi
if grep -q 'NVM_DIR' "$zshrc" 2>/dev/null; then
info "nvm is already configured in ${zshrc}"
return 0
fi
info "Adding nvm to ${zshrc} for zsh compatibility."
cat >> "$zshrc" << EOF
# Load nvm (Node Version Manager)
export NVM_DIR="${nvm_dir}"
[ -s "\$NVM_DIR/nvm.sh" ] && \. "\$NVM_DIR/nvm.sh"
[ -s "\$NVM_DIR/bash_completion" ] && \. "\$NVM_DIR/bash_completion"
EOF
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/install.sh` around lines 70 - 86, The zsh snippet in
scripts/install.sh currently hardcodes export NVM_DIR="$HOME/.nvm", which breaks
custom installs; update the code that appends to the zshrc (the block that
writes to the variable zshrc) so it persists the actual NVM_DIR value instead of
hardcoding ~/.nvm—write the export using parameter expansion to prefer an
existing environment NVM_DIR and fall back to $HOME/.nvm (mirror the behavior
used by ensure_nvm_loaded), and keep the subsequent checks that source nvm.sh
and bash_completion unchanged.

EOF
}

install_node_via_nvm() {
info "Node.js not found. Installing nvm (${NVM_VERSION}) and Node.js 24."
if ! require_cmd curl && ! require_cmd wget; then
Expand All @@ -71,6 +110,8 @@ install_node_via_nvm() {
nvm use 24 || error "nvm failed to activate Node.js 24."
nvm alias default 24 2>/dev/null || true

setup_nvm_for_zsh

info "Node.js $(node --version) and npm $(npm --version) are ready (npm ships with this Node release)."
}

Expand Down Expand Up @@ -245,16 +286,20 @@ main() {
ensure_gh
install_patternfly_cli

printf '\n'
printf 'SUCCESS: PatternFly CLI is installed.\n'
printf '\n'
printf 'To see available commands, run the CLI with:\n'
printf ' patternfly-cli --help\n'
printf '\n'
printf 'alternatively you can use the alias "pf" instead of "pattenfly-cli"\n'
printf '\n'
printf 'Alternatively you can use the alias "pf" instead of "patternfly-cli":\n'
printf ' pf --help\n'
printf '\n'
printf 'SUCCESS: PatternFly CLI is installed.\n'
printf 'NOTE: If the command is not found, restart your terminal or run:\n'
printf ' source ~/.zshrc # for zsh users\n'
printf ' source ~/.bashrc # for bash users\n'
printf '\n'

}

main "$@"
Loading