feat: read and write GitHub footnotes, with definitions as anchor constructs - #69
Merged
Conversation
4.2.0 adds construct boundary markers -- a matched constructStart/constructEnd pair -- to ContentBlock, which is the flat form's encoding of the construct groups the package tree already carried. Until now the whole construct descriptor vocabulary was reachable only from DocumentPackage, so a codec producing a flat ContentDocument had no path to it at all. The addition is schema-additive but TS-breaking in two ways this package feels: an exhaustive switch over ContentBlock gains two arms, and SectionChild/ShapeChild/ListChild narrow from ContentBlock to PackageBlockLeaf.
`[^label]: body` now opens a container block whose body is ordinary block content, continued by four columns of indentation exactly as a list item's is, so a multi-paragraph note parses as several blocks rather than one run of prose. `[^label]` in running text becomes a reference node, but only when the document also carries a definition under that exact label -- an unmatched label stays ordinary text, which keeps a bracketed aside from becoming a note pointing at nothing. The label grammar lives in src/inline/footnote.ts, shared by both phases the way src/inline/link.ts already shares the link-label grammar: the grammar is inline-shaped, but when a definition is recognised is a block-structure question. Labels match exactly, with no case folding -- footnotes sit outside both CommonMark and the GFM spec document, so a normalisation rule would be this package's own invention. Definitions are recognised only as direct children of the document, and never interrupt a paragraph. Both restrictions are about what the mapping downstream can represent: a definition inside a list item or block quote would put a construct's extent inside a scope the enclosing container had already opened. Recognition is behind `footnotes`, defaulting on alongside the four GFM toggles; the CommonMark conformance suite switches it off with them.
…rence to a marked run
A definition now becomes a constructStart carrying
{ kind: 'anchor', anchorType: 'footnote', name }, its lowered body blocks,
and a constructEnd. The body rides the construct's own extent rather than
AnchorDescriptor.definition: that field names a key in a package-level
definitions table, and a flat ContentDocument -- the only shape any codec
in this family produces -- has no root to carry one. A body is genuinely
block content that a string field could not have held either way, and the
descriptor's own note says a ranged anchor wraps the blocks it spans. A
bodyless `[^1]:` lowers to a pair with nothing between it, the point
anchor that note describes.
A reference site cannot become an anchor: a construct's extent is
block-scoped by the schema's own definition, and a reference sits between
two runs inside a paragraph, which no block-level boundary marker can
bracket without splitting the paragraph in two. The schema parks that case
on a run-level extent mechanism it has not shipped, so the reference is
carried as a run keeping its own `[^label]` spelling plus
FOOTNOTE_REFERENCE_FONT_MARKER, and md/footnote-reference-preserved-as-text
reports the gap. The marker is load-bearing rather than cosmetic: text
escaping turns a deliberately-escaped literal `\[^1\]` into the same run
text as a genuine reference, and only a non-pattern-based marker separates
them.
A heading inside a definition body is flattened to literal ATX text
(md/footnote-body-heading-flattened): a marker pair's extent may neither
close a heading scope opened outside it nor leave one standing at the
closing marker, and the schema states a producer must never emit such a
pair.
The writer now recovers a block list's marker pairs as a tree before rendering anything, since the flat form encodes a construct as a matched pair bracketing the blocks it spans. An anchor construct's footnote arm renders as `[^label]: body`, every body line after the first indented to the four-column continuation the reader measures against. A run carrying FOOTNOTE_REFERENCE_FONT_MARKER renders its `[^label]` text unescaped, so a reference reparses as a reference while an ordinary run that merely looks like one still escapes to literal text. A construct kind markdown has no syntax for renders transparently rather than being dropped: its extent still appears in place and only the construct's own identity is lost, because a ContentDocument arriving from another codec carries real content inside markers markdown cannot spell, and dropping the wrapper's content with the wrapper would lose the document. Balance is checked up front through document-schema.js's own findConstructMarkerImbalance -- the one shared definition of that contract -- and an unbalanced list throws MarkdownUnbalancedConstructMarkersError rather than being repaired: with the pairing broken there is no extent to render and no correct guess about which blocks the producer meant. The two marker kinds are excluded from renderTopLevelBlock's own input type rather than handled as unreachable switch arms, so the compiler is what guarantees a marker never reaches content rendering.
One file across all four stages rather than four scattered additions: the point of the feature is that a footnote's two halves are carried by two different mechanisms and still have to reproduce each other, which no single-stage test can show. The round-trip assertion is that read -> write -> read -> write reaches a fixed point, not that write reproduces the source byte for byte. That is not a weaker bar chosen for convenience: this package normalises freely on the way out (escaping ASCII punctuation, regenerating code fences, picking its own bullet glyph), so byte equality with arbitrary source text is not a property writeMarkdown has for any construct. Adds the four new diagnostic codes to the reachability sweep, and a footnote round trip to the workerd suite, since the definition path now runs through document-schema.js's marker helpers.
Records why a definition and a reference land on different mechanisms -- one is a block-scoped anchor construct, the other cannot be one until the schema grows a run-level extent -- alongside the new diagnostic codes, the `footnotes` option, and the write-side balance check.
…p-level list continueBlock reports a `list` node as continued unconditionally regardless of what the current line is, so a `[^label]:` line right after a top-level list's last item sees that LIST as its matched container in step 1, not the document -- even though the list is one line away from closing. The block start's own guard rejected anything other than a literal `document` container, so this exact case fell through to an ordinary paragraph. That paragraph then went through extractDefinitions, which reads `[label]: dest` as a link reference definition regardless of the leading `^`, silently turning a footnote definition into a hyperlink target and, for the common case of a single-line body, discarding the whole definition (a paragraph holding nothing but a reference definition leaves no block behind). tryFootnoteDefinitionStart's guard now walks up through any `list` ancestors before checking for the document, mirroring what addChild already does once a definition is actually opened: it finalises whatever the tip can't contain, closing a bare top-level list exactly like any other block start does. A list nested inside a block quote or a list item still walks up to that container rather than the document, so the existing restriction against defining a footnote inside either continues to hold.
… as something else AnchorDescriptorSchema.name is a bare z.string() with no grammar constraint of its own, so it can carry whitespace or a "]" when it arrives from a producer other than this package's own reader (ooxml.js, odf.js, or any other codec sharing the ContentDocument pivot). renderFootnoteDefinition spelled that name straight into a "[^name]:" marker with no check against the [^label] grammar src/inline/footnote.ts already defines, so such a name emitted markdown this package's own reader cannot parse back as a footnote: whitespace reopens the "[^ see above]" ambiguity, and a "]" ends the marker early. Depending on the body, reading the result back either drops the whole document (the malformed marker line reparses as a link reference definition and eats everything after it) or degrades the definition to a literal paragraph, in both cases with no diagnostic. isValidFootnoteLabel in src/inline/footnote.ts tests a candidate name against the same grammar, and renderConstruct now falls back to the transparent unrepresented-construct degrade -- render the extent, report CONSTRUCT_UNREPRESENTED -- for a footnote anchor whose name fails it, the same degrade already used for a construct kind markdown has no syntax for at all.
…hor name Adds the exact text -> list -> definition shape that previously fell through tryFootnoteDefinitionStart's guard, plus its own round-trip source, and a matching case that confirms a definition indented into a list item's content is still correctly refused. Also covers a footnote anchor whose name contains whitespace or a "]": both now degrade transparently instead of emitting markdown that reparses into a different document entirely.
Contributor
|
🎉 This PR is included in version 3.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
1 participant
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.
Implements the footnote row of #66, now that document-schema.js 4.2.0 has put the construct descriptor vocabulary within reach of a flat
ContentDocument. Reader and writer both, with round-trip tests.Before merging: close #68 first. It is the sibling-automation bump to document-schema.js 4.2.0, which this branch subsumes (first commit here does the same bump). Merging this while that one is open has bitten before — the heal job auto-merges the now-stale bump PR on the merge push and downgrades the dependency back.
Why the earlier attempt stopped, and what changed
The last run at this reported (in a comment on #66) that every 4.1.0 addition — the six construct descriptors and the
definitions/layers/attachments/destinationstables — was wired exclusively intoDocumentPackage/PackageNode, withContentDocumentgaining nothing, so a codec that produces a flat document had no path to any of it. That was correct and it was the right call to stop.4.2.0 closes exactly that gap for the block-scoped half:
ContentBlockgained a matchedconstructStart/constructEndpair, the flat form's encoding of the construct group the package tree already had. That is enough for a footnote definition. It is not enough for a footnote reference, and the rest of this description is mostly about that split.Reading
AnchorDescriptorbefore designing the mappingdefinitionisz.string().optional(), and its comment is unambiguous about what the string is: "the definitions-table key holding this marker's body". So it is a key into a package-level definitions table, not the body itself — andDefinitionsTableSchemahangs offDocumentPackageSchema's root, not offContentDocumentSchema, which has no root fields beyondkind/metadata/symbolTable/sections. There is no table for a flat document to key into, sodefinitionstays absent.That is not a workaround for the field being too small. A footnote body is genuinely block content — several paragraphs, a code block, a list — which a
stringcould not have carried whichever table it pointed at. The construct's own extent is the right home, andAnchorDescriptorSchema's own note says so directly: "A point anchor — a footnote reference, a bookmark with no range — is a group with no children; a ranged anchor — a docx bookmark pair, a comment extent — wraps the blocks it spans."So a definition lowers to:
and a bodyless
[^1]:lowers to that same pair with nothing between it — the point anchor the note describes, not a special case. A consumer that later factors these documents into a package is free to move the body into a definitions entry and populatedefinitionthen; nothing here has to be undone for that.The reference site is a real, still-open schema gap
A reference is not a second anchor instance, and this is the one part I want a second opinion on.
src/construct.tsstates the extent scope once and binds every kind to it: "a construct group wraps BLOCK-scoped extents… It does not wrap a sub-sequence of one paragraph's runs, because a run-level extent is not expressible without changing ContentParagraph's own shape". And it names this exact case as deferred: "the inline field/bookmark/tracked-change cases wait on a run-level extent mechanism rather than being forced into a block wrapper that would split the paragraph they sit inside."A footnote reference sits between two runs inside a paragraph. Bracketing it with block-level markers means splitting the paragraph in two, which loses the paragraph — so I did not do it.
ContentRunhas no field for it either: nostyleId, no anchor, andhyperlinkis documented as a resolved external URI (an internal target is whatLinkTarget'sinternalarm exists for, and that is unreachable from a run).What ships instead is the same degrade this package already uses twice: the reference becomes a
ContentRunkeeping its own[^1]spelling, marked withFOOTNOTE_REFERENCE_FONT_MARKER, andmd/footnote-reference-preserved-as-textreports it. The marker is load-bearing rather than decorative —escapeMarkdownTextescapes[,^, and], so a genuine reference and a literal\[^1\]an author escaped on purpose are the same run text by the time they reach the writer, and only a non-pattern-based marker separates them. (This is the identical trap that made the pattern-based approach to inline math get reverted; the constant's own comment records it.)fontFamilycarrying a semantic fact is not great and I would rather not be doing it. The value is Word's own character-style name for exactly this run (FootnoteReference) rather than an invented sentinel, so a consumer resolving it as a font finds none and falls back — visible, not wrong. The right fix is a run-level extent mechanism in document-schema.js, and this stays a marked run until there is one. That gap is unchanged by this PR and I have not papered over it: it has a named diagnostic, a README section, and a note on #66.The marker contract constrains where definitions can live
ContentBlockSchema's comment states that balance is necessary but not sufficient: an extent must also not cross a heading- or list-group scope boundary, decompose is the sole enforcement point, and a producer "must never open one inside a heading or list scope that some other block inside the extent then closes." Two consequences here, both deliberate:ContentListMembershipon every block, and a body table or image cannot carry one at all — closing the item's list scope from inside the pair. A block quote is the same shape one level along (its>prefix is recovered per-paragraph fromindentLeftPt, and a label line has no paragraph to carry it). In either container the text stays an ordinary paragraph, exactly as before.md/footnote-body-heading-flattened). A heading inside the extent both closes whatever heading scope was open outside and leaves one standing at the closing marker. Deliberately unconditional rather than "only when a shallower heading is actually open outside": a level comparison would make one footnote's fidelity depend on which heading happens to precede it, so the same body would lower two different ways in two documents.Round trips either way —
#is escaped on the way out and unescaped identically on the way back.Writer
emitBlocksnow recovers a block list's marker pairs as a tree before rendering. Balance is validated through document-schema.js's ownfindConstructMarkerImbalance(the shared definition every codec anddecomposemust agree on) and an unbalanced list throwsMarkdownUnbalancedConstructMarkersErrorrather than being repaired — with the pairing broken there is no extent to render and no correct guess about which blocks were meant.A construct kind markdown has no syntax for renders transparently: its extent still appears in place, only the construct's identity is lost. Dropping the extent with the wrapper would lose real content arriving from another codec (an odt division, a docx content control, a tracked-change wrapper).
renderTopLevelBlocktakesExclude<ContentBlock, ContentConstructStart | ContentConstructEnd>rather than carrying two unreachable switch arms, so the compiler is what guarantees a marker never reaches content rendering.Explicitly out of scope
Both of these are named so the gap stays visible rather than getting quietly counted as done:
sourcefacility spanning the whole content model, not a construct, andsrc/construct.tssays outright that it deliberately does not define one. Untouched here;md/raw-html-preserved-as-textbehaves exactly as before.md/front-matter-key-unmappedis unchanged. It needs the same package-level residue channel.Reference-link definitions are also still unmapped, for the same definitions-table-is-package-only reason that governs
AnchorDescriptor.definitionabove.Verification
pnpm lint,pnpm typecheck,pnpm test(982),pnpm test:workers(3, incl. a new footnote round trip under workerd),pnpm test:smoke(18),pnpm build— all green. Conformance is unchanged: neither vendored corpus contains a[^anywhere, and the CommonMark suite switchesfootnotesoff with the four GFM toggles, so the exclusion list neither grew nor shrank.Related to #66 and #63. Not closing either — footnotes is one row of #66's checklist.
🤖 Generated with Claude Code