-
-
Notifications
You must be signed in to change notification settings - Fork 20.2k
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
[macOS] Add support for loading shell environment from UI apps. #81266
base: master
Are you sure you want to change the base?
Conversation
5f660f4
to
bd73188
Compare
<method name="load_shell_environment" qualifiers="const"> | ||
<return type="void" /> | ||
<description> | ||
Loads the default shell and copies environment variables set by the shell startup scripts to the app environment. | ||
[b]Note:[/b] This method is implemented on macOS for non-sandboxed applications only. | ||
</description> | ||
</method> |
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.
If it's macOS specific, is there really much need for exposing it? Sounds more like a Godot editor implementation detail than a generic API users would make use of.
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.
It might be useful for custom tools that are running external executables or configurable with environment variables, but still use GUI (and app bundle).
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.
What if we always call this method on startup (not just in the editor)? Is there a downside to always calling it?
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.
The only negative impact is likely start-up time. Also running a shell on every start might look suspicious.
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.
Then, if we don't want to expose the method maybe we can expose a project setting to determine whether to call the method at startup or not.
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.
I guess project setting OK as well, not sure what's better. We have plenty of OS
methods that are useful only on a specific platform, or work completely differently (sandbox
and permission
related stuff, get_cmdline_platform_args
).
Adds
OS::load_shell_environment
method to loadzsh
and copy environment variables to the macOS GUI app environment.OS::load_shell_environment
is automatically used by the editor, and can be used manually from any other non-sandboxed project.See #81240
Fixes #96409
Fixes #96596