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.

Sign up for GitHub

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

Jump to bottom

🐛 Fix bash version check #51

Merged
merged 1 commit into from
Jul 28, 2020
Merged

🐛 Fix bash version check #51

merged 1 commit into from
Jul 28, 2020

Conversation

louib
Copy link
Contributor

@louib louib commented Jul 24, 2020 •
edited
Loading

Hey @mrowa44 👋

The bash version check was performed after declaring the
emoji array, so an error message was displayed by bash
before printing the custom error message. I also changed
the exit code to 1 (error) in the case of an old bash version.

Output before:

$ ../bash-3.2.57/bash emojify
emojify: line 28: :100:: syntax error: operand expected (error token is ":100:")
Oh my! That’s a very old version of bash you’re using, we don’t support that anymore :(
 
Consider upgrading it or, if you must use bash 3.2.57(1)-release download an old version of emojify from here: https://github.com/mrowa44/emojify/blob/old_bash_support/emojify

Output after:

$ ../bash-3.2.57/bash emojify
Oh my! That’s a very old version of bash you’re using, we don’t support that anymore :(
Consider upgrading it or, if you must use bash 3.2.57(1)-release, download an old version of
emojify from here: https://github.com/mrowa44/emojify/blob/old_bash_support/emojify

Thanks for reviewing 😄

@louib
🐛 Fix bash version check
0d2ab3a 
The bash version check was performed before declaring the
emoji array, so an error message was displayed by bash
before printing the custom error message. I also changed
the exit code to 1 (error) in the case of an old bash version.
@GrenderG
Copy link
Collaborator

Makes sense, thanks!

@mrowa44 mrowa44 merged commit 6dc2c1d into mrowa44:master Jul 28, 2020
@louib louib deleted the fix_version_check branch July 28, 2020 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GrenderG GrenderG GrenderG approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@louib @GrenderG @mrowa44