Skip to content

Conversation

@florentpoujol
Copy link

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

…ng (only partial support, several cases will not work)
…herever there is other characters than digits, dots or spaces in the input string (like Lua's tonumber() does)
… in the pattern (like I did on november 9 with lua_gmatch_next() )
…ad of an array (no matter the arraymode property)
@florentpoujol
Copy link
Author

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
Copy link
Contributor

elisee commented Jan 8, 2014

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

@florentpoujol
Copy link
Author

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

@florentpoujol
Copy link
Author

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

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.

4 participants