Skip to content

feat(core/inlines): support [[[SPEC#id]]] for cross-spec section links#5146

Open
marcoscaceres wants to merge 32 commits intomainfrom
feat/inlines-cross-spec
Open

feat(core/inlines): support [[[SPEC#id]]] for cross-spec section links#5146
marcoscaceres wants to merge 32 commits intomainfrom
feat/inlines-cross-spec

Conversation

@marcoscaceres
Copy link
Copy Markdown
Contributor

Summary

  • Extends the [[[...]]] inline expansion syntax to allow [[[SPEC#fragment]]]
  • Resolves to the spec's href from the bibliography with the #fragment appended
  • Example: [[[fetch#data-fetch]]] → link to the Fetch spec's #data-fetch section, with link text taken from the spec title

Test plan

  • Run unit tests: pnpm test:unit
  • Verify [[[fetch#data-fetch]]] produces a link to https://fetch.spec.whatwg.org/#data-fetch

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Extends ReSpec’s [[[...]]] inline expansion parsing so authors can cite a spec and link directly to a section using a fragment (e.g., [[[fetch#data-fetch]]]), leveraging existing data-cite fragment handling.

Changes:

  • Updated the [[[...]]] inline-expansion regex to allow an optional #fragment suffix.
  • Added a unit test asserting that [[[fetch#data-fetch]]] resolves to the spec URL with the fragment and uses the spec title as link text.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/core/inlines.js Broadens inlineExpansion matching to include SPEC#fragment forms.
tests/spec/core/inlines-spec.js Adds coverage for cross-spec section linking via [[[SPEC#id]]].

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/inlines.js Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/spec/core/inlines-spec.js
@marcoscaceres
Copy link
Copy Markdown
Contributor Author

@sidvishnoi Good point. The spec title alone isn't useful for section-level links. The intended behavior for [[[fetch#cookie-header]]] is to display the actual heading text, like "§ 3.1.1 Cookie header (Fetch Standard)".

The data for this is available in w3c/webref's headings data. I've opened speced/respec-web-services#469 to add a /xref/headings endpoint that looks up {spec, id}{title, number, href, specTitle}.

Holding this PR until the headings API is available and the inlines.js expansion can use real heading text. Will also add alias support ([[[fetch#cookie-header|cookie handling]]]).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/data-cite.js
Comment thread src/core/data-cite.js Outdated
Comment thread src/core/data-cite.js Outdated
Comment thread src/core/data-cite.js Outdated
Comment thread tests/spec/core/inlines-spec.js
@marcoscaceres
Copy link
Copy Markdown
Contributor Author

@copilot Can you check and fix any issues identified by ci?

auto-merge was automatically disabled April 15, 2026 15:56

Head branch was pushed to by a user without write access

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 15, 2026

@copilot Can you check and fix any issues identified by ci?

Fixed in 8c873cf. The CI failures were caused by my data-lt alias handling in data-cite.js applying unconditionally to any element with data-lt, including IDL references from inline-idl-parser.js (which use data-lt as a lookup term but have <code> child content). This corrupted IDL link text — e.g., addEventListener(type, callback) was duplicated because elem.textContent = elem.dataset.lt replaced the <code> children of <a>, then the sibling <code>(type, callback)</code> was appended again.

The fix: guard with elem.textContent === "" so alias text is only applied to empty elements (triple-bracket expansions), not IDL references that already have child content.

@marcoscaceres
Copy link
Copy Markdown
Contributor Author

@copilot, can you fix any outstanding comments and resolve any that have already been addressed?

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 16, 2026

@copilot, can you fix any outstanding comments and resolve any that have already been addressed?

Done in 47181d8. Here's the status of each outstanding thread:

  • headingElements dead code — was already removed in a prior commit; no action needed.
  • Heading fetching scoping + heading text overwrite + <cite> sibling DOM shape — all three are addressed by removing the premature heading API integration (fetchHeadings, HEADINGS_API_URL, § N title text, <cite> sibling). The /xref/headings endpoint doesn't exist yet, so this is deferred to a follow-up PR when the API is ready. [[[fetch#data-fetch]]] now stably produces <a href="https://fetch.spec.whatwg.org/#data-fetch">Fetch Standard</a> with no network calls beyond biblio.

marcoscaceres and others added 15 commits April 28, 2026 11:49
toCiteDetails() may mutate elem.dataset.cite for elements with
fragment-only citations (#foo). Capture the original key before
pre-computation so error messages show what the author wrote.
…atterns

Agent-Logs-Url: https://github.com/speced/respec/sessions/b4127b19-353e-4a16-a5e8-60b46aa9ae47

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
The data-lt expansion for [[[SPEC#id]]] alias text must not overwrite
textContent when data-lt is "the-empty-string" (the WebIDL sentinel
for enum value ""). Matches the existing guard in linkElem.
- Test [[[!SPEC#id]]] normative with fragment
- Test [[[?SPEC#id]]] informative with fragment
- Test [[[?SPEC#id|alias]]] informative + fragment + alias
- Test [[[?SPEC|alias]]] informative + alias, no fragment
- Test [[[not-found|Custom Text]]] error path with alias
- Fix misleading "avoids double calls" comment in data-cite.js
Per Sid's review: moved the complex expansion syntax validation from
the regex into the handler function. The regex now matches any [[[...]]]
content. Invalid syntax gets a warning with expected formats.
Rename generic 'defined' constant to 'inlineExpansionPattern'.
Update regex comment to list all supported syntaxes.
Remove stray JSDoc block that didn't document any function.
- inlineRefMatches return type: HTMLElement | string (returns string on invalid syntax)
- Expand warning hint to include all supported forms (aliases, prefixes)
- Fix stale data-cite.js comment about toCiteDetails mutation ordering
Strip !/?/\ prefix before checking if ref starts with # so that
[[[!#id]]] and [[[?#id]]] are treated as in-document links (href)
rather than cross-spec citations (data-cite). Previously the prefix
caused the # check to fail, routing them to the wrong code path.
…e for heading (#5207)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
…sification

Agent-Logs-Url: https://github.com/speced/respec/sessions/357f987f-64f6-4773-9ca0-76c94ec3abaa

Co-authored-by: marcoscaceres <870154+marcoscaceres@users.noreply.github.com>
@marcoscaceres marcoscaceres force-pushed the feat/inlines-cross-spec branch from b98c3bc to ced1700 Compare April 28, 2026 02:01
marcoscaceres and others added 3 commits April 28, 2026 19:51
Calls POST /xref/search/headings to fetch actual section heading text
for [[[SPEC#id]]] cross-spec links. When the API returns a result, the
link text shows "§N Title" (e.g., "§4 Fetching") instead of just the
spec title. Falls back to spec title when the API is unavailable or
has no data for the given fragment. Alias text ([[[SPEC#id|text]]])
always takes precedence over heading titles.
- Merge heading collection into existing citeDetailsMap loop
- Default `result` destructuring to empty array for defensive parsing
- Guard against empty heading titles producing invisible links
- Use regex in heading test for section number flexibility
@marcoscaceres marcoscaceres requested a review from Copilot April 28, 2026 12:09
@marcoscaceres marcoscaceres marked this pull request as ready for review April 28, 2026 12:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/data-cite.js Outdated
Comment thread tests/spec/core/inlines-spec.js Outdated
- Remove API_URL import from xref.js to avoid its top-level side
  effects (preconnect link injection). Define HEADINGS_API_URL locally.
- Widen heading test assertion to accept unnumbered headings (bare
  title without section number prefix).
[[[\SPEC#id]]] now renders as literal [[[SPEC#id]]] text, matching
the escape pattern used by [[\internal-slot]] and {{\EscapedIDL}}.
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.

4 participants