diff --git a/src/markdown/portable/adf-to-portable-markdown.ts b/src/markdown/portable/adf-to-portable-markdown.ts index 5e07c51..2b4bff6 100644 --- a/src/markdown/portable/adf-to-portable-markdown.ts +++ b/src/markdown/portable/adf-to-portable-markdown.ts @@ -12,7 +12,7 @@ import { largestNesting } from '../../nesting.ts' import { linkAttrs } from '../../adf/mark-attributes.ts' import { nodeAttrs, nodeContent, nodeMarks, writableContent } from '../../adf/document.ts' import { portableWriting } from './block-spelling.ts' -import { splitTaskList } from './task-list.ts' +import { splitTaskList, taskListItems } from './task-list.ts' import { taskMarker } from './conventions.ts' import { editorNormalForm, sourceIndex, toEditorNormal } from './editor-normal.ts' import { withinEngineLimits } from '../../length-bound.ts' @@ -269,14 +269,14 @@ function reduceTaskList(node: AdfNode, reduction: Reduction): Result if (!reduced.ok) return reduced if (reduced.value.length > 0) kept.push({ blocks: reduced.value, child }) } - const regular = isTaskItem(kept[0]?.child) && kept.every(({ child }) => isTaskItem(child) || child.type === 'taskList') + const nestsTasks = kept.every(({ child }) => isTaskItem(child) || child.type === 'taskList') const tasks: AdfNode[] = [] for (const { blocks, child } of kept) { - const standsAlone = !regular && !isTaskItem(child) && child.type !== 'taskList' + const standsAlone = !nestsTasks && !isTaskItem(child) && child.type !== 'taskList' for (const block of standsAlone ? [{ content: blocks, type: 'listItem' }] : blocks) tasks.push(block) } - if (regular) return success(splitTaskList({ content: tasks, type: 'taskList' })) - return success(listOf(tasksAsText(tasks), 'bulletList')) + const items = nestsTasks ? taskListItems({ content: tasks, type: 'taskList' }) : undefined + return success(items === undefined ? listOf(tasksAsText(tasks), 'bulletList') : splitTaskList(items)) } function reduceTask(task: AdfNode, at: Reduction): Result { diff --git a/src/markdown/portable/block-spelling.test.ts b/src/markdown/portable/block-spelling.test.ts new file mode 100644 index 0000000..e889644 --- /dev/null +++ b/src/markdown/portable/block-spelling.test.ts @@ -0,0 +1,19 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +import { portableWriting } from './block-spelling.ts' +import { writeMarkdown } from '../emit/adf-to-markdown.ts' + +test('refuses a task list a child other than a task opens, where the portable reduction would have left none', () => { + const list = { content: [{ content: [{ text: 'stray', type: 'text' }], type: 'paragraph' }, { attrs: { state: 'TODO' }, type: 'taskItem' }], type: 'taskList' } + const written = writeMarkdown({ content: [{ type: 'rule' }, list], type: 'doc', version: 1 }, portableWriting) + assert.deepEqual(written.ok ? written.value : [written.error.code, written.error.path], ['unsupported-node-shape', ['content', 1]]) +}) + +test('names the path of a failure inside a task list by the child it writes', () => { + const task = { attrs: { state: 'TODO' }, content: [{ text: 't', type: 'text' }], type: 'taskItem' } + const unwritable = { attrs: { state: 'TODO' }, content: [{ marks: [{ type: 'strong' }, { type: 'strong' }], text: 'x', type: 'text' }], type: 'taskItem' } + const list = { content: [task, { content: [task], type: 'taskList' }, unwritable], type: 'taskList' } + const written = writeMarkdown({ content: [list], type: 'doc', version: 1 }, portableWriting) + assert.deepEqual(written.ok ? written.value : written.error.path, ['content', 0, 'content', 2, 'content', 0]) +}) diff --git a/src/markdown/portable/block-spelling.ts b/src/markdown/portable/block-spelling.ts index 878649a..554f80b 100644 --- a/src/markdown/portable/block-spelling.ts +++ b/src/markdown/portable/block-spelling.ts @@ -9,6 +9,7 @@ import { failure, success } from '../../result.ts' import { isTaskItem } from '../../adf/block-nodes.ts' import { largestNesting } from '../../nesting.ts' import { nodeAttrs, nodeContent } from '../../adf/document.ts' +import { taskListItems } from './task-list.ts' const portableBlockWriters: FlavourBlockWriters = { // GitHub reads a task marker opening any item's first paragraph as a checkbox, whatever its siblings hold. @@ -40,19 +41,25 @@ function headed(head: string): (body: BlockText) => BlockText { } function tryTaskList(node: AdfNode, path: ConvertErrorPath, depth: number, writing: Writing): Result { - const items: PlacedBlock[][] = [] + const items = taskListItems(node) + // The portable reduction opens every task list with a task: a child nesting in no task would have no list item to stand in. + if (items === undefined) return failure('unsupported-node-shape', 'a task list opens with a child other than a task', path) + const inners: BlockText[] = [] let headroom = largestNesting - depth - 1 - // A child other than a task nests in the task before it. - for (const [index, child] of nodeContent(node).entries()) { - const task = isTaskItem(child) - const walk = task ? taskBlocks(child, [...path, 'content', index], depth + 1, writing) : placedBlock(child, [...path, 'content', index], depth + 1, writing) - if (!walk.ok) return walk - headroom = Math.min(headroom, walk.value.headroom) - const previous = items.at(-1) - if (task || previous === undefined) items.push([...walk.value.blocks]) - else for (const block of walk.value.blocks) previous.push(block) + let index = 0 + for (const { after, task } of items) { + const blocks: PlacedBlock[] = [] + for (const child of [task, ...after]) { + const childPath = [...path, 'content', index] + const walk = isTaskItem(child) ? taskBlocks(child, childPath, depth + 1, writing) : placedBlock(child, childPath, depth + 1, writing) + if (!walk.ok) return walk + headroom = Math.min(headroom, walk.value.headroom) + for (const block of walk.value.blocks) blocks.push(block) + index += 1 + } + inners.push(joinBlocks(blocks, 'list-item')) } - const list = spelledList(items.map((blocks) => joinBlocks(blocks, 'list-item')), undefined, headroom) + const list = spelledList(inners, undefined, headroom) // The portable reduction leaves no task a list item cannot hold: a directive here would break the flavour. return list === undefined ? failure('unsupported-node-shape', 'a task holds blocks no list item spells', path) : success(list) } diff --git a/src/markdown/portable/portable-markdown-to-adf.ts b/src/markdown/portable/portable-markdown-to-adf.ts index 9512784..be1039f 100644 --- a/src/markdown/portable/portable-markdown-to-adf.ts +++ b/src/markdown/portable/portable-markdown-to-adf.ts @@ -2,6 +2,7 @@ import type { AdfDocument, AdfNode } from '../../adf/document.ts' import type { Block } from '../parse/blocks.ts' import type { ConvertError, ConvertErrorPath, ParseError, Result, SourcePosition } from '../../result.ts' import type { FlavourBlockReaders, FlavourReading } from '../parse/markdown-to-adf.ts' +import type { TaskListItem } from './task-list.ts' import { containerNode, contentNode, listNode, readBlocks, readDocument, tooDeep } from '../parse/markdown-to-adf.ts' import { largestNesting } from '../../nesting.ts' import { leadingMarker, readAlertMarker, readTaskMarker } from './conventions.ts' @@ -127,7 +128,7 @@ function taskBlocks(items: readonly Block[][], reading: FlavourReading, path: Co const led = taskItems(items) if (led === undefined) return undefined if (depth + 1 > largestNesting) return tooDeep([...path, 'content', 0]) - const tasks: AdfNode[] = [] + const tasks: TaskListItem[] = [] let refusal: Refusal | undefined for (const [index, { marker, others, position, text }] of led.entries()) { const body = text.replace(/^(?:[ \t\n]|\\\n)+/, '') @@ -138,10 +139,10 @@ function taskBlocks(items: readonly Block[][], reading: FlavourReading, path: Co for (const node of read.nodes) task.content.push(node) refusal ??= read.refusal } - tasks.push(task) + tasks.push({ after: [], task }) if (refusal !== undefined) break } - return success({ nodes: splitTaskList({ content: tasks, type: 'taskList' }), refusal }) + return success({ nodes: splitTaskList(tasks), refusal }) } // A refused block reads to a placeholder. The split tells only paragraphs and task lists from other blocks, so a paragraph that failed to read stays a paragraph. diff --git a/src/markdown/portable/task-list.test.ts b/src/markdown/portable/task-list.test.ts index 89c7f79..92cbd52 100644 --- a/src/markdown/portable/task-list.test.ts +++ b/src/markdown/portable/task-list.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert/strict' import test from 'node:test' import type { AdfNode } from '../../adf/document.ts' -import { pathTo, splitTaskList } from './task-list.ts' +import { pathTo, splitTaskList, taskListItems } from './task-list.ts' test('names the steps from a block to a node deep in it', () => { const target: AdfNode = { text: 'x', type: 'text' } @@ -18,6 +18,23 @@ test('names no steps to a node the blocks do not hold', () => { test('keeps the inline nodes of a lone paragraph in the taskItem it becomes, so pathTo still finds them', () => { const inline: AdfNode = { text: 'x', type: 'text' } const item: AdfNode = { attrs: { state: 'TODO' }, content: [{ content: [inline], type: 'paragraph' }], type: 'blockTaskItem' } - const blocks = splitTaskList({ content: [item], type: 'taskList' }) + const blocks = splitTaskList([{ after: [], task: item }]) assert.deepEqual(pathTo(blocks, inline), [0, 'content', 0, 'content', 0]) }) + +test('groups a task list into tasks, each with the children after it up to the next task', () => { + const first: AdfNode = { attrs: { state: 'TODO' }, type: 'taskItem' } + const nested: AdfNode = { content: [first], type: 'taskList' } + const second: AdfNode = { attrs: { state: 'DONE' }, content: [{ type: 'paragraph' }], type: 'blockTaskItem' } + assert.deepEqual(taskListItems({ content: [first, nested, nested, second], type: 'taskList' }), [ + { after: [nested, nested], task: first }, + { after: [], task: second }, + ]) + assert.deepEqual(taskListItems({ type: 'taskList' }), []) +}) + +test('groups no task list a child other than a task opens', () => { + const task: AdfNode = { attrs: { state: 'TODO' }, type: 'taskItem' } + assert.equal(taskListItems({ content: [{ type: 'paragraph' }, task], type: 'taskList' }), undefined) + assert.equal(taskListItems({ content: [{ content: [task], type: 'taskList' }, task], type: 'taskList' }), undefined) +}) diff --git a/src/markdown/portable/task-list.ts b/src/markdown/portable/task-list.ts index 7610243..02a729b 100644 --- a/src/markdown/portable/task-list.ts +++ b/src/markdown/portable/task-list.ts @@ -4,21 +4,36 @@ import { isTaskItem } from '../../adf/block-nodes.ts' import { nodeContent } from '../../adf/document.ts' // Atlassian's schema holds a blockTaskItem to paragraphs and extensions, and the portable flavour spells no extension: an item keeps its paragraphs, then the task lists nested in it, and the rest of it stands after the list, which resumes at the next task. -export function splitTaskList(list: AdfNode): AdfNode[] { +export function splitTaskList(items: readonly TaskListItem[]): AdfNode[] { const blocks: AdfNode[] = [] let tasks: AdfNode[] = [] - for (const item of itemsOf(nodeContent(list))) { + for (const item of items) { const { kept, rest } = itemParts(item) for (const node of kept) tasks.push(node) if (rest.length === 0) continue - if (tasks.length > 0) blocks.push({ ...list, content: tasks }) + if (tasks.length > 0) blocks.push({ content: tasks, type: 'taskList' }) tasks = [] for (const block of rest) blocks.push(block) } - if (tasks.length > 0) blocks.push({ ...list, content: tasks }) + if (tasks.length > 0) blocks.push({ content: tasks, type: 'taskList' }) return blocks } +// A task and the children after it up to the next task, which nest in it. +export type TaskListItem = { after: AdfNode[]; task: AdfNode } + +// The list's items, or undefined where a child other than a task opens it. +export function taskListItems(list: AdfNode): TaskListItem[] | undefined { + const items: TaskListItem[] = [] + for (const child of nodeContent(list)) { + const item = items.at(-1) + if (isTaskItem(child)) items.push({ after: [], task: child }) + else if (item === undefined) return undefined + else item.after.push(child) + } + return items +} + // Where `target` stands among `blocks`: its block's index, then the steps down to it. The split rebuilds lists and items but never an inline node, so an inline marker stays findable. export function pathTo(blocks: readonly AdfNode[], target: AdfNode): [number, ...ConvertErrorPath] | undefined { const pending: Visit[] = blocks.map((node, step) => ({ node, parent: undefined, step })) @@ -38,21 +53,8 @@ function stepsTo(visit: Visit): [number, ...ConvertErrorPath] { return [at.step, ...steps.reverse()] } -// An item is a task and the children up to the next one. -function itemsOf(children: readonly AdfNode[]): AdfNode[][] { - const items: AdfNode[][] = [] - for (const child of children) { - const item = items.at(-1) - if (item === undefined || isTaskItem(child)) items.push([child]) - else item.push(child) - } - return items -} - // The item reads as one run: the task's blocks, then the children after it. -function itemParts(item: readonly AdfNode[]): { kept: AdfNode[]; rest: AdfNode[] } { - const [task, ...after] = item - if (task === undefined || !isTaskItem(task)) return { kept: [], rest: [...item] } +function itemParts({ after, task }: TaskListItem): { kept: AdfNode[]; rest: AdfNode[] } { const content = task.type === 'blockTaskItem' ? nodeContent(task) : [] const run = [...content, ...after] const paragraphs = runEnd(content, 0, 'paragraph') diff --git a/todo.md b/todo.md index dc376cb..fe21883 100644 --- a/todo.md +++ b/todo.md @@ -13,7 +13,6 @@ | ID | Release | Blocked by | Exempt | Item | L | E | I | R | S | Moves | Score | |---|---|---|---|---|---|---|---|---|---|---|---| | 153 | 0.4.0 | | question | **Decide whether `src/markdown/` root may import `emit/`, `parse/` or `portable/`, and hold the answer in `.oxlintrc.json` and §The source parts by ADF and format.** | 1 | 2 | 2 | 3 | 2 | code health by 3 | 26.2 | -| 175 | 0.4.0 | | | **Delete the `previous === undefined` branch, the `'a task holds blocks no list item spells'` failure and the `path` arguments of `tryTaskList`, once a type makes a task list opening with a non-task unrepresentable.** | 1 | 1 | 1 | 1 | 2 | code health by 2 | 17.4 | | 176 | 0.4.0 | | | **Delete `runEdge`'s `standing === 'moves'` give-up, and any code only it needs, once a test or an argument shows `runEdge` never reaches it, or else pin the case with a hand-worked line.** | 1 | 1 | 2 | 2 | 2 | code health by 2 | 17.4 | | 45 | 0.4.0 | | defect | **Replace `isAdfDocument` with a reader returning `Result`.** | 2 | 1 | 3 | 4 | 3 | 1 by 2 | 13.1 | | 127 | 0.4.0 | | principle | **Hold what `entity-references.test.ts`, `json-text.test.ts` and `result.test.ts` check by asserting outcomes, so none reads source text.** | 2 | 2 | 1 | 3 | 3 | code health by 2 | 13.1 | @@ -72,16 +71,6 @@ a paragraph opening with a spellable `!adf:link` (§Nothing recurses unbounded). `src/markdown/**` bans only `html/`; banning `emit/`, `parse/` and `portable/` for the root's own files, with its `ci.sh` probe, extends the decision. -### 175. Delete the `previous === undefined` branch, the `'a task holds blocks no list item spells'` failure and the `path` arguments of `tryTaskList`, once a type makes a task list opening with a non-task unrepresentable. - -`reduceToPortable` leaves every task list opening with a task. So `adfToPortableMarkdown` reaches -neither the `previous === undefined` branch of `tryTaskList` (`portable/block-spelling.ts`) nor -its failure. Deleting the branch made two panel seats rule the code harder to read: a block pushed -before the first task would vanish silently. Delete it only together with a type that makes a task -list opening with a non-task unrepresentable. Mutants of the `path` arguments to `taskBlocks` and -`placedBlock` survived, so no test shows they reach an error. The `headroom` is not a guard: a list -that cannot be a markdown list refuses by it, and a test pins that. - ### 176. Delete `runEdge`'s `standing === 'moves'` give-up, and any code only it needs, once a test or an argument shows `runEdge` never reaches it, or else pin the case with a hand-worked line. In `src/markdown/emit/unspellable-runs.ts`, Stryker's mutants of `runEdge`'s