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

fix: fix for Termux not having '/dev/stdin' #241

Merged
merged 4 commits into from
Aug 27, 2019
Merged

fix: fix for Termux not having '/dev/stdin' #241

merged 4 commits into from
Aug 27, 2019

Conversation

bbigras
Copy link
Contributor

@bbigras bbigras commented Aug 26, 2019

Description

Use $BASH_VERSINFO to determine if source <(starship init bash --print-full-init) can be used.

This can only work if bash has $BASH_VERSINFO in 3.2.

Motivation and Context

To make eval "$(starship init bash)" work on Termux.

Closes #235

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots (if appropriate):

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@chipbuster
Copy link
Contributor

[ "${{BASH_VERSINFO[0]}}" -ge 4 ] gives me bash: ${{BASH_VERSINFO[0]}}: bad substitution on MacOS :(

I assume there was a reason you elected not to go for single curly-braces?

@bbigras
Copy link
Contributor Author

bbigras commented Aug 26, 2019

Did you copy paste that from the code to a terminal? I had to escape those because I'm using the format macro.

It should not be double at runtime.

@chipbuster
Copy link
Contributor

@bbigras Yes, yes I did.

Sorry, hasn't been the best day for me.

@chipbuster
Copy link
Contributor

Code looks good to me, and works in a bash 3.2 shell on MacOS.

I'd like to get some confirmation that the process substitution syntax does work on Bash 4.0. Have you tested with that version? The SO post in the comments mentions that it's a bash 3.2 limitation, but the post they link to on the bash mailing lists only says that this is a limitation that will be lifted "someday."

@chipbuster
Copy link
Contributor

@bbigras Bad news: on bash-4.0 on my Linux machine, it doesn't work. Displays the same behavior as it does in 3.2, namely it looks like a no-op.

It does work for bash-5.0 on my machine, so I guess we just need to bisect minor versions until we find the first working one. I'll take a crack at it tomorrow--I actually downloaded all the source archives for various bash versions for testing exactly this kind of stuff, but I haven't gotten around to actually building them yet.

@bbigras
Copy link
Contributor Author

bbigras commented Aug 26, 2019

Sorry, hasn't been the best day for me.

No worries. Thanks for testing.

I'd like to get some confirmation that the process substitution syntax does work on Bash 4.0.

I tested it successfully on 4.4.23(1)-release on NixOS.

It does work for bash-5.0 on my machine, so I guess we just need to bisect minor versions until we find the first working one. I'll take a crack at it tomorrow--I actually downloaded all the source archives for various bash versions for testing exactly this kind of stuff, but I haven't gotten around to actually building them yet.

Termux has Bash 5 so we could just test for >= 5 if you want. I guess it could be a good idea to figure out which minor version fixes it.
image

@bbigras bbigras changed the title Fix for Termux not having '/dev/stdin' fix: fix for Termux not having '/dev/stdin' Aug 26, 2019
@chipbuster
Copy link
Contributor

@bbigras On linux, the earliest version it works with is bash 4.1. Your call--we can either look specifically for that version or we can just go for 5.0 and up.

@bbigras
Copy link
Contributor Author

bbigras commented Aug 27, 2019

I updated the PR to check for 4.1. I hope I did it right.

@chipbuster
Copy link
Contributor

After further testing, it seems that neither init system works correctly for bash-4.0. The shell acknowledges that PROMPT_COMMAND is set to starship_precmd, and it even executes the line that alters PS1, but then the prompt never actually changes.

Given that that's not within the scope of this PR, I went ahead and reverted the version change (just so that it's easier to tell what's going on), and we can open an issue if bash-4.0 support is ever important to anyone.

Thanks for the work, and sorry about the complications of testing! Every time I wander into shell-land, it seems things become more complex than they used to be.

@chipbuster chipbuster merged commit fa2d1c0 into starship:master Aug 27, 2019
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.

termux: 'bash: /dev/stdin: No such file or directory"
2 participants