feat: Add Fish shell support in installer#398
Merged
svarlamov merged 2 commits intogit-ai-project:mainfrom Jan 26, 2026
Merged
Conversation
Member
|
Thank you for your contribution @Krishnachaitanyakc! I just realized that we only install in one shell, but for example, I now have both zsh and fish installed on my laptop. Do you think there's a sane way to make sure that we end up in the PATH on all the user's shells? Curious what your thoughts are on this |
Contributor
Author
|
@svarlamov One option I can think of is, instead of detecting which shell is primary and modifying only that one, we can detect all shells that have config files present and modify each of them. What do you think? |
Member
|
@Krishnachaitanyakc I think that's a great idea -- can you update the PR? Excited to get this merged :) |
Previously the installer only configured PATH for a single detected shell. Now it detects and configures all shells that have existing config files: - Bash: ~/.bashrc (preferred) or ~/.bash_profile - Zsh: ~/.zshrc - Fish: ~/.config/fish/config.fish Key changes: - Replace detect_shell() with detect_all_shells() that returns all shells - Loop through all detected shells and configure PATH in each - Use shell-appropriate PATH syntax (fish_add_path for Fish, export for others) - Improved user feedback showing which configs were updated - Provide reload instructions for each configured shell - Falls back to $SHELL detection only when no config files exist - Idempotent: won't duplicate PATH entries on re-run
Add comprehensive tests for the detect_all_shells() function: - Detects individual shells (bash, zsh, fish) when only one config exists - Detects multiple shells when multiple configs exist - Prefers .bashrc over .bash_profile for bash - Falls back to $SHELL when no config files exist - Falls back to bash as default when $SHELL is unknown/empty Also tests PATH configuration behavior: - Idempotency: doesn't duplicate PATH entries on re-run - Correct syntax for fish (fish_add_path) vs bash/zsh (export PATH) - Integration: configures all detected shells - Only modifies existing config files, doesn't create new ones
2efca34 to
eda1726
Compare
Contributor
Author
|
@svarlamov done, can you review it? |
Member
|
Just reviewed and tested a bunch -- looks great! I'll merge as soon as the CI finishes |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #386