shellenv: use PATH variable, fish_user_paths should remain universal

[charego]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

There are two ways to manipulate PATH in fish shell.

• set the PATH variable directly
• set the fish_user_paths variable, whose contents are automatically prepended to PATH

fish_user_paths is supposed to be a universal variable.

> set --show fish_user_paths
$fish_user_paths: not set in local scope
$fish_user_paths: not set in global scope
$fish_user_paths: set in universal scope, unexported, with 1 elements
$fish_user_paths[1]: length=20 value=|/home/charego/fzf/bin|

When you evaluate the output of shellenv, fish_user_paths is set as a global variable which shadows the universal variable. This is technically legal, but it goes against best practice and causes issues when the user wants to update fish_user_paths in the future.

> eval (/home/linuxbrew/.linuxbrew/bin/brew shellenv)

> set --show fish_user_paths
$fish_user_paths: not set in local scope
$fish_user_paths: set in global scope, unexported, with 3 elements
$fish_user_paths[1]: length=30 value=|/home/linuxbrew/.linuxbrew/bin|
$fish_user_paths[2]: length=31 value=|/home/linuxbrew/.linuxbrew/sbin|
$fish_user_paths[3]: length=20 value=|/home/charego/fzf/bin|
$fish_user_paths: set in universal scope, unexported, with 1 elements
$fish_user_paths[1]: length=20 value=|/home/charego/fzf/bin|

> echo $fish_user_paths
/home/linuxbrew/.linuxbrew/bin /home/linuxbrew/.linuxbrew/sbin /home/charego/fzf/bin

> set -U fish_user_paths ~/bin $fish_user_paths
set: Universal variable 'fish_user_paths' is shadowed by the global variable of the same name.

> set --show fish_user_paths
$fish_user_paths: not set in local scope
$fish_user_paths: set in global scope, unexported, with 3 elements
$fish_user_paths[1]: length=30 value=|/home/linuxbrew/.linuxbrew/bin|
$fish_user_paths[2]: length=31 value=|/home/linuxbrew/.linuxbrew/sbin|
$fish_user_paths[3]: length=20 value=|/home/charego/fzf/bin|
$fish_user_paths: set in universal scope, unexported, with 4 elements
$fish_user_paths[1]: length=16 value=|/home/charego/bin|
$fish_user_paths[2]: length=30 value=|/home/linuxbrew/.linuxbrew/bin|
$fish_user_paths[3]: length=31 value=|/home/linuxbrew/.linuxbrew/sbin|
$fish_user_paths[4]: length=20 value=|/home/charego/fzf/bin|

> echo $fish_user_paths
/home/linuxbrew/.linuxbrew/bin /home/linuxbrew/.linuxbrew/sbin /home/charego/fzf/bin

For this reason it makes sense that shellenv should output instructions to update the PATH variable directly instead of fiddling with fish_user_paths.

Reference: fish shell tutorial, $PATH section

[MikeMcQuaid]

Nice work! One suggested tweak. Thanks!

[MikeMcQuaid]

Can we kill the contains? We don't do that for anything else and it seems overkill.

[faho]

It is not overkill, it should be done for the rest as well.

Fish loads its config always, whether login or not, interactive or not. That means this could easily result in growing $*PATH.

[charego]

Hi @MikeMcQuaid, I didn't want to modify the shellenv output too much and was planning to not use contains, but discussion on the fish gitter led me to include it. As @faho explained the PATH variables will continue to grow if you open fish within fish, for example. Thanks for the review.

[MikeMcQuaid]

As @faho explained the PATH variables will continue to grow if you open fish within fish, for example.

This also applies to Bash and ZSH and no-one has complained about this until this PR.

Can we scope this just to PATH for now, please.

[charego]

Done. Scope is limited to PATH variable for fish shell only.

[MikeMcQuaid]

Thanks so much for your first contribution! Without people like you submitting PRs we couldn't run this project. You rock, @charego!