Fix list, table, code, and link rendering on rich pages - #17
Open
jom wants to merge 5 commits into
Open
Conversation
phpunit.xml references a bootstrap.php that wasn't tracked; without it the test suite refuses to start. Provide a minimal one that loads the renderer and the wp_html_to_markdown() entry point. Also ignore the phpunit result cache so it doesn't show up in git status. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both element types previously had no dedicated container in the renderer: <li> relied on the parent <ol>/<ul> as an implicit item collector, and <table>'s handlers just emitted inline pipe characters into the line buffer. Both approaches assume inline-only content and collapse when the source HTML puts block-level children inside (paragraphs, code blocks, nested lists, etc.). On the WordPress.org developer reference pages this produced two visible defects: user-contributed notes flattened into one long numbered list, and related-function tables emitted as malformed pipe-prefixed paragraphs. Introduce four new block classes: - Block_ListItem groups all of a single <li>'s block-level children under one list item. A sub-list directly following its preceding sibling renders without a blank line; other sibling combinations get one. - Block_Table tracks rows and the <thead> boundary so the GFM header separator can be emitted in the right place. If no <thead> is present but the first row is all <th>, that row is treated as the header. Tables with no header at all get an empty header prepended so the result still parses as a GFM table. - Block_Table_Row renders a single |-delimited row. - Block_Table_Cell collects inline content (and absorbs any <p> children encountered inside the cell), escapes pipes, collapses internal newlines, and strips the invisible nobr markers links carry through Line_Buffer::flush. The renderer's LI handler now pushes a ListItem on open and flushes it on close; the OL/UL close handler flushes any still-open implicit ListItem before flushing the list. The TABLE handler pushes a Table on open and flushes any implicit cell/row/table on close. THEAD/TBODY/TFOOT/TR/TD/TH each route through the new block types with close_a_paragraph called first so a still-open Paragraph (from a cell's inline text) flushes into the cell before the cell itself is flushed. Also add a strip_nobr_markers() helper next to line_wrap() — line_wrap already strips the U+E0001/U+E007F nobr markers it consumes, but consumers that bypass it (table cells here, headings in the next commit) need to do their own stripping. Block_List's flush now skips the continuation prefix on empty lines so a list item containing two paragraphs no longer has trailing whitespace on the blank separator line. Existing fixtures in unordered-lists-1, ordered-lists-1, and table-1 update to match the new (semantically correct) output: each <li> is rendered as its own list item even when its first child is a sub-list, and tables emit the |---|---| header separator GFM expects. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Block_Code unconditionally pushed a 4-space indent onto options->indent
before rendering, which produced fenced code blocks like:
```php
echo 'hi';
```
A 4-space indent in front of a fence makes GFM parsers treat the fence as
content of an indented code block, so the backticks render literally. The
issue was even more obvious inside a list item: the list's own continuation
indent (4 spaces) stacked with the code block's self-indent (4 more) to
produce 8-space-prefixed code that no longer aligned to the list item.
Drop the self-indent. Fenced code now renders at column 0 at the top level
and inherits whatever options->indent the surrounding container has set up
(e.g. a blockquote prefix), which is the same as paragraphs.
Existing fixtures in basic-1.html (basic pre, p pre p pre, pre code, and
the erlang language-class case) drop the leading 4 spaces from each line of
the expected code-block output.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Format_Link wraps every rendered link with U+E0001 / U+E007F (Unicode Tags block) so line_wrap() can identify a non-breakable region and avoid wrapping mid-link. line_wrap() consumes the markers and strips them in passing, so paragraphs come out clean. But Block_ATX calls Line_Buffer::flush() directly without going through line_wrap, so the markers leaked through into every heading that contained a link. The markers are invisible in editors and browsers but are real bytes inside the cell or heading. They confuse downstream consumers — markdown parsers that don't recognise them, LLMs reading the output (the Tags block is a known prompt-injection vector), copy-paste into other systems, etc. Pipe heading text through strip_nobr_markers() before emitting. (Table cells already gained the same call in the previous commit.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The ternary checked whether the child was a Block or a Line_Buffer and called $child->flush( $options ) in both branches — both classes expose the same flush( Options ) signature, so the branch was dead weight. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
Hey @dmsnell! I was wondering if you've had a chance to check out this PR. It fixes a few things on WordPress.org's developer.wordpress.org. I also put up WordPress/wporg-developer#565 to hide a few elements that don't seem very useful to LLMs. |
Owner
|
thanks for the poke @jom — I missed this entirely so I’ll try and look at it more closely in the next few days. pretty over-booked at the moment so I can’t promise anything; but thank you! |
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
2 participants
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.
Why
The markdown produced for rich pages like https://developer.wordpress.org/reference/functions/add_role/?output_format=md had several visible defects that traced back to the renderer assuming structural elements contain only inline content. Each block-level child inside an
<li>or<td>dissolved the boundary because there was no container to anchor the children to.What changed
<li>and<table>now route through real block containers. Previously<li>had no container — the parent<ol>/<ul>was its implicit item collector — so each<p>,<pre>, or nested list inside a single<li>became its own top-level item. A 10-comment "User Contributed Notes" section rendered as items1.through82.. Tables had the same shape of bug:<table>had no block, and its handlers wrote pipe characters directly into the line buffer. Any<td>containing a<p>exploded the row mid-line, and no|---|---|separator was ever emitted, so even simple tables didn't parse as GFM tables.Four new block classes —
Block_ListItem,Block_Table,Block_Table_Row,Block_Table_Cell— own their children explicitly. Each<li>becomes one list item with its block children stacked underneath; a sub-list directly following its preceding sibling renders without a blank line, other sibling combinations get one. Tables detect the header from<thead>or, failing that, from a leading all-<th>row, and emit the GFM separator row in the right place. Tables with no header at all get an empty header prepended so the result still parses. Cells flatten any inline paragraph children into a single line, collapse embedded newlines, and escape pipes.Fenced code blocks no longer self-indent.
Block_Codewas unconditionally pushing 4 spaces ontooptions->indent, so even a top-level code block came out 4-space-indented. 4-space indent in front of a fence makes GFM treat the whole thing as an indented code block whose content is literal backticks. The problem doubled inside list items (4 spaces from list continuation + 4 from code self-indent = 8 spaces, no longer aligned with the item content). Fences now sit at column 0 at the top level and inheritoptions->indentfrom the surrounding container — the same as paragraphs.Invisible nobr markers no longer leak into headings or table cells.
Format_Linkwraps every link with U+E0001 / U+E007F (Unicode Tags block) soline_wrap()can keep links un-wrapped.line_wrap()consumes and strips the markers — butBlock_ATXand the newBlock_Table_CellcallLine_Buffer::flush()directly without wrapping, so the markers leaked through into every heading and table cell that contained a link. They're invisible in editors and browsers but real bytes — they confuse downstream markdown parsers and are a known prompt-injection vector when handed to an LLM. A newstrip_nobr_markers()helper sits next toline_wrap()for the two consumers that bypass it.Phpunit bootstrap added.
phpunit.xmlreferenced abootstrap.phpthat wasn't tracked, sovendor/bin/phpunitrefused to start. Added a minimal one that loads the renderer.Existing fixtures updated
Three existing test fixtures' expectations change to match the new (semantically correct) output:
unordered-lists-1.htmlandordered-lists-1.html: each<li>is now a proper list item even when its first child is a sub-list. The previous expected output dropped the bullet for the second sibling so the sub-list looked like a continuation of the first, which was a misrepresentation of the HTML.table-1.html: now expects the GFM|---|---|header separator row.basic-1.html(four sections): code-block expectations drop the leading 4-space prefix.Out of scope (noticed but not addressed here)
`\$role`stringrequired). The renderer already adds a space when there's any whitespace between tags; the WordPress.org parameter list happens to emit them with none. I put up a fix for this in Trim browser-only chrome from ?output_format=md requests WordPress/wporg-developer#565.<span class="array”>showing as text) — invalid source HTML breaking attribute parsing; recovery emits the malformed run. I think this is an issue in the actual doc source (see add-role's\$capabilitiesdoc).Tested
vendor/bin/phpunit— 74/74 pass (PHP 8.5; spec passes on 8.3+).developer.wordpress.org(sandbox-deployed): user-contributed notes now break into separate items, related/changelog tables render as proper GFM, code samples align to the list-item content column, headings and cells are free of invisible markers.🤖 Generated with Claude Code