search

ocaml / omd

extensible Markdown library and tool in "pure OCaml"
ISC License
156 stars 45 forks source link

Suggestion to simplify add_uchar #282

Closed shonfeder closed 2 years ago

shonfeder commented 2 years ago

Hi, @tatchi. What do you think of this suggestion for the add_uchar function? The tests seem to be passing, and I think the logic follows, tho you've thought more about this than I have and I'm not sure if I may be missing an edge case.

I think we can do away with the flag to track start by ensuring that we add the space uchar when we first detect white space, then use the seen_ws to ensure we don't add repeated whitespace.

WDYT?

This PR targets your PR #277, so if you like the suggestion you can merge it in.

tatchi commented 2 years ago

Hey @shonfeder, thanks for the suggestion! It's a nice simplification but I don't think your solution works. As the spec says:

To normalize a label, strip off the opening and closing brackets, perform the Unicode case fold, strip leading and trailing spaces, tabs, and line endings, and collapse consecutive internal spaces, tabs, and line endings to a single space.

Your solution doesn't remove leading space(s). It correctly removes trailing space(s) and consecutive internal spaces though.

shonfeder commented 2 years ago

Ah! That explains the need for start :)

I think no tests are failing to catch this behavior? Could that be included in your PR?