[Fix] do not colorize output in a pipe, unless --colors is passed

Fixes 2497
nvm-sh · Jul 29, 2024 · f4994c4 · f4994c4
1 parent ff1257e
commit f4994c4
Show file tree

Hide file tree

Showing 9 changed files with 120 additions and 19 deletions.
diff --git a/nvm.sh b/nvm.sh
@@ -2994,14 +2994,29 @@ nvm() {
  case $i in
  --) break ;;
  '-h'|'help'|'--help')
- NVM_NO_COLORS=""
- for j in "$@"; do
- if [ "${j}" = '--no-colors' ]; then
- NVM_NO_COLORS="${j}"
- break
- fi
+ local NVM_NO_COLORS
+ NVM_NO_COLORS=''
+ local NVM_YES_COLORS
+ NVM_YES_COLORS=0
+
+ while [ $# -ne 0 ]; do
+ case "${1}" in
+ --colors) NVM_YES_COLORS=1 ;;
+ --no-colors) NVM_NO_COLORS='--no-colors' ;;
+ --) ;;
+ esac
+ shift
  done
 
+ if [ $NVM_YES_COLORS -eq 1 ]; then
+ if [ "${NVM_NO_COLORS-}" = '--no-colors' ]; then
+ nvm_err '--colors and --no-colors are mutually exclusive'
+ return 55
+ fi
+ elif ! nvm_stdout_is_terminal; then
+ NVM_NO_COLORS='--no-colors'
+ fi
+
  local NVM_IOJS_PREFIX
  NVM_IOJS_PREFIX="$(nvm_iojs_prefix)"
  local NVM_NODE_PREFIX
@@ -3598,6 +3613,7 @@ nvm() {
 
  return $EXIT_CODE
  ;;
+
  "uninstall")
  if [ $# -ne 1 ]; then
  >&2 nvm --help
@@ -3678,6 +3694,7 @@ nvm() {
  nvm unalias "$(command basename "${ALIAS}")"
  done
  ;;
+
  "deactivate")
  local NVM_SILENT
  while [ $# -ne 0 ]; do
@@ -3727,6 +3744,7 @@ nvm() {
  unset NVM_BIN
  unset NVM_INC
  ;;
+
  "use")
  local PROVIDED_VERSION
  local NVM_SILENT
@@ -3864,6 +3882,7 @@ nvm() {
  nvm_echo "${NVM_USE_OUTPUT}"
  fi
  ;;
+
  "run")
  local provided_version
  local has_checked_nvmrc
@@ -3948,6 +3967,7 @@ nvm() {
  EXIT_CODE="$?"
  return $EXIT_CODE
  ;;
+
  "exec")
  local NVM_SILENT
  local NVM_LTS
@@ -4008,14 +4028,18 @@ nvm() {
  fi
  NODE_VERSION="${VERSION}" "${NVM_DIR}/nvm-exec" "$@"
  ;;
+
  "ls" | "list")
  local PATTERN
  local NVM_NO_COLORS
+ local NVM_YES_COLORS
+ NVM_YES_COLORS=0
  local NVM_NO_ALIAS
 
  while [ $# -gt 0 ]; do
  case "${1}" in
  --) ;;
+ --colors) NVM_YES_COLORS=1 ;;
  --no-colors) NVM_NO_COLORS="${1}" ;;
  --no-alias) NVM_NO_ALIAS="${1}" ;;
  --*)
@@ -4028,6 +4052,16 @@ nvm() {
  esac
  shift
  done
+
+ if [ $NVM_YES_COLORS -eq 1 ]; then
+ if [ "${NVM_NO_COLORS-}" = '--no-colors' ]; then
+ nvm_err '--colors and --no-colors are mutually exclusive'
+ return 55
+ fi
+ elif ! nvm_stdout_is_terminal; then
+ NVM_NO_COLORS='--no-colors'
+ fi
+
  if [ -n "${PATTERN-}" ] && [ -n "${NVM_NO_ALIAS-}" ]; then
  nvm_err '`--no-alias` is not supported when a pattern is provided.'
  return 55
@@ -4046,10 +4080,13 @@ nvm() {
  fi
  return $NVM_LS_EXIT_CODE
  ;;
+
  "ls-remote" | "list-remote")
  local NVM_LTS
  local PATTERN
  local NVM_NO_COLORS
+ local NVM_YES_COLORS
+ NVM_YES_COLORS=0
 
  while [ $# -gt 0 ]; do
  case "${1-}" in
@@ -4060,6 +4097,7 @@ nvm() {
  --lts=*)
  NVM_LTS="${1##--lts=}"
  ;;
+ --colors) NVM_YES_COLORS=1 ;;
  --no-colors) NVM_NO_COLORS="${1}" ;;
  --*)
  nvm_err "Unsupported option \"${1}\"."
@@ -4086,6 +4124,15 @@ nvm() {
  shift
  done
 
+ if [ $NVM_YES_COLORS -eq 1 ]; then
+ if [ "${NVM_NO_COLORS-}" = '--no-colors' ]; then
+ nvm_err '--colors and --no-colors are mutually exclusive'
+ return 55
+ fi
+ elif ! nvm_stdout_is_terminal; then
+ NVM_NO_COLORS='--no-colors'
+ fi
+
  local NVM_OUTPUT
  local EXIT_CODE
  NVM_OUTPUT="$(NVM_LTS="${NVM_LTS-}" nvm_remote_versions "${PATTERN}" &&:)"
@@ -4166,12 +4213,15 @@ nvm() {
  local ALIAS
  local TARGET
  local NVM_NO_COLORS
+ local NVM_YES_COLORS
+ NVM_YES_COLORS=0
  ALIAS='--'
  TARGET='--'
 
  while [ $# -gt 0 ]; do
  case "${1-}" in
  --) ;;
+ --colors) NVM_YES_COLORS=1 ;;
  --no-colors) NVM_NO_COLORS="${1}" ;;
  --*)
  nvm_err "Unsupported option \"${1}\"."
@@ -4188,6 +4238,15 @@ nvm() {
  shift
  done
 
+ if [ $NVM_YES_COLORS -eq 1 ]; then
+ if [ "${NVM_NO_COLORS-}" = '--no-colors' ]; then
+ nvm_err '--colors and --no-colors are mutually exclusive'
+ return 55
+ fi
+ elif ! nvm_stdout_is_terminal; then
+ NVM_NO_COLORS='--no-colors'
+ fi
+
  if [ -z "${TARGET}" ]; then
  # for some reason the empty string was explicitly passed as the target
  # so, unalias it.
@@ -4216,6 +4275,7 @@ nvm() {
  nvm_list_aliases "${ALIAS-}"
  fi
  ;;
+
  "unalias")
  local NVM_ALIAS_DIR
  NVM_ALIAS_DIR="$(nvm_alias_path)"

diff --git a/test/common.sh b/test/common.sh
@@ -245,3 +245,8 @@ _json_parse() {
 nvm_json_extract() {
  nvm_json_tokenize | _json_parse | grep -e "${1}" | awk '{print $2 $3}'
 }
+
+# ensures that commands are run as if in a terminal
+run_in_terminal() {
+ script -q -a /dev/null sh -c "\. ../nvm.sh ; $*"
+}
diff --git a/test/fast/Aliases/Running 'nvm alias ˂aliasname˃ ˂target˃' again should change the target b/test/fast/Aliases/Running 'nvm alias ˂aliasname˃ ˂target˃' again should change the target
@@ -11,14 +11,14 @@ if [ -n "$ZSH_VERSION" ]; then
  setopt noclobber
 fi
 
-nvm alias test-stable-1 0.0.2 || die '`nvm alias test-stable-1 0.0.2` failed'
+nvm alias test-stable-1 0.0.2 || die "\`nvm alias test-stable-1 0.0.2\` failed"
 
-OUTPUT="$(nvm alias test-stable-1 | strip_colors)"
+OUTPUT="$(run_in_terminal 'nvm alias test-stable-1' | strip_colors)"
 EXPECTED_OUTPUT='test-stable-1 -> 0.0.2 (-> v0.0.2)'
 echo "$OUTPUT" | \grep -F "$EXPECTED_OUTPUT" || die "nvm alias test-stable-1 0.0.2 did not set test-stable-1 to 0.0.2: got '$OUTPUT'"
 
-nvm alias test-stable-1 0.0.1 || die '`nvm alias test-stable-1 0.0.1` failed'
+nvm alias test-stable-1 0.0.1 || die "\`nvm alias test-stable-1 0.0.1\` failed"
 
-OUTPUT="$(nvm alias test-stable-1 | strip_colors)"
+OUTPUT="$(run_in_terminal 'nvm alias test-stable-1' | strip_colors)"
 EXPECTED_OUTPUT='test-stable-1 -> 0.0.1 (-> v0.0.1)'
 echo "$OUTPUT" | \grep -F "$EXPECTED_OUTPUT" || die "nvm alias test-stable-1 0.0.1 did not set test-stable-1 to 0.0.1: got '$OUTPUT'"
diff --git a/...Aliases/Running 'nvm alias' lists manual aliases instead of implicit aliases when present b/...Aliases/Running 'nvm alias' lists manual aliases instead of implicit aliases when present
@@ -32,7 +32,7 @@ nvm alias unstable "$EXPECTED_STABLE"
 nvm alias node stable
 nvm alias iojs unstable
 
-NVM_ALIAS_OUTPUT=$(nvm alias | strip_colors)
+NVM_ALIAS_OUTPUT=$(run_in_terminal 'nvm alias' | strip_colors)
 
 echo "$NVM_ALIAS_OUTPUT" | command grep -F "stable -> $EXPECTED_UNSTABLE (-> $UNSTABLE_VERSION)" \
  || die "nvm alias did not contain the overridden 'stable' alias; got '$NVM_ALIAS_OUTPUT'"

diff --git a/test/fast/Aliases/Running 'nvm alias' should list all aliases b/test/fast/Aliases/Running 'nvm alias' should list all aliases
@@ -5,7 +5,7 @@
 
 die () { echo "$@" ; exit 1; }
 
-NVM_ALIAS_OUTPUT="$(nvm alias | strip_colors)"
+NVM_ALIAS_OUTPUT="$(run_in_terminal 'nvm alias' | strip_colors)"
 echo "$NVM_ALIAS_OUTPUT" | \grep -F 'test-stable-1 -> 0.0.1 (-> v0.0.1)' \
  || die "did not find test-stable-1 alias; got '$NVM_ALIAS_OUTPUT'"
 echo "$NVM_ALIAS_OUTPUT" | \grep -F 'test-stable-2 -> 0.0.2 (-> v0.0.2)' \

diff --git a/test/fast/Aliases/nvm_ensure_default_set b/test/fast/Aliases/nvm_ensure_default_set
@@ -12,9 +12,9 @@ nvm_ensure_default_set 0.3 || die "'nvm_ensure_default_set' with an existing def
 
 nvm unalias default || die "'nvm unalias default' failed"
 
-OUTPUT="$(nvm_ensure_default_set 0.2)"
+OUTPUT="$(run_in_terminal 'nvm_ensure_default_set 0.2')"
 EXPECTED_OUTPUT="Creating default alias: default -> 0.2 (-> iojs-v0.2.10)"
 EXIT_CODE="$?"
 
-[ "_$(echo "$OUTPUT" | strip_colors)" = "_$EXPECTED_OUTPUT" ] || die "'nvm_ensure_default_set 0.2' did not output '$EXPECTED_OUTPUT', got '$OUTPUT'"
-[ "_$EXIT_CODE" = "_0" ] || die "'nvm_ensure_default_set 0.2' did not exit with 0, got $EXIT_CODE"
+[ "_$(echo "${OUTPUT}" | strip_colors)" = "_${EXPECTED_OUTPUT}" ] || die "'nvm_ensure_default_set 0.2' did not output '${EXPECTED_OUTPUT}', got '${OUTPUT}'"
+[ "_${EXIT_CODE}" = "_0" ] || die "'nvm_ensure_default_set 0.2' did not exit with 0, got ${EXIT_CODE}"
diff --git a/test/fast/Set Colors/Using --colors and --no-colors errors b/test/fast/Set Colors/Using --colors and --no-colors errors
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+die () { echo "$@" ; cleanup ; exit 1; }
+
+\. ../../../nvm.sh
+
+set -ex
+
+nvm ls --colors --no-colors && die "\`nvm ls --colors --no-colors\` did not fail"
+nvm ls-remote --colors --no-colors && die "\`nvm ls-remote --colors --no-colors\` did not fail"
+nvm alias --colors --no-colors && die "\`nvm alias --colors --no-colors\` did not fail"
+nvm --help --colors --no-colors && die "\`nvm --help --colors --no-colors\` did not fail"
diff --git a/test/fast/Set Colors/`--no-colors` is implied in a pipe b/test/fast/Set Colors/`--no-colors` is implied in a pipe
@@ -0,0 +1,22 @@
+#!/bin/sh
+
+die () { echo "$@" ; cleanup ; exit 1; }
+
+\. ../../../nvm.sh
+\. ../../common.sh
+
+
+set -ex
+
+for cmd in ls ls-remote alias --help; do
+ NO_COLORS="$(run_in_terminal nvm "${cmd}" --no-colors)"
+ COLORS="$(run_in_terminal nvm "${cmd}" --colors)"
+
+ [ "${COLORS}" != "${NO_COLORS}" ] || die "Expected \`nvm ${cmd} --colors\` to be different from \`nvm ${cmd} --no-colors\`"
+
+ PIPE="$(nvm "${cmd}")"
+ PIPE_COLORS="$(nvm "${cmd}" --colors)"
+
+ [ "${PIPE}" = "${NO_COLORS}" ] || die "Expected \`nvm ${cmd}\` in a pipe to be the same as \`nvm ${cmd} --no-colors\`"
+ [ "${PIPE_COLORS}" = "${COLORS}" ] || die "Expected \`nvm ${cmd} --colors\` in a pipe to be the same as \`nvm ls\` in a terminal (with colors)"
+done
diff --git a/test/fast/Unit tests/nvm ls-remote b/test/fast/Unit tests/nvm ls-remote
@@ -6,15 +6,16 @@ die () { echo "$@" ; cleanup ; exit 1; }
 
 cleanup() {
  unset -f nvm_download nvm_ls_remote nvm_ls_remote_iojs
- if [ -n TEMP_NVM_COLORS ]; then
- export NVM_COLORS=TEMP_NVM_COLORS
+ if [ -n "${TEMP_NVM_COLORS}" ]; then
+ export NVM_COLORS="${TEMP_NVM_COLORS}"
  fi
  unset TEMP_NVM_COLORS
 }
 
 \. ../../../nvm.sh
-if [ -n ${NVM_COLORS} ]; then
- export TEMP_NVM_COLORS=NVM_COLORS
+
+if [ -n "${NVM_COLORS}" ]; then
+ export TEMP_NVM_COLORS="${NVM_COLORS}"
  unset NVM_COLORS
 fi
 
@@ -24,6 +25,7 @@ nvm deactivate 2>/dev/null || die 'unable to deactivate'
 
 MOCKS_DIR="$PWD/mocks"
 
+# shellcheck disable=SC2317
 nvm_download() {
  if [ "$*" = "-L -s $(nvm_get_mirror node std)/index.tab -o -" ]; then
  cat "$MOCKS_DIR/nodejs.org-dist-index.tab"