Handle bold/strong nested inside italics/em #307
Conversation
|
I don't see how the testcase makes the case for this PR? Don't understand the random |
|
@miekg It does seem to fix the following: Per babelmark, most other return: The case |
…t treat them as the italics marker This is giving us left-to-right precedence for applying styles (which is why the one existing test case changed)
This is a feedback loop from the "is next character c?" check. Without this move, we may as well just `return 0, nil` from the "is next character c?" check. This is an explicit edge case of ending with a triple
…bold and italics ordering case
cddude229
force-pushed
the
chris-patch-nested-italics-and-bold
branch
from
b27d897
to
46938c9
Compare
Apologies - I got lazy at the end of the work day and should have provided more context. I've updated the PR description and split the main commit into two (to better highlight what each change does) in hopes it better communicates the goals. I also realized another edge case and added a test for that + fixed it. This reverted the optimization case I was talking about as the bug was there instead. Lemme know if you need more information! |
| @@ -1201,7 +1201,7 @@ func helperEmphasis(p *Parser, data []byte, c byte) (int, ast.Node) { | |||
| } | |||
|
|
|||
| if i+1 < len(data) && data[i+1] == c { | |||
| i++ | |||
| i += 2 | |||
There was a problem hiding this comment.
i++ guarantees the next character checked is c. This means helperFindEmphChar above returns a guaranteed 0 for length, which then short-circuits to the return 0, nil case. We choose to skip the next c character here by incrementing by 2 instead. (This will expose a separate bug if we're actually in the triple c case, which is what the next commit fixes.)
| @@ -1192,9 +1192,6 @@ func helperEmphasis(p *Parser, data []byte, c byte) (int, ast.Node) { | |||
|
|
|||
| for i < len(data) { | |||
| length := helperFindEmphChar(data[i:], c) | |||
| if length == 0 { | |||
There was a problem hiding this comment.
Building on last commit, in the triple c case, we'd still have short-circuited here - but we actually want to claim this c as our own. So we do the rest of the validity checks first, and then do this check at the end to exit the loop
|
@miekg Just following up on this, as I think I addressed all your concerns. Let me know your thoughts and if you need me to do anything else! |
While trying out this library, my initial input ran up against some edge cases. (It handled everything else perfectly though, which is awesome.). I decided to take a stab at fixing the bugs, which is this PR. I've broken down every test case addition + related fix into its own commit for easy review.
Here's the summary of changes in input/output:
*italics **and bold** end*<p>*italics <strong>and bold</strong> end*</p><p><em>italics <strong>and bold</strong> end</em></p>*italics **and bold***<p>*italics <strong>and bold</strong>*</p><p><em>italics <strong>and bold</strong></em></p>***bold** and italics*<p>*<strong>bold</strong> and italics*</p><p><em><strong>bold</strong> and italics</em></p>*start **bold** and italics*<p>*start <strong>bold</strong> and italics*</p><p><em>start <strong>bold</strong> and italics</em></p>*improper **nesting* is** bad<p>*improper <strong>nesting* is</strong> bad</p><p><em>improper **nesting</em> is** bad</p>