Skip to content

Fix pygments block matching#462

Merged
nicholasserra merged 4 commits intotrentm:masterfrom
mhils:fix-pygments-block-matching
Aug 16, 2022
Merged

Fix pygments block matching#462
nicholasserra merged 4 commits intotrentm:masterfrom
mhils:fix-pygments-block-matching

Conversation

@mhils
Copy link
Copy Markdown
Contributor

@mhils mhils commented Aug 2, 2022

This PR is a fix for mitmproxy/pdoc#421. Consider the following Markdown:

# Section 1

```python
x = 1
```

# Section 2

<div>
test
</div>

markdown2 would previously first convert the fenced code block to <div class="codehilite">...</div>, and then, in _hash_html_blocks, match from the first pygments <div class="codehilite"> to the final </div> at the end of the file1. Everything in between (such as # Section 2) would be skipped.

I also considered to directly hash the pygments output instead (mhils@9622c05), but that changes <p> paragraph wrapping and would be more of a breaking change. So I think this approach is preferrable.

Footnotes

  1. _strict_tag_block_re does this before _liberal_tag_block_re matches.

@nicholasserra
Copy link
Copy Markdown
Collaborator

The tests that were modified, they're all just html formatting changes right? Wanna make sure I didn't miss anything.

@mhils
Copy link
Copy Markdown
Contributor Author

mhils commented Aug 10, 2022

Yes, correct! :)

@mhils mhils force-pushed the fix-pygments-block-matching branch from 21fc597 to eb1938b Compare August 11, 2022 13:59
@mhils mhils force-pushed the fix-pygments-block-matching branch from eb1938b to f1634cc Compare August 11, 2022 14:00
@mhils
Copy link
Copy Markdown
Contributor Author

mhils commented Aug 11, 2022

I've rebased this onto master and also added a fix for mitmproxy/pdoc#429, which depends on the formatting changes here. 😃

@nicholasserra nicholasserra merged commit ac5e7b9 into trentm:master Aug 16, 2022
@nicholasserra
Copy link
Copy Markdown
Collaborator

LGTM thank you for the work on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants