eeymoo/nvm
1
0
Fork 0
mirror of https://github.com/nvm-sh/nvm.git synced 2026-09-09 00:00:07 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

[Fix] nvm --help: render the set-colors legend in color

Browse Source 
Every legend line is built inside a command substitution,
where `nvm_has_colors` can never succeed,
because `[ -t 1 ]` sees the capture pipe rather than the terminal.
The legend has therefore always printed plain,
on every terminal and in every shell.

Resolve color support once and pass it in,
the way `nvm_print_alias_path` already accepts the same flag.
A command-prefix assignment cannot carry it:
POSIX expands a command's arguments before applying its assignments,
so the substitutions would still see it unset,
and in sh and ksh it would outlive the call and override `--no-colors` for everything after.

The test needs no tty,
and fails against an unfixed `nvm.sh` in sh, bash, dash, and zsh.

Fixes #3896
This commit is contained in:
CJstate
2026-09-03 12:10:19 -07:00
committed by Jordan Harband
parent 38f0f2cfdf
commit 3a72926f4a
2 changed files with 113 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nvm.sh
+15 -4
View File
@@ -1129,6 +1129,8 @@ nvm_set_colors() {
local CURRENT_COLOR
local NOT_INSTALLED_COLOR
local DEFAULT_COLOR
local NVM_HAS_COLORS
NVM_HAS_COLORS=0
INSTALLED_COLOR="$(echo "$1" | awk '{ print substr($0, 1, 1); }')"
LTS_AND_SYSTEM_COLOR="$(echo "$1" | awk '{ print substr($0, 2, 1); }')"
@@ -1139,6 +1141,7 @@ nvm_set_colors() {
nvm_echo "Setting colors to: ${INSTALLED_COLOR} ${LTS_AND_SYSTEM_COLOR} ${CURRENT_COLOR} ${NOT_INSTALLED_COLOR} ${DEFAULT_COLOR}"
nvm_echo "WARNING: Colors may not display because they are not supported in this shell."
else
NVM_HAS_COLORS=1
nvm_echo_with_colors "Setting colors to: $(nvm_wrap_with_color_code "${INSTALLED_COLOR}" "${INSTALLED_COLOR}")$(nvm_wrap_with_color_code "${LTS_AND_SYSTEM_COLOR}" "${LTS_AND_SYSTEM_COLOR}")$(nvm_wrap_with_color_code "${CURRENT_COLOR}" "${CURRENT_COLOR}")$(nvm_wrap_with_color_code "${NOT_INSTALLED_COLOR}" "${NOT_INSTALLED_COLOR}")$(nvm_wrap_with_color_code "${DEFAULT_COLOR}" "${DEFAULT_COLOR}")"
fi
export NVM_COLORS="$1"
@@ -1176,7 +1179,10 @@ nvm_wrap_with_color_code() {
CODE="$(nvm_print_color_code "${1}" 2>/dev/null ||:)"
local TEXT
TEXT="${2-}"
if nvm_has_colors && [ -n "${CODE}" ]; then
# `nvm_has_colors` cannot answer for itself inside a command substitution,
# where `[ -t 1 ]` sees the capture pipe: callers that already checked pass
# the answer in, as `nvm_print_alias_path` also accepts it.
if { [ "${NVM_HAS_COLORS-}" = 1 ] || nvm_has_colors; } && [ -n "${CODE}" ]; then
nvm_echo_with_colors "\033[${CODE}${TEXT}\033[0m"
else
nvm_echo "${TEXT}"
@@ -1184,9 +1190,14 @@ nvm_wrap_with_color_code() {
}
nvm_print_color_legend() {
# Every line is built in a command substitution, so this cannot stay
# inline: resolving color support has to happen in one scope that all of
# them share. Pure code motion for now; see the following commit.
# Every line below builds its text in a command substitution, which cannot
# detect color support itself, so resolve it once here. `local` matters:
# leaking the flag would override `--no-colors` for every later call.
local NVM_HAS_COLORS
NVM_HAS_COLORS=0
if nvm_has_colors; then
NVM_HAS_COLORS=1
fi
nvm_echo ' Initial colors are:'
nvm_echo_with_colors " $(nvm_wrap_with_color_code 'b' 'b')$(nvm_wrap_with_color_code 'y' 'y')$(nvm_wrap_with_color_code 'g' 'g')$(nvm_wrap_with_color_code 'r' 'r')$(nvm_wrap_with_color_code 'e' 'e')"
nvm_echo ' Color codes:'
test/fast/Unit tests/nvm_print_color_legend
+98 -1
View File
@@ -1,11 +1,21 @@
#!/bin/sh
die () { echo "$@" ; exit 1; }
ONESHOT=''
cleanup () {
if [ -n "${ONESHOT}" ]; then
command rm -f "${ONESHOT}"
fi
}
die () { echo "$@" ; cleanup ; exit 1; }
\. ../../../nvm.sh
set -e
# assert about nvm, not about the caller's environment
NVM_HAS_COLORS_WAS="${NVM_HAS_COLORS-}"
unset NVM_HAS_COLORS
ESC="$(printf '\033')"
# The legend's letters are wrapped in color codes when the terminal supports
# them, so every text assertion below compares against the output with any
@@ -77,6 +87,87 @@ HELP_PLAIN="$(nvm --help | strip_colors)"
contains_line ' Color codes:' "${HELP_PLAIN}" \
|| die 'nvm --help no longer includes the color legend'
# --- the fix: colors have to survive command substitution ------------------
# the bug condition: inside `$( )`, `[ -t 1 ]` sees the capture pipe, so
# `nvm_has_colors` always fails
nvm_has_colors () { return 1; }
WRAPPED="$(nvm_wrap_with_color_code 'b' 'b')"
case "${WRAPPED}" in
*"${ESC}"*) die "expected plain text when colors are unsupported, got >${WRAPPED}<" ;;
esac
NVM_HAS_COLORS=1
export NVM_HAS_COLORS
WRAPPED="$(nvm_wrap_with_color_code 'b' 'b')"
[ "${WRAPPED}" = "${ESC}[0;34mb${ESC}[0m" ] \
|| die "expected a colored 'b' with NVM_HAS_COLORS=1, got >${WRAPPED}<"
WRAPPED="$(nvm_wrap_with_color_code 'X' 'X')"
[ "${WRAPPED}" = 'X' ] \
|| die "expected plain text for an unknown color code, got >${WRAPPED}<"
unset NVM_HAS_COLORS
# `0` is what the resolving callers start from, so it must not read as truthy
NVM_HAS_COLORS=0
export NVM_HAS_COLORS
WRAPPED="$(nvm_wrap_with_color_code 'b' 'b')"
case "${WRAPPED}" in
*"${ESC}"*) die "NVM_HAS_COLORS=0 must not enable colors, got >${WRAPPED}<" ;;
esac
unset NVM_HAS_COLORS
UNSUPPORTED="$(nvm_print_color_legend)"
case "${UNSUPPORTED}" in
*"${ESC}"*) die "the legend emitted colors while unsupported: ${UNSUPPORTED}" ;;
esac
# Model the real asymmetry: yes to the first, direct call, no to every later
# one. The state has to live in a file, because each `$( )` is a fresh subshell
# that would take its own copy of an in-shell counter and answer yes every time.
ONESHOT="$(command mktemp)"
oneshot_reset () { nvm_echo 0 > "${ONESHOT}"; }
nvm_has_colors () {
if [ "$(command cat "${ONESHOT}")" = 0 ]; then
nvm_echo 1 > "${ONESHOT}"
return 0
fi
return 1
}
oneshot_reset
COLORED_OUTPUT="$(nvm_print_color_legend)"
# `grep -c` drains its input, so it cannot SIGPIPE its producer
COLORED="$(nvm_echo "${COLORED_OUTPUT}" | nvm_grep -c "${ESC}" || true)"
[ "${COLORED}" = 9 ] \
|| die "expected all 9 legend lines colored, got ${COLORED}: ${COLORED_OUTPUT}"
# the sample specifically, which is the case #3896 reports
case "${COLORED_OUTPUT}" in
*"${ESC}[0;34mb${ESC}[0m${ESC}[0;33my${ESC}[0m"*) : ;;
*) die "the initial-colors sample was not colored: ${COLORED_OUTPUT}" ;;
esac
# in a substitution, so `nvm_set_colors`' `export NVM_COLORS` stays contained
oneshot_reset
SET_COLORS_OUTPUT="$(nvm_set_colors 'bygre')"
case "${SET_COLORS_OUTPUT}" in
*"${ESC}"*) : ;;
*) die "nvm_set_colors did not color its confirmation line: ${SET_COLORS_OUTPUT}" ;;
esac
# coloring must change only the escapes, never the text or the alignment
COLORED_STRIPPED="$(nvm_echo "${COLORED_OUTPUT}" | strip_colors)"
[ "${COLORED_STRIPPED}" = "${PLAIN}" ] \
|| die 'the colored legend does not match the plain legend once escapes are stripped'
# called directly rather than in a substitution, so a leak is observable at all
oneshot_reset
nvm_print_color_legend >/dev/null
[ -z "${NVM_HAS_COLORS-}" ] \
|| die "nvm_print_color_legend leaked NVM_HAS_COLORS=${NVM_HAS_COLORS}"
# And `nvm unload` has to clean it up, the way it does its sibling helpers.
# `set +e` inside the subshell so a non-zero `nvm unload` cannot abort it before
# the lookup runs; its status is reported separately, so "unload failed" can
@@ -97,4 +188,10 @@ case "${UNLOAD_OUT}" in
*) die "nvm unload did not remove nvm_print_color_legend (${UNLOAD_OUT})" ;;
esac
if [ -n "${NVM_HAS_COLORS_WAS}" ]; then
NVM_HAS_COLORS="${NVM_HAS_COLORS_WAS}"
export NVM_HAS_COLORS
fi
cleanup
echo "nvm_print_color_legend: passed"