search

github / cmark-gfm

GitHub's fork of cmark, a CommonMark parsing and rendering library and program in C
Other
889 stars 173 forks source link

Tasklist items aren't identical to list items when it comes to what they can contain. #170

Open NightFlyer opened 5 years ago

NightFlyer commented 5 years ago

I think that tasklist items should be essentially identical to list items (except for the marker).

However, tasklist.c:can_contain essentially always returns true (since the only nodes that have the tasklist extension will have a type of CMARK_NODE_ITEM, and that is all that it checks). It doesn't check the child type at all. Thus, it says that it can contain any kind of child type.

For a regular CMARK_NODE_ITEM, the equivalent code is:

return CMARK_NODE_TYPE_BLOCK_P(child_type) && child_type != CMARK_NODE_ITEM

Thus, it is checking the child type is a block type and isn't another item.

I don't understand why these two should be so different.

kivikakk commented 5 years ago

Indeed, I think your analysis is right. (For some context on how there can be so many issues in the tasklist extension, it isn't actually used by GitHub, and instead was added by a contributor in #94.)

I'm open to updating this to match regular list items.

kivikakk commented 5 years ago

By the way, CI is now green on master, so future PRs should be green if nothing's broken 👍

NightFlyer commented 5 years ago

I may have some time to fix this, although it seems like a very low priority issue for now. The main thing is to have everyone expect that tasklist items and regular list items should behave as much like each other as possible -- the rest is just bug fixing.

Hurray on fixing the tests!