Fix spurious SHLVL variable assignment by fenv.

oh-my-fish / plugin-foreign-env

Run foreign bash scripts and capture exported environment variables

MIT License

221 stars 15 forks source link

Fix spurious SHLVL variable assignment by fenv. #19

Closed edlanglois closed 5 years ago

edlanglois commented 5 years ago

Originally, the 2nd env call was made within one additional bash subshell causing the SHLVL variable to differ and therefore be assigned by fenv.

In fish 3.0 this resulted in a warning message because SHLVL is read only.

edlanglois commented 5 years ago

Correction: The SHLVL warning I encountered seems to have been triggered by the update to bash 5.0 rather than the update to fish 3.0. I don't have a fully understanding of the issues but possibly the latest version of bash increments SHLVL for certain subshells when past versions did not.

I don't know if that makes this a bash issue or not, but either way this change solves the problem.

Example invocation before the change in this commit:

> fenv true
set: Tried to change the read-only variable “SHLVL”

edlanglois commented 5 years ago

Done

scorphus commented 5 years ago

LGTM

Dear @oh-my-fish/core, squash and merge at your earliest convenience 😉

dansanduleac commented 4 years ago

I still get the same error running fish installed from nix, specifically when sourcing the nix per-user profile at ~/.nix-profile/etc/profile.d/nix.sh with fenv.

I've added some debug logging in fenv.parse.diff.fish to showcase what changes:

Setting: NIX_SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt
Setting: PATH=/Users/dansanduleac/.nix-profile/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin
Setting: _=/Users/dansanduleac/.nix-profile/bin/env
set: Tried to change the read-only variable “_”