fix: make installer shell detection prefer the reported shell#12942
fix: make installer shell detection prefer the reported shell#12942
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Byron
force-pushed
the
fix-shell-detection
branch
2 times, most recently
from
2c17d4c to
66931a1
Compare
Byron
force-pushed
the
fix-shell-detection
branch
2 times, most recently
from
0aebbf6 to
b7e42d5
Compare
…-based detection otherwise. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Byron
force-pushed
the
fix-shell-detection
branch
from
b7e42d5 to
8e3bac2
Compare
There was a problem hiding this comment.
Pull request overview
Updates the but-installer shell detection so it prefers the shell reported by $SHELL over config-file heuristics, preventing an existing fish config from incorrectly overriding zsh/bash during installation.
Changes:
- Add
$SHELL-based shell detection and use it as the primary preference when selecting which shell config to update. - Create POSIX shell config files when missing before appending PATH/completions.
- Add regression tests for preference logic and missing-config creation (introducing
instaas a dev dependency for inline snapshots).
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/but-installer/src/shell.rs | Prefer $SHELL when selecting shell config; create missing POSIX config files; add regression tests. |
| crates/but-installer/Cargo.toml | Add insta as a dev-dependency for new snapshot-based tests. |
| Cargo.lock | Lockfile update to include insta for but-installer dev builds/tests. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e3bac24e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Prefer ~/.bash_profile when the reported shell is Bash on macOS so PATH and completion setup survives terminal restart. If ~/.bashrc exists but ~/.bash_profile does not, create a minimal ~/.bash_profile that sources ~/.bashrc before appending installer-managed entries. Also add macOS regression tests covering fresh Bash configs and the existing-.bashrc case. Co-authored-by: Sebastian Thiel <sebastian.thiel@icloud.com>
Tasks
Question for the reviewer
Is there a good reason to not use
$SHELL? For all I know, it's only set by the login shell and future sub-shells inherit it.🧢 Changes
$SHELLbefore falling back to config-file heuristics☕️ Reasoning
The installer was picking fish whenever
~/.config/fish/config.fishexisted, even for users running the install from a zsh environment viacurl ... | sh. Using the reported shell first matches the user's configured terminal shell and fixes the incorrect completion instructions from #12931.🎫 Affected issues
Closes #12931
✅ Verification
cargo test -p but-installer