Improve fish setup

[Kaylebor]

Is your feature request related to a problem? Please describe

The current way that the asdf.fish script works is not very "fish-like":

• It is not necessary to loop over $PATH to make sure the .asdf folders are not duplicated; we can just use the built-in fish_add_path function for it.
• It is not necessary to always check the path on every single run, since fish already persists the path in the file .config/fish/fish_variables when running fish_add_path.
  • Admittedly, future changes made by the user may break asdf, but rerunning fish_add_path is a safe way to make sure the path is in the right order.
• Loading the asdf wrapper function can be done by symlinking to .config/fish/functions instead, as a one-time setup step.

I recently ran into a problem where Ruby was using the system-provided bundle gem, instead of asdf; reshim did not help, and restarting the shell did not help either, as the current wrapper was not correctly prepending the folders to PATH.

Describe the proposed solution

We can replace the current script with a setup file, run only once, that modifies the path and symlinks the files.

Roughly speaking, we only need 3 lines, plus a few extra to define the right variables:

fish_add_path --move $ASDF_DIR/bin
fish_add_path --move $ASDF_DATA_DIR/shims
ln -s $ASDF_DIR/lib/asdf.fish ~/.config/fish/functions/asdf.fish

Running this once is enough, it will be persisted without modifying .config/fish/config.fish.
Future issues related to this can safely be fixed by rerunning this code.

We can replace the current wrapper with an asdf_setup.fish file, keeping the first half where we get the right variables, adding the lines above, and change fish instructions to tell users to just run the file once.

This will also keep the user's .config/fish/config.fish file clean.

Describe similar asdf features and why they are not sufficient

When using the existing fish script, the shims path was somehow put at the end of PATH, breaking my intended Ruby installation until I manually fixed it.
Rerunning the script by restarting the fish changed nothing, a manual fix was needed.

Describe other workarounds you've considered

Modifying the script as said above, without the symlink, and using it as it is now (a simple wrapper) should still work.

Still, after looking at how fish handles configuration and functions, I consider it more appropriate to run this script only once instead, and let fish handle the specifics.

[Kaylebor]

I should have read more: PR 1317

Did you already consider doing it the way I suggest above? I'd like to know reasons against it.

[jthegedus]

@Kaylebor #1317 was merged before your issue was raised, so your suggestions above were not considered. We're happy to accept PRs for small improvements. Since the core team do not use all shells, shell-specific improvements should probably tag the contributors to those Shell code paths from prior PRs, eg, those in #1317

[jthegedus]

@hyperupcall Do your changes from #1524 resolve this issue any?

[hyperupcall]

It does not resolve the issue, but does make some points moot. For example, since fish_add_path is no longer used, the second point about persistence does not apply.

I made the choice not to tackle this issue at the same time as #1524. Although I agree that objectively that closing this issue would make our Fish scripts more idiomatic, I thought it would be better (for me) to make such changes at least after #1351, and I would want to use Fish a bit more before making such a sweeping change (since it changes not just the behavior to arrive at the state, but the persistence of the state). So I guess these changes would be for another time.

[karmicdude]

Please note to this fish-shell/fish-shell#9819 issue. The current asdf behavior with set global scope for fish_add_path breaks the default expected behavior from the shell, where the scope is expected to be universal.

# from asdf.fish
if type -q fish_add_path
  if test -n "$ASDF_DATA_DIR"
    fish_add_path --global --move "$ASDF_DATA_DIR/shims" "$ASDF_DIR/bin"
  else
    fish_add_path --global --move "$HOME/.asdf/shims" "$ASDF_DIR/bin"
  end

And many other utils and scripts based on adding fish_add_path, which has a universal scope by default, stop working because it is overridden by the global scope.