Repository navigation
Discussion: Markdown correctness and extension points for downstream integrations #37
Description
Activity
- Correct generic link and image parsing
Is this part of the official spec? If so we should support it
- Preserve raw HTML and distinguish literal text from markup
Would be good to fix indeed
- Finish structural Markdown before adding CMS syntax
Sure, any bug can be fixed
- Keep application behavior downstream
I don't really understand. Care to elaborate?
2. Correct generic link and image parsing
Is this part of the official spec? If so we should support it
Yes — CommonMark 0.31.2, §6.3 Links and §6.4 Images. An inline link is
[text](destination "title"): the title is a separate optional element, the destination may be wrapped in<...>(which is how a destination containing spaces is written), and a bare destination containing spaces makes the whole construct literal text.Current
mainnext to a reference implementation (league/commonmark, same input, highlighting off):Input Tempest mainCommonMark [x](/a "Title")<a href="/a "Title"">x</a><a href="/a" title="Title">x</a>[x](a b)<a href="a b">x</a>[x](a b)[x](</my uri>)<a href="</my uri>">x</a><a href="/my%20uri">x</a><img src="/a "Title"" alt="alt"><img src="/a" alt="alt" title="Title" />[x](/a&b)<a href="/a&amp;b">x</a><a href="/a&b">x</a>The last row is one I hadn't listed in the issue: entities in a destination are escaped twice.
Balanced parentheses and backslash escapes in destinations are already correct since #25 —
[x](/a(b)c)and[x](/a\)b)both match CommonMark onmain.I'll open a PR that parses destination, optional title, angle-bracket destinations and entities before rendering, and carries
titleonLinkToken/ImageTokenso it isn't folded intosrc/href.5. Keep application behavior downstream
I don't really understand. Care to elaborate?
That point was badly written, sorry. It's two related asks, and the first one already exists in the codebase.
1. A rendering hook per token.
ImageToken::parse()already does exactly what I'm describing:if ($parser->imageFactory) { return $parser->imageFactory->create($this->src, $this->alt)->html; }
That's the right shape. It's just wired to one concrete collaborator (
ResponsiveImageFactory), for one token. Generalising it — an integration registers a renderer for a token class and receives the parsed token — is all point 5 asks for.Why it matters downstream: in Pushword, images go through our own media pipeline (responsive sizes configured per site, an optional link wrapper, a placeholder comment when the file is missing). With no hook, we replace every image with a private-use-area marker before parsing, then substitute the rendered HTML back after:
TempestImageRestorer. We do the same for link titles with aPWTITLE0TOKENmarker. It works, but it means the Markdown you parse is not the Markdown the user wrote, and any change to your output format silently breaks our string matching.From our side it would look like:
$markdown->renderToken(ImageToken::class, fn (ImageToken $token) => $media->render($token->src, $token->alt, $token->title)); $markdown->renderToken(LinkToken::class, fn (LinkToken $token) => $links->render($token->content, $token->href));
2. The other half: app conventions in core should be removable.
LinkToken::parse()does:if (str_starts_with($href, '*')) { $href = substr($href, 1); $blank = ' target="_blank" rel="noopener noreferrer"'; }
*meaning "open in a new tab" isn't Markdown, it's a convention — same family as:::divs,@@,{x:handle}and automatic heading IDs. I'm not arguing they should go away; they're useful and I'd use several of them. The ask is only that they be rules an integration can remove or replace. Today an app that wants[text](*something)to keep its asterisk has no way to say so, and the same is true of heading IDs, which we currently strip back out of the HTML.So concretely: if #13 lands along the lines I proposed there, most of point 5 is solved — the conventions become removable rules — and what remains is the renderer hook.
3. Preserve raw HTML and distinguish literal text from markup
Would be good to fix indeed
The raw HTML half is #40:
pre,script,styleandtextareaare raw text elements in CommonMark, andHtmlTokenno longer re-parses their content as Markdown.<div>**a**</div>keeps parsing, since that's Tempest's own behaviour and a separate question from #20.The other half — escaping in text tokens — I've left alone for now.
a\*bkeeps its backslash,a_b_cbecomes emphasis, and"in a text node isn't escaped. That one needs a decision rather than a fix:TextToken::parse()currently returns its content verbatim, so escaping it correctly means deciding what happens to inline raw HTML that reaches a text token, and there's already a@todoabout Markdown escaping inLinkToken. Happy to take it on once you've said which way you want it to go.4. Finish structural Markdown before adding CMS syntax
Sure, any bug can be fixed
Four PRs, one per item, each with tests and a table of before/after:
- fix: keep the start number of an ordered list #41 —
2. Firstloses its list start number →<ol start="2"> - fix: parse tilde fenced code blocks #42 —
~~~fences parsed as strikethrough → fenced code, and a longer fence may now contain a shorter one - fix: require whitespace after an ATX heading marker #43 —
#titreparsed as a heading without the required space, and####### sevenemitting an<h7> - fix: render hard line breaks #44 — hard line breaks (
␣␣\nand\\\n) rendered as soft breaks
Still open from that list, and not in any of these PRs:
- Loose lists.
- a\n\n- brenders tight; CommonMark wraps each item in<p>. - Automatic heading IDs. Right now every heading gets one and there's no way to turn it off — we strip them back out of the HTML downstream. This is Make tokens and rules more configurable #13 territory rather than a bug.
- GFM task lists.
- [x] Donecurrently renders an empty-URL link. Best as an optional rule, I think. - Indented code blocks. Four leading spaces stay a paragraph.
Tell me which of those you want and in what shape, and I'll pick them up.
- fix: keep the start number of an ordered list #41 —
Four more gaps against the spec, found while diffing Pushword's content against
league/commonmark. Checked onmain(b4cd28e), highlighting off. No hurry, I know several PRs of mine are already waiting. Tell me which of these you want and I'll open them one at a time.1.
[label]without a destination becomes an empty linkInput Tempest mainCommonMark He said [sic] twice [1].He said <a href="">sic</a> twice <a href="">1</a>.He said [sic] twice [1].- [x] Done<li><a href="">x</a> Done</li><li>[x] Done</li>§6.3: a bare
[label]is only a link when a reference definition matches it. With none, the brackets are text. This is also why task lists rendered an empty link (point 4 above).test_lex_without_hrefasserts the current output ([click here]→<a href="">click here</a>), so I'd like to check before flipping it: is the empty link intended, maybe as a placeholder for reference links? If not, the fix is small:LinkRule::shouldParse()only matches when the closing]is followed by(. It doesn't touch #39.2. Code spans
18 of the 22 code span examples (§6.1, 328–349) differ, and some produce broken HTML:
Input Tempest mainCommonMark `` foo ` bar ``<code></code> foo <code> bar </code><code></code><code>foo ` bar</code>`foo<code>foo</code>`foo*foo`*`<em>foo<code></code></em><code></code>*foo<code>*</code><a href="`">`<a href="<code>"></code><a href="`">````foo``<pre class="language-foo``"></pre>(text lost)```foo``A backtick string should close on the next string of the same length, and otherwise stay literal. Code spans bind tighter than emphasis and links, but not tighter than HTML tags or autolinks. That's the same precedence and escape territory as #51 and #52, so I'd build this one on top of them once they're settled.
3. Escaped pipes in table cells
| a | b | |---|---| | x\|y | z |
Tempest
mainsplits the cell on the escaped pipe (<td>x\</td><td>y</td><td>z</td>). GFM keeps it as one cell,<td>x|y</td><td>z</td>(example 200). Small fix, limited toTableRule.4. Autolinks
Open <https://example.com> now.passes through unchanged, so the browser treats<https://example.com>as an unknown tag and drops it. CommonMark renders<a href="https://example.com">https://example.com</a>(§6.5), and<a@example.com>likewise as amailto:link. This one is a missing feature rather than a bug. Do you want it in core?I'm leaving out link reference definitions (§4.7): we have none in our content, and it's a much bigger change.
This is a discussion about which generally useful behavior belongs in Tempest and which should remain a downstream extension. The examples below were checked against upstream
main/ 1.2.2 with highlighting disabled.1. Make configured rules work inside nested tokens
prependRules()andremoveRules()affect the outer parser, but paragraph, heading, list, link, and table tokens callforToken()with hard-coded rule lists. A custom^rule, for example, fires for^xat document start but not fora ^x,- ^x, or## ^x. This prevents downstream syntax from being implemented as an ordinary Tempest rule throughout a document.This overlaps #13 (configurable rules/tokens) and #18 (
removeRules()inside nested contexts). The likely requirement is composable rules per context (block versus inline), rather than blindly copying block rules into every inline parser. It would be useful to know your preferred API before attempting a fix.2. Correct generic link and image parsing
[x](/a "Title")href="/a "Title""href="/a"andtitle="Title"[x](a b)href[x](<a b>)href="<a b>"a b, with appropriate URL encodingsrcThe general rule should parse destination, optional title, escapes, entities, and balanced parentheses before rendering. A renderer hook for parsed links/images would let integrations use those services without rewriting serialized
<a>and<img>tags.3. Preserve raw HTML and distinguish literal text from markup
<script>const x = "**a**";</script>currently becomes<script>const x = "<strong>a</strong>";</script>. The script body must remain raw. This is separate from #20, which asks whether raw HTML should be allowed, escaped, or stripped.Other generic inline cases include
a\*bretaining its backslash anda_b_cbecoming emphasis. Escaping plain text/entities correctly in text tokens, while keeping raw HTML and code opaque, would nice.4. Finish structural Markdown before adding CMS syntax
Examples still requiring downstream handling include
#titrebeing parsed as a heading without the required separator;2. Firstlosing its list start number;~~~fences being parsed as strikethrough; and nested/loose lists and hard line breaks. Automatic heading IDs should also be optional or render-configurable.Several narrower issues/PRs already cover parts of this: #27/#29, #28/#30, #31/#32, #33/#34, and #35/#36. I am listing them here for context. GFM task lists (
- [x] Done, currently rendered as an empty-URL link) could be an optional extension.5. Keep application behavior downstream
A small set of stable rule and rendering hooks in Tempest would make cool thing possible without pre-processing Markdown or post-processing HTML (Example).
@brendt, which of these would you want in the core parser, which as optional rules, and which should be solved through extension hooks? In particular, is the rule-configuration direction in #13/#18 the right prerequisite? I will wait for your feedback before opening any further PRs for the items in this discussion.