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

feat: add fish support to shell installers #958

Merged
merged 1 commit into from
Apr 24, 2024
Merged

feat: add fish support to shell installers #958

merged 1 commit into from
Apr 24, 2024

Conversation

mistydemeo
Copy link
Contributor

This adds support for fish as a shell. Like with sh, I took inspiration from rustup's implementation. This broadly leans on the existing shell support, but has a few distinct features.

  • Instead of writing to an existing configuration file like ~/.bashrc, we write to a file in fish's conf.d path. This is a directory which is autoloaded on shell boot, which means we can create an app-specific path and can avoid having to edit the user's own configuration file.
  • Unique to fish, since conf.d may not exist at the time we want to interact with it, I've added an mkdir -p call.
  • I've used fish's fish_add_path helper, which encapsulates the logic of "only prepend to the path if it's not already on the PATH" without us having to write it ourselves. (Note that fish_add_path was introduced in fish 3.2.0, which was released in 2020. Since that version is available in the current LTS release of Ubuntu and Debian stable, I believe it's old enough to be safe for us to use it.)
  • I updated our existing shell writing code to make it aware of what shell we're writing so I could branch which form of envfile to write, and whether to output source or . statements. (fish strongly prefers source and has stated . may be removed in the future.)

I gave this a test locally; it appears to be working fine, and, like with the sh side, doesn't overwrite cargo's configuration for CargoHome installs.

@mistydemeo mistydemeo requested a review from Gankra April 23, 2024 19:56
Copy link
Contributor

@Gankra Gankra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good, except for one weird thing.

@mistydemeo mistydemeo force-pushed the fish-support branch 2 times, most recently from d49b20c to da50fd4 Compare April 24, 2024 17:41
@mistydemeo mistydemeo merged commit 921b248 into main Apr 24, 2024
15 checks passed
@mistydemeo mistydemeo deleted the fish-support branch April 24, 2024 18:03
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.

2 participants