-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not escape comments #110
Conversation
|
||
let re_end_comment = Re.(compile @@ str "-->") | ||
let escape_comment = | ||
Re.replace_string ~all:true re_end_comment ~by:"-->" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In HTML (and IIRC XML), even --
is not allowed in comments, so it may be more sensible to replace -+
(as a regexp) with -
. Also, the replacement >
doesn't have any special meaning in comments.
Actually, in HTML5, |
Ok, here are all the cases for a comment ending "prematurely":
Only (3) represents a valid comment terminator, but recovering parsers (such as in browsers) are required to end the comment in all of these situations. (4) is a bit irregular: comment data can start with Probably, replacing |
@aantron Thanks a lot, I modified the code to handle all the cases. |
👍 LGTM |
I should qualify that – I looked at the comments. The XML declaration and DOCTYPE output code does look a bit sketchy, depending on where the arguments can come from. |
I doubt people will have user input ending up there. |
See #90
This is only a simplistic fix (and it adds re as a dep of the functor part, which is annoying).