Group a task list into typed task items in one place, refusing a list a non-task opens

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<AdfNode[]>
     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<AdfNode[]> {

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])
+})

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<EmittedBlock> {
-  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)
 }

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.

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)
+})

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')

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<AdfDocument>`.** | 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