lua.js
lua.js copied to clipboard
Missing string functions + other fixes
Hey
I added the missing string functions : find, format, gsub, gmatch and match. All cases of pattern translation I encountered are working OK (yet there are probably many cases that won't).
I also fixed several other functions like ensure_arraymode(), pairs(), tonumber(), os.clock().
Looks like %b
is not supported. Am I right?
You are right. I hadn't any use case for this pattern so I forgot about it. I am not sure yet how I could implement it but I will give it a proper look.
NB: You could probably use a test suite, like fperrad/lua-TestMore, to detect such omissions.
First, I want to thank you for contributing to lua.js. I hope you don't consider my comments too pedantic, I'm used to taking style pretty seriously (in fact I want to go through and correct my own style mistakes!). I am actually really grateful for your time and effort here.
I would recommend you try adding tests to your code as well (added by another contributor). It's pretty simple, just add new files to tests/, you can check out the files that are already there for examples.
Thanks again!
Resume of what I did since the last batch of commits :
- Fixed all styles issues as reported (braces, inline var declaration, etc...). (note that lua_add, lua_substract & Co are doing some inline variable declaration).
- Improved some patterns, made unsupported patterns to call not_supported()
- Added very partial support for the %b pattern (it's supported when it's the only item in a pattern).
- Fixed a lot of bugs in the string functions
The cases where I had table.arraymode set to true but table.uints as object is fixed. It was due to another technology (CraftStudio's web player) that would add arraymode itself, yet left table.uints as an object.
I am new to node.js and I couldn't find how to run the tests in the "tests" folder. Which is why I hadn't them included in the pull request yet. Can you explain ?
Thanks !
@florentpoujol looking at the package.json file, you should be able to run the tests with npm run test
.
@mherkender Hey, can you find time to review my latest commits ? Thanks !
@mherkender Maybe it's time to do something about these improvements ?
They have been used without issues in several of my projects. The Travis CI build failed but apparently because of Node itself, not because a test failed. So I am not sure what I can do from here...