kikito / inspect.lua

Human-readable representation of Lua tables
https://github.com/kikito/inspect.lua
MIT License
1.38k stars 196 forks source link

avoid __len metamethod #21

Closed Nymphium closed 8 years ago

Nymphium commented 8 years ago

I found the problem that inspect sometimes uses # operator to user defined table. If the table has __len metamethod, the function is called and an unexpected thing occur. To avoid this problem, you should use rawlen.

kikito commented 8 years ago

Hi, thanks for sending this PR.

I'd rather not modify the table just to calculate its length. Why do you think this is necessary?

Also, can you give me an example of a table built with a __len metamethod which gives an error? Ideally, in a test. But if you are not able to do that, just write the table here, along with what you get and what you think you should get instead.

Thanks again!

Nymphium commented 8 years ago

I found it was a little corner case. The problem occur with lulpeg

local inspect = require('inspect')
local lpeg = require('lulpeg')
print(inspect(lpeg.locale().space))

The code say:

lua: /home/nymphium/.luarocks/share/lua/5.3/inspect.lua:249: 'for' limit must be a number
stack traceback:
        /home/nymphium/.luarocks/share/lua/5.3/inspect.lua:249: in local 'f'
        /home/nymphium/.luarocks/share/lua/5.3/inspect.lua:196: in method 'down'
        /home/nymphium/.luarocks/share/lua/5.3/inspect.lua:242: in method 'putTable'
        /home/nymphium/.luarocks/share/lua/5.3/inspect.lua:291: in method 'putValue'
        /home/nymphium/.luarocks/share/lua/5.3/inspect.lua:322: in function 'inspect.inspect'
        (...tail calls...)
        stdin:3: in main chunk
        [C]: in ?
mpeterv commented 8 years ago

I think it should be enough to use # if rawlen is not defined, because in Lua 5.1 __len doesn't work for tables.

Nymphium commented 8 years ago

Oh, i didn't know __len doesn't work in Lua 5.1. Thak you.

kikito commented 8 years ago

Thank you for providing an example. I have changed the .travis.yml file so that more versions of Lua are included in the tests (5.1, 5.2, 5.3 & two versions of LuaJIT). I will try to write a test for this edge case and a fix soon.

kikito commented 8 years ago

Thanks! I have merged this, adding a test to make sure it works as intended. I will now release a new version of the lib.