Reject emoji-parsing if the slice ends with ** - #947
Conversation
|
@xoofx, will you have a look? |
Yes, I'm on holidays, but will check when I'm back |
MihaZupan
left a comment
MihaZupan
left a comment
There was a problem hiding this comment.
Please add tests for any such behavior changes
| { | ||
| // Only look at the following char if the emoji ends with a '*', otherwise it is not needed. | ||
| var followingChar = slice.PeekChar(match.Key.Length); | ||
| if (lastEmojiChar == '*' && followingChar == '*') |
There was a problem hiding this comment.
lastEmojiChar == '*' is already checked above
| return false; | ||
| } | ||
|
|
||
| // If the found emoji ends with a ´*´, the following char must not be a '*'. |
There was a problem hiding this comment.
Your current example is
**Non-goals (explicitly out of scope for this plan):**
but why would we treat that differently for emoji compared to italic
*Non-goals (explicitly out of scope for this plan):*
or other types of emphasis?
There was a problem hiding this comment.
Which parser detects italic?
There was a problem hiding this comment.
There was a problem hiding this comment.
Hmmm. I see the point. The EmphasisDescriptor takes a minumum and maximum count - bold is maximum count and italic is minimum count.
So actually the EmojiParser need to know if an EmphasisParser has open emphasis or not - and reject the emoji ending in ´*´ if there is an open emphasis at the point of it.
|
We still need to have tests for this PR |
|
I looked a bit more into the code, and started wondering why the Emphasis parser is an inline parser instead of a block parser. Is there anything in Markdown specification that forces it to be inline? All Emphasis instances are as far as I see it blocks per nature since they have a starting tag/definition and an ending tag/definition (like bold being started and ended with a double If so, it would be a question of finding the ending Emphasis descriptor for the starting Emphasis descriptor found - and then running a new parser set on everything between and not including the Emphasis start and end. This would also parse my example line correctly as the ending Emphasis descriptor would be evaluated prior to evaluating Emojis. |
Yes, block have precedence over inline, and block and inline parsers are quite different in their behavior. See also https://spec.commonmark.org/0.31.2/#blocks-and-inlines The work for solving such problem (an extension like emoji perturbating core functionalities) should be best effort without causing disruption to the core. Unfortunately, it's not always easy and sometimes it requires to add specific code path to the core to handle plugins cases. Will check if it can be further improved from your PR. |
Resolve trailing emphasis delimiters before converting ambiguous emoji matches. Reuse the existing emphasis parser rather than guessing whether an opener remains active or rejecting only double asterisks. Cover italics, strong/nested emphasis, custom delimiters, links, images, tables, source positions, renderers, all default mappings, and 2688 emphasis-structure comparisons. Document emphasis and generic-attribute precedence and the named-shortcode escape hatch.
3 participants
This pull request introduces an additional validation step in the emoji parsing logic to prevent false positives. Specifically, it ensures that emoji candidates ending with two asterisks (
**) are not matched as valid emojis.Emoji parsing improvement:
Matchmethod inEmojiParser.csto returnfalseif an emoji candidate ends with two asterisks, preventing incorrect emoji matches in such cases.A sample markdown is provided on the #946 issue.
Closing #946