Escape literal emphasis markers merged into a text token - #480
Conversation
MarkdownRenderer.text() escaped a literal "*"/"_" only when the whole text
token consisted of markers. That holds while a paragraph has no real emphasis,
where "\*baz\*" arrives as three tokens ("*", "baz", "*"), but not once it
does: the inline parser then hands the renderer one merged token, so
a *b* c \*d\* e
re-emitted as "a *b* c *d* e" and the literal markers came back as emphasis.
Whether a literal marker survived a round-trip depended on unrelated content
elsewhere in the same paragraph.
Escape per marker run instead of per token. A run needs a non-space character
on one side to open or close emphasis, and a "_" run also cannot do so from
inside a word, so those two cases stay bare and prose such as "2 * 3" or
snake_case is still emitted unescaped.
Escaping the text of an autolink would have demoted it to a verbose inline
link, backslashes and all, for any URL containing "_" (a Wikipedia article, a
dunder path), so match the "<url>" shortcut on the raw text rather than on the
escaped render.
One known edge, unchanged in kind by the run-based rule but worth naming: a
bare unmatched marker sitting directly against real emphasis markers, as in
"a***a*a*", is escaped where the token ends, which shortens the source's
delimiter run and can drop the emphasis. It needs an unescaped leftover marker
adjacent to emphasis punctuation; input that spells "\*" deliberately is
unaffected.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #480 +/- ##
==========================================
+ Coverage 91.18% 91.21% +0.02%
==========================================
Files 36 36
Lines 3631 3641 +10
Branches 677 677
==========================================
+ Hits 3311 3321 +10
Misses 193 193
Partials 127 127 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I've encountered this too: import mistune
from mistune.renderers.markdown import MarkdownRenderer
reformat = mistune.create_markdown(renderer=MarkdownRenderer())
print(reformat(r"(2) \\(2^n\\)"))Output: (Ends up as Expected (the same as input): (Renders to It seems that a broader fix is needed. (Somewhat related, may help: xberg-io/html-to-markdown#458.) |
MarkdownRenderer.text() re-emitted a literal backslash bare, so a re-parse consumed it as an escape for whatever followed. KaTeX's "\\(2^n\\)" came back as "(2^n)", and a backslash ending a line turned into a hard line break. Escape it first, ahead of the "*"/"_" and backtick escapes, the way _escape_title() already does for the same reason. The escape is unconditional: parse_escape gives each escaped character its own text token, so by then there is no following character to condition on, and a lookahead rule misses the reported case entirely. The cost is a backslash that would not have been consumed being doubled too, so a Windows path is re-emitted with doubled separators. One of the 652 bundled CommonMark examples changes this way. Over that corpus, meaning- changing round-trips drop from 85 to 79 with none newly broken. Reported by kbulygin on lepture#480.
MarkdownRenderer.text() re-emitted a literal backslash bare, so a re-parse
consumed it as an escape for whatever followed. KaTeX's "\\(2^n\\)" came back
as "(2^n)", and a backslash ending a line turned into a hard line break.
Escape it first, ahead of the "*"/"_" and backtick escapes, the way
_escape_title() already does for the same reason.
The escape is unconditional. parse_escape gives each escaped character its own
text token, so "(2) \\(2^n\\)" reaches the renderer as five tokens and the
backslash never sees the "(" beside it: a rule that looks ahead for escapable
punctuation misses the reported case entirely, and one that also escapes on a
token edge costs a regex plus that special case to buy back a single example
in the bundled CommonMark corpus.
The price is a backslash that would not have been consumed being doubled too,
so a Windows path is re-emitted with doubled separators. Over the 652 bundled
examples, meaning-changing round-trips drop from 85 to 79, none newly broken.
Reported by kbulygin on lepture#480.
c0ddb80 to
4ee0b44
Compare
|
Thanks -- same root cause one character over. Pushed a fix on this branch rather than opening a second PR, since it lands on the same three lines of the same method. The backslash is escaped first, ahead of the >>> print(reformat(r"(2) \\(2^n\\)"))
(2) \\(2^n\\)It also picks up a structural case I had missed: a literal backslash ending a line was re-emitted bare and came back as a hard line break, splitting a paragraph in two. That is in the test alongside your repro. On over-escaping, since it is the obvious objection. The escape is unconditional, so a backslash that would not have been consumed gets doubled too and a Windows path comes back with doubled separators. Conditioning on the following character is harder than it looks, because >>> [t["raw"] for t in ast(r"(2) \\(2^n\\)")]
['(2) ', '\\', '(2^n', '\\', ')']The backslash never sees the @lepture -- I rechecked the whole PR against current One divergence to disclose, since it is the only thing that sweep flagged: src = "[\\`]`"
# before this branch: unchanged and stable
# after: '[\\\\`]`', doubling on every further passThat is a pre-existing parser bug rather than a renderer one. The literal backslash then lands in a text token, and the renderer now faithfully escapes it, so an input mistune already mis-parses grows a backslash per pass instead of resting on a broken fixed point. I tried a guard that skips a candidate preceded by an odd number of backslashes: it gives the CommonMark-correct output above and changes no example in the bundled corpus. That belongs in its own PR though -- happy to open one if you want it. Used AI assistance on this; I reviewed and tested the change myself. |
MarkdownRendererre-emits a literal*/_bare when the paragraph also contains real emphasis, so reformatting turns plain text into emphasis.Root cause
text()escaped markers only when the whole token was made of them:That condition depends on inline-token fragmentation. With no emphasis in the paragraph,
\*baz\*arrives as three tokens (*,baz,*) and each marker token is escaped. Once real emphasis is present the parser hands the renderer a single merged token (*bar* \*baz\*→emphasis+' *baz*'), the condition is false, and the markers are re-emitted bare. So whether a literal marker survived a round-trip depended on unrelated content elsewhere in the same paragraph.Fix
Escape per marker run rather than per token. A run needs a non-space character on one side to open or close emphasis, and a
_run also cannot do so from inside a word, so those two cases stay bare —2 * 3andsnake_caseare still emitted unescaped, as b042996 intended.The two exemptions deliberately mirror
_can_open_emphasis/_can_close_emphasisin_inline/emphasis.pyand use the samestr.isspace()/str.isalnum()predicates, so the renderer cannot disagree with the parser about what a delimiter is.One follow-on: escaping the text of an autolink demoted it to a verbose inline link for any URL containing
_(<https://en.wikipedia.org/wiki/Foo_(bar)>→[https://en.wikipedia.org/wiki/Foo\_(bar)](...)). The<url>shortcut now matches on the raw text rather than the escaped render.Known edge
A bare unmatched marker sitting directly against real emphasis markers, as in
a***a*a*, is escaped where the token ends, which shortens the source's delimiter run and can drop the emphasis. Over an exhaustive sweep of{a,b,*,_}strings up to length 12 this affects 166 of 114,231 inputs against 4,521 fixed, and every affected input is an unescaped leftover marker adjacent to emphasis punctuation — never text that spells\*deliberately, which is what this is protecting. Happy to take a different trade-off if you would rather not escape at a token edge, though that reopens the case above.Testing
Three cases added to
TestMarkdownRendererRoundTrip; each fails without its source change (verified by reverting each hunk separately):test_escaped_marker_alongside_real_emphasis— the bug.test_literal_markers_escaped_only_where_they_could_delimit— asserts the emitted Markdown, not the HTML.assert_round_tripcompares HTML and so cannot see a stray backslash; without this, the two exemptions could be deleted with the suite still green.test_autolink_url_containing_a_marker— the autolink shortcut.Full suite passes (1160),
ruff checkandmypyclean. Reformatting all 652 bundled CommonMark spec examples and re-rendering goes from 85 meaning-changing round-trips to 82, with no new failures in any section.Used AI assistance on this; I reviewed and tested the change myself.