Missing string functions + other fixes

mherkender / lua.js

An ECMAscript framework to compile and run Lua code, allowing Lua to run in a browser or in Flash

http://blog.brokenfunction.com/

600 stars 73 forks source link

Missing string functions + other fixes #35

Open florentpoujol opened 10 years ago

florentpoujol commented 10 years ago

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().

agladysh commented 10 years ago

Looks like %b is not supported. Am I right?

florentpoujol commented 10 years ago

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.

agladysh commented 10 years ago

NB: You could probably use a test suite, like fperrad/lua-TestMore, to detect such omissions.

mherkender commented 10 years ago

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!

florentpoujol commented 10 years ago

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 !

elisee commented 10 years ago

@florentpoujol looking at the package.json file, you should be able to run the tests with npm run test.

florentpoujol commented 10 years ago

@mherkender Hey, can you find time to review my latest commits ? Thanks !

florentpoujol commented 9 years ago

@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...