Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prepend asdf directories to the PATH in Nushell #1496

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

etoal83
Copy link
Contributor

@etoal83 etoal83 commented Mar 8, 2023

Summary

In the Nushell setup script, I change the paths to asdf shims and bin to be prepended in front of the existing $PATH instead of being appended at the end. This change makes asdf plugins have priority over the existing commands in the search path as well as other shells' setup.

@etoal83 etoal83 requested a review from a team as a code owner March 8, 2023 15:05
@jthegedus
Copy link
Contributor

All our Shell integrations use append due to significant user feedback when changing this to prepend in the past. I think we should stick with append.

What is the specific use case troubling you that requires this?

@Stratus3D
Copy link
Member

@jthegedus it appears we prepend to the asdf paths to $PATH in asdf.sh. It appears the changes here from @etoal83 make Nushell consistent with Bash/Zsh in this regard. I am in favor of this change if I'm understanding things correctly.

To me it also seems easier to reason about a script that adds something to the beginning of the $PATH. When you source it in your .zshrc/.bashrc you be certain that asdf is added at the beginning of $PATH, anything that comes before it in the rc file ends up after asdf on PATH, and anything after it in the rc file comes before it on the PATH.

@etoal83
Copy link
Contributor Author

etoal83 commented Mar 11, 2023

@jthegedus @Stratus3D

Thank you for commenting! What I intended with this PR is correctly explained by @Stratus3D .

What is the specific use case troubling you that requires this?

Appending the path becomes a problem when the same command already exists in the user environment. Because the current asdf.nu appends the $shims_dir and $asdf_bin_dir paths to the existing $PATH, a preinstalled version of executable was used in the command over the asdf version. I think in most situations people installing asdf would prefer to use the asdf version of the executable rather than the existing one.

This PR changes the asdf path setting from/to:

  • before: $PATH:$shims_dir:$asdf_bin_dir
  • after: $asdf_bin_dir:$shims_dir:$PATH

Sorry if the use of my word "append" and "prepend" were misleading😣

@jthegedus
Copy link
Contributor

Sorry for the confusion, I misunderstood the current state. I should have validated 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants