wooorm / markdown-rs

CommonMark compliant markdown parser in Rust with ASTs and extensions
https://docs.rs/markdown/1.0.0-alpha.21/markdown/
MIT License
965 stars 53 forks source link

The spread and ordered fields in list and list items nodes should be optional #131

Closed bnchi closed 2 months ago

bnchi commented 2 months ago

I've noticed that in the rust implementation the spread field for lists and list item node are not optional while in the docs it's mentioned that they can be null referring back to the mdast docs :

https://github.com/syntax-tree/mdast?tab=readme-ov-file#list https://github.com/syntax-tree/mdast?tab=readme-ov-file#listitem

while in the rust representation :

https://github.com/wooorm/markdown-rs/blob/af3ebbcff48a9c0e990e25299b9c832fd7857650/src/mdast.rs#L633 https://github.com/wooorm/markdown-rs/blob/af3ebbcff48a9c0e990e25299b9c832fd7857650/src/mdast.rs#L657

bnchi commented 2 months ago

The same goes for the ordered field in the mdast documentation the definition is :

An ordered field can be present. It represents that the items have been intentionally ordered (when true), or that the order of items is not important (when false or not present).

it can be present or not which I think means they're nullable ?

wooorm commented 2 months ago

mdast defines them this way, so that people in JavaScript can create objects without those fields, and that every tool will support those nodes. In rust, plain objects don’t exist. The optionality doesn’t add anything. Changing this here only makes the data bigger (because now there are more states), and all the code more complex (because now there are 3 values for false).