-
-
Notifications
You must be signed in to change notification settings - Fork 169
Add a Highlight Extension ==for highlights== #672
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
Conversation
kivikakk
left a comment
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.
This is looking great! One minor change needed, but otherwise all looks very good.
One comment: this currently parses both =x= and ==x== as valid highlights. Is this intentional?
src/tests/highlight.rs
Outdated
| "<p>This is <mark>important!</mark>.</p>\n", | ||
| "<p>This is <mark>very important</mark> OK?</p>\n", | ||
| "<p><mark>It is a\nshrubbery</mark></p>\n", | ||
| "<p>Vendo <mark>Opel Corsa <strong>em bom estado</strong></mark></p>\n" |
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.
:D
src/parser/inlines.rs
Outdated
| b'\'' => 4, | ||
| b'_' => 5, | ||
| b'*' => 6 + (if c.can_open { 3 } else { 0 }) + (c.length % 3), | ||
| b'*' => 7 + (if c.can_open { 3 } else { 0 }) + (c.length % 3), |
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.
We should leave this as-is, otherwise the c.can_open with c.length == 2 case will overlap with the = case (both giving an index of 12).
| b'*' => 7 + (if c.can_open { 3 } else { 0 }) + (c.length % 3), | |
| b'*' => 6 + (if c.can_open { 3 } else { 0 }) + (c.length % 3), |
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.
That's actually a typo. I originally meant to place the = before that, but then changed my mind and forgot to change it back. Thanks for spotting it!
|
Thank you so much for this! One minor change needed and one query, otherwise ready to go. :) |
No, not at all. I guess that strikethrough does work with only one Actually, let me add a test case for that. |
It does, yep!
That should do it; you'll probably want to separate the strikethrough and highlight cases there! |
9d434e5 to
de96106
Compare
|
OK, I think I figured it out. I see you also made some changes in the meantime. I hope I didn't mess the rebase up. |
|
This is looking excellent. Thank you so much for your work on this! |
|
Thanks! |
|
This was released in Comrak v0.48.0! |
Closes #553.
This seems to have been attempted in the past, but the PR seems to have gone stale. I'm not 100% sure about
inlines.rs:1158, some clarification would be great.Hope this helps, it's a much-needed feature.