Skip to content

fix: keep underscores in identifier heading anchors - #400

Open
marc0olo wants to merge 1 commit into
mainfrom
infra/anchor-slug-code-spans
Open

marc0olo wants to merge 1 commit into
mainfrom
infra/anchor-slug-code-spans

Conversation

@marc0olo

@marc0olo marc0olo commented Sep 18, 2026

Copy link
Copy Markdown
Member

Summary

npm run validate fails on main for a correct link: the slug library mis-slugged any identifier heading with two or more underscores (flexible_http_request became flexiblehttprequest, canister_inspect_message likewise).

  • renderedText parses the heading with mdast-util-from-markdown and mdast-util-to-string, replacing the regexes that approximated inline markdown
  • It parses the whole line, # markers included, so 1. Create a target canister keeps its number
  • Both packages are declared in devDependencies; the lockfile root is hand-edited rather than regenerated

Verified against a built site: every heading id in dist/ matches the library across all 200 pages, both directions. A footnote reference in a heading stays out of reach, noted in the code.

@marc0olo
marc0olo requested a review from a team as a code owner September 18, 2026 12:04
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown

🤖 Here's your preview: https://keluh-vqaaa-aaaam-ai7wa-cai.icp0.io

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The regex handling still diverges from CommonMark for nested underscores and multi-backtick code spans.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates anchor validation to better match Starlight-generated heading IDs, especially for identifiers containing underscores.

Changes:

  • Separates asterisk and underscore emphasis handling.
  • Protects inline code and unwraps links before slug generation.
  • Updates shared anchor logic used by validation and static-site synchronization.

Verified: Reviewed the changed helper, its callers, and the configured Markdown pipeline.

File summaries
File Description
scripts/lib/anchors.mjs Revised rendered-heading text and slug extraction logic.
Review details

Suppressed comments (1)

scripts/lib/anchors.mjs:31

  • This split recognizes only code spans delimited by exactly one backtick. CommonMark permits matching runs of multiple backticks, so a heading with a two-backtick code span around _literal_ keeps those underscores literal in the rendered heading, while this code exposes them to UNDERSCORE_EMPHASIS and computes a different anchor. Use a delimiter-run-aware code-span scanner or the Markdown AST before applying the emphasis rules.
    .split(/(`[^`]*`)/g)
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/lib/anchors.mjs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The parser does not enable the site's GFM extensions, so some supported headings can receive different anchor IDs.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 2/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread scripts/lib/anchors.mjs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Script-only mdast packages should follow the existing convention and be moved to devDependencies.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 2/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread package.json Outdated
Identifier headings with two or more underscores were mis-slugged, which failed validate on a correct link.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation addresses the reported anchor mismatch and aligns with existing dependency and validation conventions.

Review details
  • Files reviewed: 2/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@marc0olo
marc0olo marked this pull request as ready for review September 18, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants