Skip to content
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

Require bash for rgbenv #18

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Require bash for rgbenv #18

wants to merge 3 commits into from

Conversation

ZoomTen
Copy link
Collaborator

@ZoomTen ZoomTen commented Jun 25, 2024

Enforces bash to run rgbenv. Under sh, it first checks for the existence of bash on the device, and then re-executes itself under bash if it exists.

On slower environments, e.g. Termux, it might cause a slight delay on startup. However it should have trivial effect on the "usual" (PC, etc.) environments.


Although bash is the most commonly-used shell (especially on Linux), I was concerned about other environments (outside of WIndows with batch files) where that may not be available (see #9).

But I think I want the extra checking facilities that bash provides, and honestly POSIX sh is a bit too limiting. I'm concerned about Mac OS since I'm still unable to test that, but I'd like to hear some thoughts on this.

@ZoomTen
Copy link
Collaborator Author

ZoomTen commented Jun 25, 2024

I suspect the "re-exec" method may have tripped the CI and froze the tests. Seems to work well on my end :(

Will figure out something.

@ISSOtm
Copy link
Member

ISSOtm commented Jun 25, 2024

macOS provides bash always, so that should be fine?

@ZoomTen
Copy link
Collaborator Author

ZoomTen commented Jun 25, 2024

It's an old version though, is it? I probably might have used features of the newer version without even realizing it...

@ISSOtm
Copy link
Member

ISSOtm commented Jun 28, 2024

Hmm, good point. Does ShellCheck have a feature for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants