From a09b80e65de83f1e8ca403cbb0621fe8c5d55e54 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Sun, 30 Aug 2026 19:50:41 +0200 Subject: [PATCH] Answer the architecture review: one interrupt rule both directions read --- AGENTS.md | 4 ++- spec/flavour.md | 6 ++-- src/markdown/commonmark-grammar.ts | 6 ++++ src/markdown/emit/adf-to-markdown.test.ts | 4 +++ src/markdown/emit/adf-to-markdown.ts | 8 +++-- src/markdown/parse/blocks.ts | 20 ++++++++++-- src/markdown/parse/markdown-to-adf.test.ts | 1 + src/markdown/parse/markdown-to-adf.ts | 38 +++++++++++++++------- src/nesting.ts | 1 + 9 files changed, 67 insertions(+), 21 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4f0394a..3527e67 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -142,7 +142,9 @@ live Atlassian APIs; property-generated ADF trees; the CommonMark spec suite aga - `src/adf/` holds ADF's own knowledge and imports no format. Each format directory (`markdown/`, `html/`) parts into `emit/` (ADF→format) and `parse/` (format→ADF), its root holding what both directions read. A construct's reader lives in that root beside the regex the emitter escapes - against, so the two cannot drift; a reader with no emit counterpart goes in `parse/`. + against, so the two cannot drift; a reader with no emit counterpart goes in `parse/`. A rule both + directions must answer alike — whether a list marker interrupts a paragraph — is one function + there too, never a copy per direction, however conservative the copy would be. - The attribute vocabulary is ADF's: `adf/` walks it and narrows each value to its kind, and a format spells the narrowed value. A spelling that re-checks the type is the check's second copy. - Explicit over implicit; descriptive names; no catch-all files (`utils`, `helpers`, `misc`); a diff --git a/spec/flavour.md b/spec/flavour.md index 4bac422..839c3aa 100644 --- a/spec/flavour.md +++ b/spec/flavour.md @@ -19,9 +19,9 @@ normalizes to it through the round-trip. - Bullet lists `- `; ordered lists incrementing `1.` `2.` `3.`, the first number the node's `order` attribute. Continuation lines align with the first character after the marker (two spaces for `- `, three for `1. `); blank lines inside an item are empty lines, none - between a nested list and a CommonMark block above it — one where the nested list's own marker - cannot interrupt a paragraph (any ordered list, or a bullet list whose first item opens with an - empty block), which the block above would otherwise swallow. Blank lines between items normalize + between a nested list and a CommonMark block above it — one wherever the nested list's own + marker cannot interrupt a paragraph (an ordered list whose first number is not 1, or a list + whose first item is empty), whatever block sits above it. Blank lines between items normalize away; ADF does not record tightness. - Blockquotes prefix lines with `> `; a blank line inside a blockquote is a bare `>`. - ATX headings (`#` … `######`); setext input normalizes to ATX. diff --git a/src/markdown/commonmark-grammar.ts b/src/markdown/commonmark-grammar.ts index 619c05c..b8cad49 100644 --- a/src/markdown/commonmark-grammar.ts +++ b/src/markdown/commonmark-grammar.ts @@ -86,6 +86,12 @@ export function isUnicodeWhitespace(character: string): boolean { return unicodeWhitespace.test(character) } +// The first marker of a list, `undefined` for a bullet: one answer both directions read, or the emitter +// spells a list the parser folds into the paragraph above it. +export function markerInterruptsParagraph(start: number | undefined, empty: boolean): boolean { + return !empty && (start === undefined || start === 1) +} + export function openingCodeFence(line: string): { info: string; marker: string } | undefined { const marker = codeFenceOpener.exec(line)?.[1] if (marker === undefined) return undefined diff --git a/src/markdown/emit/adf-to-markdown.test.ts b/src/markdown/emit/adf-to-markdown.test.ts index 2e94731..6647675 100644 --- a/src/markdown/emit/adf-to-markdown.test.ts +++ b/src/markdown/emit/adf-to-markdown.test.ts @@ -267,6 +267,7 @@ test('parts a nested list the tight spelling would swallow from the block above const outer = (...content: AdfNode[]): AdfDocument => document({ content: [item(...content)], type: 'bulletList' }) const ordered: AdfNode = { attrs: { order: 2 }, content: [item(text('b'))], type: 'orderedList' } assert.equal(markdown(adfToMarkdown(outer(text('a'), ordered))), '- a\n\n 2. b\n') + assert.equal(markdown(adfToMarkdown(outer(text('a'), { ...ordered, attrs: { order: 1 } }))), '- a\n 1. b\n') assert.equal(markdown(adfToMarkdown(outer(text('a'), { content: [item()], type: 'bulletList' }))), '- a\n\n -\n') assert.equal(markdown(adfToMarkdown(outer(text('a'), { content: [item(text('b'))], type: 'bulletList' }))), '- a\n - b\n') }) @@ -381,6 +382,9 @@ test('refuses a document nested deeper than the emitter carries', () => { let node: AdfNode = paragraph({ text: 'x', type: 'text' }) for (let depth = 0; depth < 600; depth += 1) node = { content: [node], type: 'blockquote' } assert.equal(code(adfToMarkdown(document(node))), 'unsupported-nesting-depth') + let carried: AdfNode = paragraph({ text: 'x', type: 'text' }) + for (let depth = 0; depth < 500; depth += 1) carried = { content: [carried], type: 'blockquote' } + assert.ok(adfToMarkdown(document(carried)).ok) }) test('emits an empty list item without trailing whitespace', () => { diff --git a/src/markdown/emit/adf-to-markdown.ts b/src/markdown/emit/adf-to-markdown.ts index d3f016e..1975ed2 100644 --- a/src/markdown/emit/adf-to-markdown.ts +++ b/src/markdown/emit/adf-to-markdown.ts @@ -7,7 +7,7 @@ import { carriesOnly, isAdfDocument } from '../../adf/document.ts' import { emitInlineLine } from './inline-line.ts' import { failure, success, type ConvertErrorPath, type Result } from '../../result.ts' import { fencedCodeBlock } from '../backtick-runs.ts' -import { holdsControlCharacter, holdsEntityReference, holdsNullCharacter, isThematicBreak } from '../commonmark-grammar.ts' +import { holdsControlCharacter, holdsEntityReference, holdsNullCharacter, isThematicBreak, markerInterruptsParagraph } from '../commonmark-grammar.ts' import { largestNesting } from '../../nesting.ts' import { spellDirectiveHeader } from './block-directive-spelling.ts' import { tryImage } from './image.ts' @@ -72,8 +72,10 @@ function separationBetween(previous: PlacedBlock, next: PlacedBlock, container: } function interruptsParagraph(node: AdfNode): boolean { - if (node.type === 'orderedList') return false - return ((node.content ?? [])[0]?.content ?? []).length > 0 + const items = node.content ?? [] + const empty = (items[0]?.content ?? []).length === 0 + if (node.type !== 'orderedList') return markerInterruptsParagraph(undefined, empty) + return markerInterruptsParagraph(listStart(node, items.length) ?? 0, empty) } function emitBlock(node: AdfNode, path: ConvertErrorPath, depth: number): Result { diff --git a/src/markdown/parse/blocks.ts b/src/markdown/parse/blocks.ts index f21e924..7487267 100644 --- a/src/markdown/parse/blocks.ts +++ b/src/markdown/parse/blocks.ts @@ -1,5 +1,14 @@ import type { LinkDefinition } from './link-reference-definitions.ts' -import { atxHeading, claimsDirectiveLine, claimsPipeLine, closingCodeFence, isThematicBreak, openingCodeFence, setextHeadingLevel } from '../commonmark-grammar.ts' +import { + atxHeading, + claimsDirectiveLine, + claimsPipeLine, + closingCodeFence, + isThematicBreak, + markerInterruptsParagraph, + openingCodeFence, + setextHeadingLevel, +} from '../commonmark-grammar.ts' import { openingHtmlBlock } from './html-blocks.ts' import { readLinkDefinitions } from './link-reference-definitions.ts' @@ -85,10 +94,11 @@ function continuesContainer(walk: Walk, container: OpenContainer, line: string): } function openContainers(walk: Walk, line: string, paragraphOpen: boolean, depth: number): { opened: boolean; rest: string } { + const unmatched = walk.stack[depth] let opened = false let rest = line while (leadingColumns(rest) < indentedCodeColumns) { - const start = containerStart(rest, opened ? false : paragraphOpen, opened ? undefined : walk.stack[depth]) + const start = containerStart(rest, opened ? false : paragraphOpen, opened ? undefined : unmatched) if (start === undefined) break if (!opened) closeContainers(walk, depth) opened = true @@ -102,11 +112,15 @@ function containerStart(line: string, paragraphOpen: boolean, enclosing: OpenCon const opener = removeColumns(line, largestOpenerIndentation) if (opener.startsWith('>')) return { kind: 'blockquote', rest: removeColumns(opener.slice(1), 1) } if (isThematicBreak(opener) || (paragraphOpen && setextHeadingLevel(opener) !== undefined)) return undefined + return itemStart(line, opener, paragraphOpen, enclosing) +} + +function itemStart(line: string, opener: string, paragraphOpen: boolean, enclosing: OpenContainer | undefined): ContainerStart | undefined { const marker = itemMarker(opener) if (marker === undefined) return undefined const after = opener.slice(marker.width) const blank = blankLine.test(after) - if (paragraphOpen && (blank || (marker.list.kind === 'orderedList' && marker.list.start !== 1))) return undefined + if (paragraphOpen && !markerInterruptsParagraph(marker.list.kind === 'orderedList' ? marker.list.start : undefined, blank)) return undefined const spaces = leadingColumns(after) const padding = blank || spaces > indentedCodeColumns ? 1 : spaces const continued = enclosing?.kind === 'item' && enclosing.list.kind === marker.list.kind && enclosing.marker === marker.marker diff --git a/src/markdown/parse/markdown-to-adf.test.ts b/src/markdown/parse/markdown-to-adf.test.ts index c39219f..0d7e250 100644 --- a/src/markdown/parse/markdown-to-adf.test.ts +++ b/src/markdown/parse/markdown-to-adf.test.ts @@ -197,6 +197,7 @@ test('opens a list beside a paragraph only where the marker interrupts it', () = assert.deepEqual(content(markdownToAdf('Part.\n- - -\n')), [paragraph('Part.'), { type: 'rule' }]) assert.deepEqual(content(markdownToAdf('Part.\n-\n')), [{ attrs: { level: 2 }, content: [text('Part.')], type: 'heading' }]) assert.deepEqual(content(markdownToAdf('- a\n 2. b\n')), [bulletList(item(paragraph('a 2. b')))]) + assert.deepEqual(content(markdownToAdf('- a\n 1. b\n')), [bulletList(item(paragraph('a'), orderedList(1, item(paragraph('b')))))]) }) test('folds a lazy continuation into the paragraph the container holds', () => { diff --git a/src/markdown/parse/markdown-to-adf.ts b/src/markdown/parse/markdown-to-adf.ts index 19dd22f..9fdc898 100644 --- a/src/markdown/parse/markdown-to-adf.ts +++ b/src/markdown/parse/markdown-to-adf.ts @@ -22,16 +22,28 @@ function blockNodes(blocks: readonly Block[], path: ConvertErrorPath, depth: num return success(content) } +// Switched, not chained: `noImplicitReturns` then refuses the kind a later milestone adds and forgets. function blockNode(block: Block, path: ConvertErrorPath, depth: number): Result { - if (block.kind === 'blockquote') return containerNode({ type: 'blockquote' }, block.blocks, path, depth) - if (block.kind === 'bulletList') return listNode({ type: 'bulletList' }, block.items, path, depth) - if (block.kind === 'claim') return claimFailure(block.construct, path) - if (block.kind === 'code') return success(codeBlockNode(block.language, block.text)) - if (block.kind === 'heading') return success(withContent({ attrs: { level: block.level }, type: 'heading' }, block.text)) - if (block.kind === 'html') return failure('unmappable-html', `no ADF node carries ${block.construct}`, path) - if (block.kind === 'orderedList') return listNode({ attrs: { order: block.start }, type: 'orderedList' }, block.items, path, depth) - if (block.kind === 'paragraph') return success(withContent({ type: 'paragraph' }, block.text)) - return success({ type: 'rule' }) + switch (block.kind) { + case 'blockquote': + return containerNode({ type: 'blockquote' }, block.blocks, path, depth) + case 'bulletList': + return listNode({ type: 'bulletList' }, block.items, path, depth) + case 'claim': + return claimFailure(block.construct, path) + case 'code': + return success(codeBlockNode(block.language, block.text)) + case 'heading': + return success(withContent({ attrs: { level: block.level }, type: 'heading' }, block.text)) + case 'html': + return failure('unmappable-html', `no ADF node carries ${block.construct}`, path) + case 'orderedList': + return listNode({ attrs: { order: block.start }, type: 'orderedList' }, block.items, path, depth) + case 'paragraph': + return success(withContent({ type: 'paragraph' }, block.text)) + case 'rule': + return success({ type: 'rule' }) + } } function containerNode(node: AdfNode, blocks: readonly Block[], path: ConvertErrorPath, depth: number): Result { @@ -51,8 +63,12 @@ function listNode(node: AdfNode, items: readonly Block[][], path: ConvertErrorPa } function claimFailure(construct: ClaimedConstruct, path: ConvertErrorPath): Result { - if (construct === 'directive') return failure('malformed-directive', 'the line claims a directive and parses as none', path) - return failure('malformed-pipe-table', 'the line claims a pipe table and parses as none', path) + switch (construct) { + case 'directive': + return failure('malformed-directive', 'the line claims a directive and parses as none', path) + case 'pipe-table': + return failure('malformed-pipe-table', 'the line claims a pipe table and parses as none', path) + } } function codeBlockNode(language: string, text: string): AdfNode { diff --git a/src/nesting.ts b/src/nesting.ts index 1019551..89984d2 100644 --- a/src/nesting.ts +++ b/src/nesting.ts @@ -1 +1,2 @@ +// One level per block-list recursion in either direction — a list and its items count once — or the two guards disagree. export const largestNesting = 500