FEATURE: Add an AST normalization pass for target-format nesting rules#67
Merged
Conversation
gschlager
force-pushed
the
feature/ast-normalizer
branch
from
July 18, 2026 13:36
041798f to
c7592a9
Compare
This moves nesting-rule enforcement out of the migrations-tooling custom Url tag and into Markbridge. The default only fixes target-format legality and stays a general converter; the Discourse-specific policy, like moving an image out of a link, stays on the migrations side, but shrinks from a whole custom tag to a single rule. Real markup nests elements in ways Markdown cannot express: a link inside a link, an image inside a link, a block element inside a link label or inside bold. The renderer printed whatever the tree held, so these came out broken. The inner link wins and the outer one turns into plain text; a block's blank lines cut the [..](url) construct in half. migrations-tooling worked around the link cases in its own Url tag, but that is knowledge in one consumer that every other consumer would have to rediscover, and two copies of "what may nest in what" that drift. Markbridge::Normalizer walks the AST once, between the parse-time yield hook and rendering, and rewrites it so the renderer only gets markup the target format can express. The tags stay simple string emitters; the nesting rules live one level up, in one place. The pass runs for all four source formats and for Markbridge.render, so a consumer's own AST changes made in the yield hook get normalized too. The default rules are target-format legality only: no link inside a link (CommonMark forbids it at any depth), no block element inside an inline container (its blank lines would break the container), and a code span that stays inline only while it is on one line. Discourse policy, like moving an image out of a link, is not built in. A consumer adds it with one #rule call, which is what migrations-tooling does. Each match resolves to a strategy: :keep, :hoist_after, :unwrap, :textify, :drop, or a callable that decides at apply time. Everything the pass changes is reported through diagnostics[:normalization], next to unknown_tags. #violations lists what would change without touching the tree. Hoisting is the tricky part. There are no parent pointers, so the walker keeps the ancestor stack, takes the offending node out, and puts it back as a sibling right after its outermost offending ancestor. The moved subtree is walked against the stack it will have after the move, so a node that leaves a link never sees the link while its own inside is normalized: a legal quote inside a quote stays nested, and an image inside a quote inside a link stays in the quote. A wrapper left empty by a hoist is removed, so there are no empty ** ** markers, except an empty Url, which still renders as a bare link. The rules are built once. Normalizer.shared_default is a frozen instance reused across conversions; Normalizer.default returns a fresh one to customize with #rule. #normalize and #violations keep no state on the instance, so the shared frozen instance is safe to reuse, also across threads. It runs by default. normalize: false skips it, normalize: <Normalizer> swaps in a customized one. I benchmarked the violation-free path at about 2 microseconds per post, which does not show against render time, so I left it on by default.
gschlager
force-pushed
the
feature/ast-normalizer
branch
from
July 20, 2026 07:52
c7592a9 to
cfa3bba
Compare
gschlager
marked this pull request as ready for review
July 20, 2026 07:54
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This moves nesting-rule enforcement out of migrations-tooling's custom
Urltag and into Markbridge. Markbridge's default only fixes target-format legality and stays a general converter; the Discourse-specific policy — like moving an image out of a link — stays on the migrations side, but shrinks from a whole custom tag to a single rule.Real markup nests elements in ways Markdown can't express — a link inside a link, an image inside a link, a block element inside a link label or inside bold. The renderer printed whatever the tree held, so those came out broken: the inner link wins and the outer one degrades to plain text, a linked image renders as
[](url), a block's blank lines cut the[…](url)construct in half. migrations-tooling worked around the link cases in its ownUrltag, but that's one consumer holding rules every other consumer would have to rediscover, and two copies of "what may nest in what" that drift.Markbridge::Normalizerwalks the AST once, between the parse-timeyieldhook and rendering, and rewrites it so the renderer only gets markup the target format can express. The tags stay simple string emitters — the nesting rules live one level up, in one place. The pass runs for all four source formats and forMarkbridge.render, so a consumer's own AST changes made in theyieldhook get normalized too.The default rules are target-format legality only, so Markbridge stays a general markup→Markdown converter and does not bake in Discourse policy. No link inside a link (CommonMark forbids it at any depth). No block element inside an inline container, because its blank lines would break the container. A code span stays inside an inline container only while it is on one line; a fenced or multi-line block is moved out. Moving an image out of a link is a Discourse choice, not a legality rule, so it is not a default — a consumer adds it with a single
#rulecall, which is what migrations-tooling does.Each match resolves to a strategy —
:keep,:hoist_after,:unwrap,:textify,:drop, or a callable that decides at apply time — and everything the pass changes is reported throughdiagnostics[:normalization], next tounknown_tags.#violationslists what would change without touching the tree.Hoisting is the tricky part. There are no parent pointers, so the walker keeps the ancestor stack, takes the offending node out, and puts it back as a sibling right after its outermost offending ancestor. The moved subtree is walked against the stack it will have after the move, so a node that leaves a link never sees the link while its own inside is normalized: a legal quote inside a quote stays nested, and an image inside a quote inside a link stays in the quote. A wrapper left empty by a hoist is removed, so there are no empty
****markers — except an emptyUrl, which still renders as a bare link.The rules are built once.
Normalizer.shared_defaultis a frozen instance reused across conversions;Normalizer.defaultreturns a fresh one to customize with#rule.#normalizeand#violationskeep no state on the instance, so the shared frozen instance is safe to reuse, also across threads.It runs by default.
normalize: falseskips it,normalize: <Normalizer>swaps in a customized one. I benchmarked the violation-free path at about 2 µs per post — it does not show against render time — so I left it on by default; a post that needs fixing costs a few µs per change.