[Fix] nvm use: do not clobber man's default search path

`man` consults its own configured search path only when MANPATH holds an empty entry;
a list without one replaces the default outright.
`nvm use` produced exactly that,
so activating a version could hide every system man page.

Contribute the empty entry as a trailing one,
so that nvm's directory keeps precedence over the system's,
and only when the list has none already,
so that repeated `nvm use` calls are idempotent and an empty entry the user placed stays where they put it.

The `$(manpath)` snapshot this replaces went stale the moment man's configuration changed,
and cost a subprocess per `nvm use`.
It was also dead: `local MANPATH` scoped the assignment to `nvm()`,
so `export` did not outlive the call and MANPATH went untouched whenever it started out unset.
Only ksh, where `local` is not a builtin, ever ran it.

`nvm deactivate` now unsets MANPATH where nvm's was the only real entry,
rather than leaving a bare `:` behind.

Reported and diagnosed by @al0ksar in #3890, which used a leading empty entry instead.
That one accrues one more colon on every `nvm use`,
which `nvm deactivate` then leaves behind,
and it loses to the system's man pages wherever nvm's bin directory is not first on PATH.

Supersedes #2077.

Refs #3890
Refs #2077

Author: ljharb

README.md

@@ -870,7 +870,7 @@ nvm exposes the following environment variables:
 - `NVM_CD_FLAGS` - used to maintain compatibility with zsh.
 - `NVM_RC_VERSION` - version from .nvmrc file if being used.
 
-Additionally, nvm modifies `PATH`, and, if present, `MANPATH` and `NODE_PATH` when changing versions.
+Additionally, nvm modifies `PATH` when changing versions, along with `MANPATH` wherever a `manpath` command exists, and `NODE_PATH`, if present.
 
 The following environment variables can be set to configure `nvm install`:
 

nvm.sh

@@ -4257,7 +4257,11 @@ nvm() {
             nvm_err "Could not find ${NVM_DIR}/*/share/man in \${MANPATH}"
           fi
         else
-          export MANPATH="${NEWPATH}"
+          # `man` treats both of these as unset anyway, so leave nothing behind
+          case "${NEWPATH}" in
+            '' | ':') unset MANPATH ;;
+            *) export MANPATH="${NEWPATH}" ;;
+          esac
           if [ "${NVM_SILENT:-0}" -ne 1 ]; then
             nvm_echo "${NVM_DIR}/*/share/man removed from \${MANPATH}"
           fi
@@ -4380,12 +4384,13 @@ nvm() {
       # Change current version
       PATH="$(nvm_change_path "${PATH}" "/bin" "${NVM_VERSION_DIR}")"
       if nvm_has manpath; then
-        if [ -z "${MANPATH-}" ]; then
-          local MANPATH
-          MANPATH=$(manpath)
-        fi
-        # Change current version
-        MANPATH="$(nvm_change_path "${MANPATH}" "/share/man" "${NVM_VERSION_DIR}")"
+        MANPATH="$(nvm_change_path "${MANPATH-}" "/share/man" "${NVM_VERSION_DIR}")"
+        # `man` consults its configured default path only where the list has an
+        # empty entry; a trailing one keeps nvm's directory ahead of it
+        case "${MANPATH}" in
+          :* | *::* | *:) ;;
+          *) MANPATH="${MANPATH}:" ;;
+        esac
         export MANPATH
       fi
       export PATH

test/fast/Running 'nvm use' should not clobber the default MANPATH

@@ -0,0 +1,68 @@
+#!/bin/sh
+
+set -e
+
+die () { echo "$@" ; exit 1; }
+
+export NVM_DIR="$(cd ../.. && pwd)"
+
+# `--delete-prefix` covers only the npmrc files, so an inherited prefix in the
+# environment would still abort `nvm use`
+unset PREFIX npm_config_prefix NPM_CONFIG_PREFIX
+
+: nvm.sh
+\. ../../nvm.sh
+\. ../common.sh
+
+# the block under test is gated on `nvm_has manpath`, and the Alpine CI images
+# ship no `manpath`; the output is recognizable so that a reintroduced
+# `$(manpath)` call fails the assertions instead of passing vacuously
+manpath() {
+  echo '/usr/share/man'
+}
+
+make_fake_node v0.10.2
+make_fake_node v0.10.3
+
+MAN_2="$(nvm_version_path v0.10.2)/share/man"
+MAN_3="$(nvm_version_path v0.10.3)/share/man"
+
+# nvm has to contribute the empty entry itself when there is nothing to preserve
+unset MANPATH
+nvm use --delete-prefix v0.10.2 --silent || die "Failed to activate v0.10.2"
+[ "${MANPATH}" = "${MAN_2}:" ] || die "MANPATH should be '${MAN_2}:'; got '${MANPATH}'"
+
+nvm deactivate --silent || die "Failed to deactivate v0.10.2"
+[ -z "${MANPATH-}" ] || die "MANPATH should be unset again; got '${MANPATH}'"
+
+# nvm's entry first, the empty entry last: the reverse of what a leading empty
+# entry produces
+MANPATH='/opt/foo/man'
+nvm use --delete-prefix v0.10.2 --silent || die "Failed to activate v0.10.2"
+[ "${MANPATH}" = "${MAN_2}:/opt/foo/man:" ] || die "MANPATH should be '${MAN_2}:/opt/foo/man:'; got '${MANPATH}'"
+
+# switching versions replaces nvm's entry without accruing more empty ones
+nvm use --delete-prefix v0.10.3 --silent || die "Failed to activate v0.10.3"
+[ "${MANPATH}" = "${MAN_3}:/opt/foo/man:" ] || die "MANPATH should be '${MAN_3}:/opt/foo/man:'; got '${MANPATH}'"
+
+nvm use --delete-prefix v0.10.2 --silent || die "Failed to reactivate v0.10.2"
+[ "${MANPATH}" = "${MAN_2}:/opt/foo/man:" ] || die "MANPATH should be '${MAN_2}:/opt/foo/man:'; got '${MANPATH}'"
+
+nvm deactivate --silent || die "Failed to deactivate v0.10.2"
+[ "${MANPATH}" = '/opt/foo/man:' ] || die "MANPATH should be '/opt/foo/man:'; got '${MANPATH}'"
+
+# a value with no empty entry at all still deactivates to nothing
+MANPATH="${MAN_2}"
+nvm deactivate --silent || die "Failed to deactivate v0.10.2"
+[ -z "${MANPATH-}" ] || die "MANPATH should be unset again; got '${MANPATH}'"
+
+# an empty entry the user placed themselves stays where they put it
+for TEST_MANPATH in ':/opt/foo/man' '/opt/foo/man:' '/a/man::/b/man'; do
+  MANPATH="${TEST_MANPATH}"
+
+  nvm use --delete-prefix v0.10.2 --silent || die "Failed to activate v0.10.2 with MANPATH '${TEST_MANPATH}'"
+  [ "${MANPATH}" = "${MAN_2}:${TEST_MANPATH}" ] || die "MANPATH should be '${MAN_2}:${TEST_MANPATH}'; got '${MANPATH}'"
+
+  nvm deactivate --silent || die "Failed to deactivate v0.10.2 with MANPATH '${TEST_MANPATH}'"
+  [ "${MANPATH}" = "${TEST_MANPATH}" ] || die "MANPATH should be '${TEST_MANPATH}'; got '${MANPATH}'"
+done

test/fast/Unit tests/nvm_change_path

@@ -44,6 +44,20 @@ NEW_PATH=`nvm_change_path "$EMPTY_PATH" "/bin" "$NVM_DIR/v0.1.2"`
 [ "$NEW_PATH" = "$NVM_DIR/v0.1.2/bin" ] || die "Not correctly prepended: $NEW_PATH "
 
 
+# empty entries select `man`'s own search path, so they have to survive intact
+MAN_PATH_WITH_EMPTY_ENTRIES=/opt/foo/man::/opt/bar/man:
+
+# New version dir
+NEW_PATH=`nvm_change_path "$MAN_PATH_WITH_EMPTY_ENTRIES" "/share/man" "$NVM_DIR/versions/node/v7.1.0"`
+
+[ "$NEW_PATH" = "$NVM_DIR/versions/node/v7.1.0/share/man:$MAN_PATH_WITH_EMPTY_ENTRIES" ] || die "Not correctly prepended: $NEW_PATH "
+
+# Replacing an existing version dir
+NEW_PATH=`nvm_change_path "$NVM_DIR/versions/node/v4.5.0/share/man::/opt/bar/man:" "/share/man" "$NVM_DIR/versions/node/v7.1.0"`
+
+[ "$NEW_PATH" = "$NVM_DIR/versions/node/v7.1.0/share/man::/opt/bar/man:" ] || die "Not correctly changed: $NEW_PATH "
+
+
 # https://github.com/nvm-sh/nvm/issues/1652#issuecomment-342571223
 MAC_OS_NESTED_SESSION_PATH=/usr/bin:/usr/local/bin:$NVM_DIR/versions/node/v4.5.0/bin