lyaml icon indicating copy to clipboard operation
lyaml copied to clipboard

Geert56

Open geert56 opened this issue 1 year ago • 3 comments

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.

geert56 avatar Mar 13 '23 13:03 geert56

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

codecov[bot] avatar Mar 13 '23 18:03 codecov[bot]

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!

gvvaughan avatar Mar 13 '23 19:03 gvvaughan

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.

geert56 avatar Mar 13 '23 19:03 geert56