cmd/shellenv: improve detection of shell name. #21374
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request simplifies shell detection in the brew shellenv command by replacing platform-specific detection methods (lsof on macOS, ps elsewhere) with a simpler ${SHELL##*/} approach that extracts the shell name from the $SHELL environment variable. The documentation is updated across all files to recommend explicitly specifying the shell name parameter and to provide clear examples.
- Simplifies shell detection by using
${SHELL##*/}instead oflsof/pscommands - Updates documentation to recommend explicit shell specification with examples
- Adds caveat that automatic detection may not always be correct
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Library/Homebrew/cmd/shellenv.sh | Simplified shell detection logic to use ${SHELL##*/} parameter expansion |
| Library/Homebrew/cmd/shellenv.rb | Updated command documentation with explicit shell examples and detection caveat |
| docs/Manpage.md | Updated man page documentation with explicit shell examples and improved guidance |
| manpages/brew.1 | Updated generated man page with corresponding documentation changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This reverts commit 0aadf99.
This reverts commit 4d96033.
Let's just use the simplest method (that we use in the installer) to detect the shell. Let's just tell users to set the shell name explicitly if this detection is wrong. This is done by many other tools and avoids us fighting with stupid checks that don't work reliably or speedily under sandboxes etc. Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
8d6f0f3 to
0819fa1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
thank you @MikeMcQuaid 🙏🏼 |
|
Whoa, this broke my setup really hard. I use fish and had Might be worth to mention this somewhere very explicitly 😄 Btw: Thanks for your work! ❤️ |
Brew Homebrew/brew#21374 changed how shell is detected. Now fish is not detected in this contexts and `brew shellenv` outputs bash syntax, breaking fish initialization
Brew Homebrew/brew#21374 changed how shell is detected. Now fish is not detected in this contexts and `brew shellenv` outputs bash syntax, breaking fish initialization.
Let's just use the simplest method (that we use in the installer) to detect the shell.
Let's just tell users to set the shell name explicitly if this detection is wrong.
This is done by many other tools and avoids us fighting with stupid checks that don't work reliably or speedily under sandboxes etc.
Fixes #21284