gvvaughan / lyaml

LibYAML binding for Lua.
gvvaughan.github.io/lyaml
Other
208 stars 34 forks source link

Geert56 #56

Open geert56 opened 1 year ago

geert56 commented 1 year ago

Merely adding an option to load() to distinguish empty sequences from empty mappings in the returned Lua table. Otherwise both would be represented by an empty Lua table. This is by the way still the case, however for an empty sequence, the Lua table will have an attached empty metatable. This presence can be tested with getmetatable(). So even with the option distinct_sequence set to true, any existing application code will not be affected unless it is attaching metatables to (some components of) the returned Lua table.

codecov[bot] commented 1 year ago

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.93%. Comparing base (37a9e51) to head (d803314).

Files Patch % Lines
lib/lyaml/init.lua 75.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #56 +/- ## ========================================== - Coverage 97.14% 96.93% -0.21% ========================================== Files 4 4 Lines 420 424 +4 ========================================== + Hits 408 411 +3 - Misses 12 13 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gvvaughan commented 1 year ago

Thanks for working on this!

In principle this looks great. What do you think about creating a single module level metatable?

...maybe with some identifying content, {_type='LYAMLSequence'} perhaps, and using that whenever necessary rather than making a new empty metatable every time? That makes it more future proof if we decide to expand on tagging returned objects with a metatable, and a lot easier to identify that a metatable being examined really is the one that was added by lyaml.

Bonus points for adding a specl example to verify it works as designed!

geert56 commented 1 year ago

Indeed a module level metatable would be a bit cheaper, but mind that empty sequences are not usually predominant. I would say they hardly are ever used in YAML.Going even beyond your idea, one could imagine that all "tree" nodes (represented as Lua tables) have associated metatables that store certain attributes, like indeed type, but maybe also file location coordinates, etc. Again, this could be made optional and only there when requested.