[jihtsan]

Fish activation currently erases the caller's NODE_PATH, NPM_CONFIG_PREFIX and npm_config_prefix before saving them. Save their original values and restore them only when leaving an active nodeenv, preserving the distinction between an empty value and an unset variable.

While active, replace NODE_PATH with the environment's module directory, matching the other shells' isolation behavior. Put the --isolate-npm restoration under the same active-environment guard, preserve empty cache/userconfig/init-module values, and remove the saved variables after restoring them. Repeated activation and nondestructive deactivation retain the original environment.

Behavioral tests now check the entire active NODE_PATH, all six restored variables, backup cleanup, unset/empty values, multiple paths, spaces and repeated operations, with and without --isolate-npm. The shell-requirement comment is current, and CHANGES notes that existing environments need --force to regenerate activate.fish.

Validation on macOS with fish 4.9.3:

• Strengthened regression tests against prior head 22e3d4b: 5 failed, 29 passed. All pass with this correction.
• NODEENV_REQUIRE_SHELLS=1 python -m pytest tests/test_activate_shells.py: 114 passed on each of Python 3.10.21 and 3.13.12, running sh, dash, bash, zsh and fish.
• python -m pytest -m 'not integration' tests: 411 passed / 1 failed / 11 skipped on Python 3.10; 412 passed / 1 failed / 10 skipped on Python 3.13; 5 integration cases deselected in each.
• The one failure is test_mirror_credentials_stay_out_of_error_messages, which receives HTTP 503 in this local environment. It fails with the same HTTP 503 on unchanged prior head 22e3d4b under both Python versions. With only that confirmed baseline case additionally excluded, the remaining 411 / 412 tests pass.
• flake8 --extend-ignore=E127 nodeenv.py tests setup.py and git diff --check: passed.

Older fish releases, other Python versions, Linux/Windows execution and real Node download/build integration are not covered by these local checks. Upstream CI results must be checked against the new head.

Closes #399.

[ekalinin]

Thanks, this fixes #399. Two changes are needed before merge, plus two small ones.

• [P1] Keep fish replacing NODE_PATH - nodeenv.py:2227

  This prepend branch has never run since fish support landed in 2017: the initial deactivate_node nondestructive pass erased NODE_PATH first, so fish replaced it like the other shells. Now it runs, and modules from the user's NODE_PATH resolve inside the env, under fish only. Please replace it, as the prefix variables below do:

  if set -q NODE_PATH
      set -gx _OLD_NODE_PATH $NODE_PATH
  end
  set -gx NODE_PATH "$NODE_VIRTUAL_ENV/__MOD_NAME__"

  Then test_activate_sets_env (tests/test_activate_shells.py:283) can compare the whole NODE_PATH, not just its first element.

• [P2] Same fix for the --isolate-npm restore - nodeenv.py:1768

  It still uses test -n, so an empty npm_config_* becomes unset and leaves an exported empty _OLD_npm_config_* behind. Please move __NPM_UNISOLATE__ inside the new guard, drop its duplicate guard, switch to set -q, and add the three npm_config_* names to names in test_fish_restores_original_variables.

• [P3] The REQUIRE_SHELLS comment (tests/test_activate_shells.py:144) still refers to xfail markers, which this PR removes.

• [P3] Please add an [unreleased] entry for #399 to CHANGES, noting that existing envs need --force to get the new activate.fish.

[ekalinin]

Thanks for the update, all four points from the last round are in. A few more things before merge.

• Please rebase on master: CHANGES conflicts with the #419 entry, and CI hasn't run on 306eb92.

• The new if set -q NODE_VIRTUAL_ENV guard (nodeenv.py:2127) skips the restore when NODE_VIRTUAL_ENV is already gone:

  set -gx NODE_PATH /old/np
  source env/bin/activate.fish
  set -e NODE_VIRTUAL_ENV
  deactivate_node

  After this, NODE_PATH, NPM_CONFIG_PREFIX and npm_config_prefix still point at the env, so npm i -g keeps installing there, and _OLD_NODE_PATH=/old/np stays exported. On master these three are at least erased. activate keeps all of these restores under one guard, _OLD_NODE_VIRTUAL_PATH, and fish can do the same: move them into the existing PATH block and drop the new guard.

      if test -n "$_OLD_NODE_VIRTUAL_PATH"
          set -gx PATH $_OLD_NODE_VIRTUAL_PATH
          set -e _OLD_NODE_VIRTUAL_PATH
  
          # NODE_PATH, NPM_CONFIG_PREFIX and npm_config_prefix restores as now
  __NPM_UNISOLATE__
      end

  The nondestructive pass at the top still skips them unless another env is active, because only activation sets _OLD_NODE_VIRTUAL_PATH. With this change the case above ends with NODE_PATH=/old/np, and the shell tests pass except test_isolate_npm_fish_content (tests/test_install_activate.py:411), which looks for the if set -q NODE_VIRTUAL_ENV string. When you update it, please make its order check fail if __NPM_UNISOLATE__ ends up outside the guard. Right now the guard is in every activate.fish and always comes first, so the check passes wherever the restores are.

• My --force suggestion for CHANGES was wrong, sorry. With -p, nodeenv appends the new code to the virtualenv's activate.fish, so the old block still runs first and erases NODE_PATH. Without -p, --force reinstalls node at the default version and drops --isolate-npm and --prompt unless they are passed again. Something like "existing environments keep the old activate.fish; recreate them with the same options to get the fix (with -p, remove the old nodeenv part from the virtualenv's activate.fish first)" would be closer.

• Nit: assert all(...) at tests/test_activate_shells.py:340 doesn't say which backup leaked. A dict comparison like the line above it would.