Emit slug-based heading IDs for anchor link navigation by falcon-enoc · Pull Request #430 · schuyler/macdown3000

falcon-enoc · 2026-05-14T07:38:45Z

Summary

Headings now receive text-derived id attributes regardless of the "Detect TOC token" preference, enabling standard [text](#section) anchor links in the preview.
The visible [TOC] renderer uses the same slugs.
Applied to both the main preview (hoedown_html_patch.c) and the Quick Look renderer (MPQuickLookRenderer.m) for parity.

Tests

xcodebuild test -workspace "MacDown 3000.xcworkspace" -scheme MacDown -destination 'platform=macOS' — 875 / 875 passing locally.
Four new tests in MPMarkdownRenderingTests.m for slug behavior.
28 golden fixtures regenerated mechanically (uniform id="..." additions only).

Related to #429

Headings now receive text-derived id attributes so standard CommonMark/GFM anchor links like [text](#section) navigate to the corresponding heading in the preview. The visible [TOC] renderer uses the same slugs for its links, keeping them consistent with the new heading ids. Existing golden fixtures and a few inline assertions that assumed <h*> without attributes are updated. Four new tests cover slug generation (basic ASCII, UTF-8 preservation, punctuation handling, and emission independent of the TOC preference). Related to schuyler#429

MPQuickLookRenderer uses its own parallel Hoedown setup, so without this the anchor link fix would only apply to the main preview pane and not to Finder Quick Look previews of the same file. The slugify helper and header renderer are duplicated rather than shared because MacDownCore is a separate Xcode target without access to MacDown's hoedown_html_patch.c. Sharing them would require restructuring the project, which is out of scope here. Related to schuyler#429

schuyler

Thanks for the PR, @falcon-enoc!

Issues

HTML entities in slugs: because slugs are derived from the rendered HTML content, a heading like Q&A becomes id="qampa" (the literal amp leaks in), so the #qa anchor a reader would naturally write silently fails to navigate — the same happens for <, >, ", ' (e.g. Tips & Tricks → tips-amp-tricks). Since rendered content always escapes a literal & to &, skipping &…; entities the same way the loop already skips <…> tags would resolve it, for example near the top of the slugify loop:
```
if (c == '&') {            // skip an HTML entity like &amp; / &lt; / &#39;
    while (i + 1 < content->size && content->data[i + 1] != ';')
        i++;
    i++;                    // consume the ';'
    continue;
}
```
(The same change would apply to mp_quicklook_slugify.)

Suggestions

Duplicate-heading test: a test pinning current behavior for identical headings (e.g. ## C and ## C++ both yielding id="c") would document the slug contract.
TOC round-trip test: since this PR changes both sides, a test asserting a heading's id equals the matching [TOC] href would lock in the navigation guarantee.
Non-ASCII anchors: UTF-8 bytes are emitted into id/href without percent-encoding, which works in modern browsers but isn't canonical.

Generated by Claude Code

Rendered heading content escapes & < > to entities, so the slug loop received "&" and leaked "amp" into ids (e.g. "Q&A" -> id="qampa"), silently breaking anchor navigation. Skip &...; sequences the same way the loop already skips <...> tags, in both slugify and mp_quicklook_slugify. Related to schuyler#430

Adds cases for &/</multiple entities in both the preview and Quick Look renderers, pins the duplicate-heading collapse contract, and locks in TOC href == heading id navigation. Related to schuyler#430

falcon-enoc · 2026-06-23T08:16:35Z

Thanks for the thorough review, @schuyler! All points addressed.

HTML entities in slugs (Issue) — Fixed. The slug loop now skips &…; entities the same way it already skipped <…> tags, in both slugify (hoedown_html_patch.c) and mp_quicklook_slugify (MPQuickLookRenderer.m). Now ## Q&A → id="qa", ## Tips & Tricks → tips-tricks, ## A < B → a-b.

Duplicate-heading test (Suggestion 1) — Added testDuplicateHeadingsCollapseToSameId pinning that ## C and ## C++ both collapse to id="c".

TOC round-trip test (Suggestion 2) — Added testTOCHrefMatchesHeadingId asserting the [TOC] href equals the heading id for the same text, locking in the navigation guarantee.

Non-ASCII anchors (Suggestion 3) — Left UTF-8 as-is on purpose: browsers match the URL fragment against the id literally (they don't percent-decode the id for matching), so encoding id="café" to id="caf%C3%A9" would break navigation rather than fix it. Documented the rationale in the slugify doc comments. Happy to open a separate issue to explore canonical encoding if useful.

Also added 3 analogous tests for the Quick Look renderer to keep both code paths in parity.

All tests green: 86 passed, 0 failures.

schuyler

Thanks for the PR, @falcon-enoc — and for the thorough write-up in #429!

This is a clean, well-targeted fix: emitting text-derived IDs unconditionally is the right call, the TOC href and heading ID staying in sync is a nice touch, and the new tests around HTML-entity handling cover a genuinely tricky path. Approving.

A handful of minor, non-blocking polish ideas came up during review (a few extra edge-case tests, a now-unused TOC counter, and a comment tweak) — none need to hold this up, so we've logged them on our side in #471 rather than expand the scope here. Much appreciated!

schuyler · 2026-06-24T05:30:43Z

Thanks again for this, @falcon-enoc! The one red check here isn't a problem with your code — it's a branch-staleness collision, and the fix is mechanical.

What's happening: the only failing test is MPMarkdownRenderingTests testTables. This branch is based on main at 75c18c3, and since then #440 (c34f75f, "restore natural table width and horizontal scrolling") added a new "Wide Table (many columns, issue #432 regression)" section to MacDownTests/Fixtures/tables.md and tables.html. CI runs against your branch merged into current main, so the rendered tables.md now includes that new heading — and your header callback (correctly!) gives it a slug id, while the merged tables.html fixture still has it as a plain <h2>. The exact-match golden comparison then fails. It passes locally for you because your branch predates #440 and never had that section.

The fix: merge (or rebase onto) the latest main and regenerate the golden fixtures so the new heading picks up its id. Concretely, tables.html just needs:

- <h2>Wide Table (many columns, issue #432 regression)</h2>
+ <h2 id="wide-table-many-columns-issue-432-regression">Wide Table (many columns, issue #432 regression)</h2>

Regenerating via the suite's REGENERATE_GOLDEN_FILES path will produce that automatically, and it's worth a quick re-run afterward in case main has added headings to any other fixtures since you branched. No changes to the slug logic itself are needed — that part's working exactly as intended.

(The Run Unit Tests jobs also log a 403 from the coverage-comment step — that's just the read-only token on fork PRs and is already marked continue-on-error, so you can safely ignore it.)

Generated by Claude Code

Regenerate tables.html golden so the "Wide Table" heading (added in schuyler#440) carries the slug id emitted by the anchor-id logic, keeping the exact-match rendering test green against current main.

falcon-enoc · 2026-06-24T08:26:05Z

Thanks, @schuyler — your diagnosis was spot on.

I rebased onto the latest main (picks up #440 and everything through #473) and regenerated the golden fixtures via the suite's REGENERATE_GOLDEN_FILES path. As you predicted, only tables.html needed updating, with exactly the one-line change:

- <h2>Wide Table (many columns, issue #432 regression)</h2>
+ <h2 id="wide-table-many-columns-issue-432-regression">Wide Table (many columns, issue #432 regression)</h2>

No other fixtures picked up new headings from main, so the slug logic itself needed no changes. The full suite is green locally.

Pushed in 9b0c13b.

schuyler · 2026-06-24T17:23:46Z

@falcon-enoc Thanks for your contribution and especially thanks for your patience with all the back and forth. Branch merged!

PR #430 copied the slug/heading-id logic into the Quick Look renderer (mp_quicklook_render_header), including the same hoedown_buffer_new( content->size) call. An empty heading (e.g. a bare setext underline) therefore creates a zero-unit buffer that aborts the Quick Look render process the first time it is written to — the same defect just fixed in hoedown_html_patch.c, reachable by previewing a .md file in Finder. Clamp the size hint with a mp_quicklook_new_growable_buffer() helper mirroring new_growable_buffer(), and add a regression test rendering a bare '-' heading through the Quick Look renderer. Related to #479

falcon-enoc added 2 commits May 14, 2026 03:32

schuyler requested changes Jun 22, 2026

View reviewed changes

falcon-enoc added 2 commits June 23, 2026 04:12

test(anchor): cover HTML entity skipping and TOC round-trip

0848f6c

Adds cases for &/</multiple entities in both the preview and Quick Look renderers, pins the duplicate-heading collapse contract, and locks in TOC href == heading id navigation. Related to schuyler#430

schuyler mentioned this pull request Jun 24, 2026

Follow-ups from PR #430: heading slug anchor IDs (polish, edge-case tests, dedup decisions) #471

Open

18 tasks

schuyler approved these changes Jun 24, 2026

View reviewed changes

falcon-enoc added 2 commits June 24, 2026 04:03

Merge remote-tracking branch 'upstream/main' into fix/heading-anchor-ids

50b0278

test(fixtures): sync wide table heading id in golden html

9b0c13b

Regenerate tables.html golden so the "Wide Table" heading (added in schuyler#440) carries the slug id emitted by the anchor-id logic, keeping the exact-match rendering test green against current main.

schuyler merged commit 9284342 into schuyler:main Jun 24, 2026
5 checks passed

schuyler mentioned this pull request Jun 30, 2026

0.7-rc.1 crashes on typing hyphen #479

Open

schuyler mentioned this pull request Jun 30, 2026

Address issue #479: fix crash on empty Markdown headings #482

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emit slug-based heading IDs for anchor link navigation#430

Emit slug-based heading IDs for anchor link navigation#430
schuyler merged 6 commits into
schuyler:mainfrom
falcon-enoc:fix/heading-anchor-ids

falcon-enoc commented May 14, 2026

Uh oh!

schuyler left a comment

Uh oh!

falcon-enoc commented Jun 23, 2026

Uh oh!

schuyler left a comment •

edited

Loading

Uh oh!

schuyler commented Jun 24, 2026

Uh oh!

falcon-enoc commented Jun 24, 2026

Uh oh!

Uh oh!

schuyler commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

falcon-enoc commented May 14, 2026

Summary

Tests

Uh oh!

schuyler left a comment

Choose a reason for hiding this comment

Issues

Suggestions

Uh oh!

falcon-enoc commented Jun 23, 2026

Uh oh!

schuyler left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schuyler commented Jun 24, 2026

Uh oh!

falcon-enoc commented Jun 24, 2026

Uh oh!

Uh oh!

schuyler commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

schuyler left a comment •

edited

Loading