Repository navigation
Conversation
Due to the way Python Markdown processes content, we must safeguard against content placed into tails that we've already processed. 1. Built HTML elements we plan to replace ahead of time and place in the cache. They are built as the text is when they were processed. Each entry in the cache will contain the element, the region it replaces, and the anchor (position of next delimiter Python Markdown will before that that region so we can account for new index of the modified buffer). 2. While the cache is not empty, we will sometimes receive tail content, already processed tail content so we can identify and ignore them. 3. Now we just return the pre-built HTML elements until the cache is spent.
Preserve the unused part of a closing-only delimiter run as literal text when its matched prefix is replaced with an inline placeholder. The changed context can otherwise turn the remainder into an opener. When the rule of three selects an earlier opener, remove the rejected openers above it before pairing. Updating only the length can consume the wrong number of closing delimiters and drop ordinary text. Restore twelve nested test methods to unittest discovery and use the existing assertion helper. Correct the expectations that encoded dropped text or contradicted the delimiter rules, and add an unreleased note. Follow up on Python-Markdown#1696; related to Python-Markdown#1694. Preserve the original PR commits as the base for this change. Co-Authored-By: GPT-6 Astra
iSanae233
changed the title
|
It would have been more helpful to comment on this case in #1696 instead of opening a new review on top of the current, ongoing review. The emphasis handling here strongly follows the solution in BetterEm, minus the punctuation rules.
These dropped text cases are now all fixed in #1696. I will be closing this PR in favor of #1696. |
iSanae233
left a comment
iSanae233
left a comment
There was a problem hiding this comment.
Thanks for the explanation, and sorry for opening a separate PR while #1696 was still under review. I should have shared the additional reproductions and findings there first.
I appreciate you looking into these cases and addressing them in #1696. I’ll keep follow-up findings in the existing discussion next time to avoid splitting the review.
|
@iSanae233 Please do report anything if you find missing gaps. I'm trying to keep the changes synced here with the reference in https://github.com/facelessuser/pymdown-extensions/blob/main/pymdownx/delimiterprocessor.py, except where the two projects purposely deviate. In this case, it was a porting issue on my part, which I'm glad you caught. But since all of these issues have been solved there, it was more about figuring out where the port went wrong, or what we missed or assumed wasn't needed here that actually was. I didn't want to introduce a different solution that was missing what the original solution had found and fixed. |
iSanae233
left a comment
iSanae233
left a comment
There was a problem hiding this comment.
Thanks for clarifying, and for the encouragement. I understand why you want to keep the implementation aligned with the reference while preserving the intentional differences between the projects.
If I find any further gaps, I’ll report them in the existing discussion with reproductions, keeping that context in mind.
Description
Some emphasis sequences still lose text or change delimiter pairing after applying #1696. This follow-up preserves the original context of unused closing delimiters and pairs a closing run with the opener actually selected by the rule of three.
This branch depends on #1696. It preserves the two original commits from that PR and adds one follow-up commit. The examples below were reproduced both on 3.11.3 and on #1696 at
fe3d5a68f156dd23032237a799e5e4eeace726bd.The follow-up is 6133a59; its diff shows only the additional fixes and test corrections on top of #1696.
First input:
Before this change:
After this change, matching 3.11.2 and the expectation already written in the upstream test:
The old result loses the colon. A second input,
_a___"_"__: x, changes from:to:
There are two related causes:
The change also restores twelve test functions that were nested inside
test_commonmark, so unittest actually collects them, and replaces their undefinedcheck_markdowncalls withassertMarkdownRenders. The two reproductions above keep their existing expected output. Three other expectations are corrected: one had already dropped the letterg, one contradicted the rule of three, and one assumed CommonMark punctuation rules that Python-Markdown intentionally relaxes. The dropped-letter and rule-of-three results were cross-checked with commonmark.js 0.31.2.The unreleased changelog describes the fix. This is related to #1694, but does not claim to fix every emphasis edge case or the separate link-parsing complexity issues.
Validation
python3 -m unittest discover tests -vbuild --strictgit diff --checkpython3 -m flake8 markdown testsThe 111 skips are existing upstream conditions: 60 Pygments version-gated tests, 39 tests requiring Tidylib, eight excluded cases, and four known-behavior cases. They were not added by this change. The full CI Python/OS matrix, the pinned Pygments environment, tox pep517check, Zensical documentation link checking, and spelling checks have not been run locally. The successful wheel build and MkDocs sample build do not replace those checks.
Local Codex review reported no actionable regressions. A scoped Codex Security scan of the same code tree completed with zero findings. Those tools also passed the focused 47-test emphasis suite, but their broader runtime checks had missing dependencies; the complete-suite result above comes from the separately prepared openEuler environment. No automated review was requested on the upstream PR.
AI Assistance Disclosure
reviewed and verified their output.
AI assistance was used. The second checkbox is intentionally unchecked in this draft because the submitting author's own review has not yet been confirmed.
AI tools were used to assist with the following:
parser, and local code/security review
Tools: OpenAI Codex Desktop ,Codex Security CLI 0.1.31 with scan plugin 0.1.95.
The follow-up commit includes
Co-Authored-By: GPT-6 Astra.The original #1696 commits retain their authorship and existing disclosure.
Checklist
this PR.