Review nits: deactivate the openers a nested link dooms, and pin the refusal order
CI / gate (push) Successful in 38s
CI / publish (push) Has been skipped

This commit is contained in:
2026-09-19 02:02:01 +02:00
parent 3a22ff388d
commit 6e9e38776f
4 changed files with 29 additions and 8 deletions
+1
View File
@@ -55,6 +55,7 @@ it named converts.
| Input | `0.1.0` | `0.2.0` | | Input | `0.1.0` | `0.2.0` |
| --- | --- | --- | | --- | --- | --- |
| a link whose `href` or `title` no CommonMark escape spells, on emit | `unspellable-link` | spells `!adf:link[text]{attrs}` | | a link whose `href` or `title` no CommonMark escape spells, on emit | `unspellable-link` | spells `!adf:link[text]{attrs}` |
| a link whose text already holds one (`[<https://example.com/>](/v)`) | drops the outer link | leaves the outer brackets literal text |
| a leaf node given a body (`media`, `listBreak`) | `unsupported-node-shape` | `malformed-directive` | | a leaf node given a body (`media`, `listBreak`) | `unsupported-node-shape` | `malformed-directive` |
| a node with a block body written as a leaf (`panel`) | `unsupported-node-shape` | `malformed-directive` | | a node with a block body written as a leaf (`panel`) | `unsupported-node-shape` | `malformed-directive` |
| an empty node the `::taskItem` spelling row names, written as a leaf | parses | `malformed-directive` | | an empty node the `::taskItem` spelling row names, written as a leaf | parses | `malformed-directive` |
+1 -1
View File
@@ -122,7 +122,7 @@ emit refuses:
whose text holds an autolink keeps the inner link and leaves the outer brackets literal text, whose text holds an autolink keeps the inner link and leaves the outer brackets literal text,
where the reference nests one `<a>` inside another against the spec's own prose. The first three where the reference nests one `<a>` inside another against the spec's own prose. The first three
are pinned `pending` in `corpus/commonmark-spec/exceptions.json`; the suite holds no example of are pinned `pending` in `corpus/commonmark-spec/exceptions.json`; the suite holds no example of
the fourth, which a normalization fixture pins instead. the fourth.
- Raw HTML in markdown input is an error result, never a silent drop — a tag, a comment and a - Raw HTML in markdown input is an error result, never a silent drop — a tag, a comment and a
processing instruction alike. ADF holds no raw-HTML node; the element mapping ships at `0.2.0`. processing instruction alike. ADF holds no raw-HTML node; the element mapping ships at `0.2.0`.
- Not every document converts back: `adfToMarkdown` is partial on valid ADF — a text node holding - Not every document converts back: `adfToMarkdown` is partial on valid ADF — a text node holding
+26 -7
View File
@@ -39,6 +39,8 @@ type Run = { canClose: boolean; canOpen: boolean; character: string; index: numb
// `container` is `undefined` inside a directive's content slot, the emitter's `bracketed`. // `container` is `undefined` inside a directive's content slot, the emitter's `bracketed`.
type Scan = { type Scan = {
container: LineContainer | undefined container: LineContainer | undefined
// Pieces below this have been walked for openers to deactivate, so a nest of doomed brackets stays linear.
deactivatedBefore: number
definitions: LinkDefinitions definitions: LinkDefinitions
openingSpellableLink: boolean openingSpellableLink: boolean
path: ConvertErrorPath path: ConvertErrorPath
@@ -61,7 +63,7 @@ export function parseInlineContent(source: string, definitions: LinkDefinitions,
} }
function parseInline(source: string, definitions: LinkDefinitions, path: ConvertErrorPath, container: LineContainer | undefined, spans: NestedSpans): Result<InlineContent> { function parseInline(source: string, definitions: LinkDefinitions, path: ConvertErrorPath, container: LineContainer | undefined, spans: NestedSpans): Result<InlineContent> {
const scan: Scan = { container, definitions, openingSpellableLink: false, path, pending: '', pieces: [], source, spans } const scan: Scan = { container, deactivatedBefore: 0, definitions, openingSpellableLink: false, path, pending: '', pieces: [], source, spans }
let index = 0 let index = 0
while (index < source.length) { while (index < source.length) {
switch (source.charAt(index)) { switch (source.charAt(index)) {
@@ -346,29 +348,46 @@ function resolveTarget(scan: Scan, bracket: Bracket, index: number): { definitio
return { definition, length: label?.length ?? 0 } return { definition, length: label?.length ?? 0 }
} }
// `false` keeps the brackets text: an empty link text gives the mark no node to ride, and a linked one no room for a second.
function closeLink(scan: Scan, at: number, inner: readonly Piece[], definition: LinkDefinition): Result<boolean> { function closeLink(scan: Scan, at: number, inner: readonly Piece[], definition: LinkDefinition): Result<boolean> {
if (holdsImage(inner)) return failure('unmappable-image', imageAlone, scan.path) if (holdsImage(inner)) return failure('unmappable-image', imageAlone, scan.path)
if (holdsCarry(inner)) return failure('unsupported-node-shape', carriedInMark, scan.path) if (holdsCarry(inner)) return failure('unsupported-node-shape', carriedInMark, scan.path)
const resolved = resolveNodes(inner, scan.path) const resolved = resolveNodes(inner, scan.path)
if (!resolved.ok) return resolved if (!resolved.ok) return resolved
const nodes = resolved.value const nodes = resolved.value
if (nodes.length === 0 || holdsLink(nodes)) return success(false) // An empty link text gives the mark no node to ride, so the brackets stay text.
if (nodes.length === 0) return success(false)
if (holdsLink(nodes)) {
deactivateOpeners(scan, at)
return success(false)
}
const attrs = definition.title === undefined ? { href: definition.destination } : { href: definition.destination, title: definition.title } const attrs = definition.title === undefined ? { href: definition.destination } : { href: definition.destination, title: definition.title }
scan.pieces.length = at truncatePieces(scan, at)
// CommonMark: no link nests inside another, though an image's description holds one. deactivateOpeners(scan, at)
for (const piece of scan.pieces) if (piece.kind === 'open' && !piece.image) piece.active = false
scan.pieces.push({ kind: 'nodes', nodes: applyMark(nodes, { attrs, type: 'link' }) }) scan.pieces.push({ kind: 'nodes', nodes: applyMark(nodes, { attrs, type: 'link' }) })
return success(true) return success(true)
} }
// CommonMark: no link nests inside another, though an image's description holds one.
function deactivateOpeners(scan: Scan, before: number): void {
for (let index = Math.min(scan.deactivatedBefore, before); index < before; index += 1) {
const piece = scan.pieces[index]
if (piece?.kind === 'open' && !piece.image) piece.active = false
}
scan.deactivatedBefore = before
}
function truncatePieces(scan: Scan, to: number): void {
scan.pieces.length = to
scan.deactivatedBefore = Math.min(scan.deactivatedBefore, to)
}
function closeImage(scan: Scan, at: number, inner: readonly Piece[], definition: LinkDefinition): Result<null> { function closeImage(scan: Scan, at: number, inner: readonly Piece[], definition: LinkDefinition): Result<null> {
if (definition.title !== undefined) return failure('unmappable-image', 'no media node carries a link title', scan.path) if (definition.title !== undefined) return failure('unmappable-image', 'no media node carries a link title', scan.path)
const resolved = imageAlt(inner, scan.path) const resolved = imageAlt(inner, scan.path)
if (!resolved.ok) return resolved if (!resolved.ok) return resolved
const alt = resolved.value const alt = resolved.value
const attrs = alt === '' ? { type: 'external', url: definition.destination } : { alt, type: 'external', url: definition.destination } const attrs = alt === '' ? { type: 'external', url: definition.destination } : { alt, type: 'external', url: definition.destination }
scan.pieces.length = at truncatePieces(scan, at)
scan.pieces.push({ alt, kind: 'image', node: { attrs: { layout: 'center' }, content: [{ attrs, type: 'media' }], type: 'mediaSingle' } }) scan.pieces.push({ alt, kind: 'image', node: { attrs: { layout: 'center' }, content: [{ attrs, type: 'media' }], type: 'mediaSingle' } })
return success(null) return success(null)
} }
@@ -1020,6 +1020,7 @@ test('names the link a directive link wraps, no link holding another', () => {
assert.equal(content(markdownToAdf('!adf:link[<http://x/>]{collection=c href="/u"}\n')), named) assert.equal(content(markdownToAdf('!adf:link[<http://x/>]{collection=c href="/u"}\n')), named)
assert.equal(content(markdownToAdf('!adf:link[[a](/v)]{collection=c href="/u"}\n')), named) assert.equal(content(markdownToAdf('!adf:link[[a](/v)]{collection=c href="/u"}\n')), named)
assert.equal(content(markdownToAdf('!adf:link[a <http://x/> b]{collection=c href="/u"}\n')), named) assert.equal(content(markdownToAdf('!adf:link[a <http://x/> b]{collection=c href="/u"}\n')), named)
assert.equal(content(markdownToAdf('!adf:link[<http://x/>]{href="/u"}\n')), named)
}) })
test('names the directive mark left without the content it wraps', () => { test('names the directive mark left without the content it wraps', () => {