-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Don't execute zsh
startup files as part of our default macOS wrapper
#7950
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered any other zsh options? I'm curious if this includes --no-globalrcs
.
I've considered it, but after reading the man page for the options I don't think it will have any effect at the moment for an average macOS user. The only file in It's worth mentioning that @Learath2 I'm curious if you have any input, or could verify this patch. |
This patch is exactly what I'm running locally. I haven't hit any issues with it after running it for a month. As for adding The only reason I didn't contribute this patch myself was that I was wondering whether we should be wrapping the users shell setting from the config the same way we wrap the one set by |
I was wondering this myself, but after a brief discussion in #alacritty, it seems like the stance is that users who run their own I'm personally still not convinced, since I hardly understand the need for the wrapping, I assume most users are also unaware of the need and will therefor do something like But that could be considered a separate issue to this PR either way. |
The point was to run login shell with And I don't think it wraps user specified programs, since users can properly start their shells. |
Most importantly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the only thing that gets sourced now is /etc/zsh/zshenv
and I didn't see an easy solution to fix that. Should be fine as-is, probably isn't going to make things worse at least.
Could you add a changelog entry documenting these changes?
@chrisduerr should be good to go now I think. |
A related issue where this is fixed (but not yet released on Homebrew): alacritty/alacritty#7950
Given a
~/.zshenv
withecho hit
inside, I've tested with bothchsh -s /bin/sh
which does not printhit
anymore, whilechsh -s /bin/zsh
continues to printhit
as it does on master.fixes #7886