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

Add linux-readline target support for 5.4.x #64

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

Conversation

Kazuo256
Copy link

According to Lua 5.4.x's README:

If you're running Linux, try "make linux-readline" to build the interactive Lua interpreter with handy line-editing and history capabilities.

Not a very good bash scripter, so maybe this is not the best approach.

@umireon
Copy link
Contributor

umireon commented Jun 21, 2021

This project won't permit bashisms.

@Kazuo256
Copy link
Author

I'm not familiar with the expression "bashisms". What does it mean?

@umireon
Copy link
Contributor

umireon commented Jun 21, 2021

This works with bash but does not work with POSIX sh. This is called bashisms.

[[ "${version}" =~ 5[.]4[.][0-9]+ ]]

You can detect them by ShellCheck.

$ shellcheck -s sh -e SC2039 luaver
 
Line 2:
[[ "${version}" =~ 5[.]4[.][0-9]+ ]]
^-- SC3010: In POSIX sh, [[ ]] is undefined.

You can use awk.

[ -n "$('echo' "${version}" | 'awk' '/^5\.4\./')" ]

@Kazuo256 Kazuo256 force-pushed the upstream-pr/readline-support branch from 1e618de to c43b23e Compare September 12, 2022 22:02
@Kazuo256
Copy link
Author

Sorry for the long delay, I somehow didn't receive any notifications for the reply.

I changed the PR to use the code you suggested. Testing locally, it seems to work.

See the following PR's discussion for more details:

DhavalKapil#64
@Kazuo256 Kazuo256 force-pushed the upstream-pr/readline-support branch from c43b23e to 4af37ba Compare September 12, 2022 22:06
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