ocaml / omd

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

fix test 206 #278

Closed tatchi closed 2 years ago

tatchi commented 2 years ago

Input:

[ΑΓΩ]: /φου

[αγω]

The current result in master:

File "tests/spec-206.html", line 1, characters 0-0:
diff --git a/_build/default/tests/spec-206.html b/_build/default/tests/spec-206.html.new
index 93c6540..97ea23b 100644
--- a/_build/default/tests/spec-206.html
+++ b/_build/default/tests/spec-206.html.new
@@ -1 +1,2 @@
-<p><a href="/%CF%86%CE%BF%CF%85">αγω</a></p>
+<p>[ΑΓΩ]: /φου</p>
+<p>[αγω]</p>

There are two issues here:

  1. Labels are not being matched, hence it is not recognized as a link. This is what I fixed in https://github.com/ocaml/omd/pull/277. I included that change in the first commit.

  2. The url destination is not percent encoded. From the spec:

    A link destination consists of either

    • a sequence of zero or more characters between an opening < and a closing > that contains no line endings or unescaped < or characters, or

    • a nonempty sequence of characters that does not start with <, does not include ASCII control characters or space character, and includes parentheses only if (a) they are backslash-escaped or (b) they are part of a balanced pair of unescaped parentheses. (Implementations may impose limits on parentheses nesting to avoid performance issues, but at least three levels of nesting should be supported.)

    And `ASCII control characters are:

    An ASCII control character is a character between U+0000–1F (both including) or U+007F.

    I'm not sure why '\x80' .. '\x9F' were being matched since, according to the spec, they're not considered as an `ASCII control characters. Removing them from the pattern fixes the test and doesn't break any other so I guess that's fine.

Fix #272

shonfeder commented 2 years ago

That's a good idea, @sonologico. Perhaps we could followup with a refactor to clean up that massive file at some point soon :)

shonfeder commented 2 years ago

Will wait to review here until it's been rebased/updated to account for the work its based on.

tatchi commented 2 years ago

Will wait to review here until it's been rebased/updated to account for the work its based on.

It's rebased 😊

shonfeder commented 2 years ago

Perfect, surgical fix here. We’ll diagnosed! Thanks :)