diff --git a/docs/contracts.md b/docs/contracts.md index 5bc7ee37..ae8897fc 100644 --- a/docs/contracts.md +++ b/docs/contracts.md @@ -262,8 +262,10 @@ no operation replaces a whole file. - `capability.ts` — `editable` | `read-only-opaque` | `repeated-source-node` | `runtime-aggregate` | `unsupported`; `editable` and (since step 7) `repeated-source-node` accept visual intents: a node a loop repeats is one - source node, and an edit of it changes every copy. Nothing is placed beside - it (the loop body is code), and a stale intent whose node changed capability + source node, and an edit of it changes every copy. Sibling placement inside + an element or component preserves that markup list and its loop bindings; + placement beside a direct loop or branch return root remains refused because + that list is JavaScript. A stale intent whose node changed capability since it was authored — wrapped in a loop outside Stacki — is refused `region-externally-modified`. The renderer shows every other capability beside the selection (`src/editor/nodeCapability.ts`, `CapabilityNotice`). diff --git a/shared/engine/planTree.ts b/shared/engine/planTree.ts index 055d2512..0a44f036 100644 --- a/shared/engine/planTree.ts +++ b/shared/engine/planTree.ts @@ -133,7 +133,7 @@ export function planInsertNode( return resolved; } const target = resolved.value; - if (!placeWritable(target.current, operation.placement)) { + if (!placeWritable(currentProjection(context), target.current, operation.placement)) { return err('unsupported-operation'); } const current = currentProjection(context); @@ -233,11 +233,10 @@ function bytesHaveCrlf(bytes: ByteString): boolean { return false; } -// A node inside a loop is one source node the loop repeats (plan §6). Its own -// content is edited like any node's (step 7), but the list it sits in is the -// loop body's code, so nothing is removed from it or placed beside it — with one -// exception: moving it. That gesture exists to take a node out of its loop, and -// the move rewrites exactly what would break (the kind-changing class of §3.3). +// A node inside a loop is one source node the loop repeats (plan §6). A move +// within its markup container preserves those bytes and bindings. Leaving a +// loop rewrites only references the destination cannot satisfy (§3.3). The +// loop's direct return roots remain code-owned, not a markup sibling list. export function planMoveNode( context: PlanContext, anchor: AnchorRef, @@ -260,7 +259,7 @@ export function planMoveNode( return err('unsupported-operation'); } } - if (!placeWritable(destination.value.current, operation.placement)) { + if (!placeWritable(currentProjection(context), destination.value.current, operation.placement)) { return err('unsupported-operation'); } if (startsWith(destination.value.current.path, moved.path)) { @@ -821,13 +820,26 @@ function shifted(path: readonly number[]): readonly number[] { } // Beside a node, the list it sits in is written; inside it, its own content. -function placeWritable(node: ProjectedNode, placement: Placement): boolean { +function placeWritable( + projection: ValidProjection, + node: ProjectedNode, + placement: Placement, +): boolean { if (inside(placement)) { if (node.callback === true) { return false; // Separate return sites cannot receive one shared sibling insertion. } + return nodeEditable(node); + } + if (node.capability === 'repeated-source-node') { + // Repetition is inherited by every descendant. Only the immediate owner + // decides whether siblings are markup or separate JavaScript return sites. + const parent = nodeAtPath(projection, parentPath(node.path)); + assert(parent !== undefined, 'A repeated source node has an enclosing owner'); + assert(node.list === 'markup', 'Repeated source nodes belong to markup syntax'); + return parent.kind === 'element' || parent.kind === 'component'; } - return inside(placement) ? nodeEditable(node) : nodePlaceable(node); + return nodePlaceable(node); } function inside(placement: Placement): boolean { diff --git a/test/electron/documents/repeatedMove.test.ts b/test/electron/documents/repeatedMove.test.ts new file mode 100644 index 00000000..1e4f689e --- /dev/null +++ b/test/electron/documents/repeatedMove.test.ts @@ -0,0 +1,269 @@ +// Goal: navigator moves inside repeated markup use source splices, preserving loop bindings, +// formatting and unrelated writers. Code-owned return roots must still reject sibling placement. +// Method: drive real page IPC on isolated scratch files; compare exact bytes and apply Undo. +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { test } from 'node:test'; +import { mainHarness } from '../../helpers/mainHarness.ts'; +import type { Edit, NodeRef } from '#dist/shared/engine/editRequest.js'; +import { parsePageDiskRead, parsePageEditResult } from '#dist/shared/ipc/pageSave.js'; +import type { PageDiskRead } from '#dist/shared/ipc/pageSave.js'; +import type { PageNode } from '#dist/shared/page/pageNode.js'; + +function fixture(source: string) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'stacki-repeated-move-')); + fs.mkdirSync(path.join(root, 'src/pages'), { recursive: true }); + fs.mkdirSync(path.join(root, 'user')); + fs.writeFileSync(path.join(root, 'package.json'), '{"dependencies":{"astro":"*"}}'); + const file = path.join(root, 'src/pages/index.astro'); + fs.writeFileSync(file, source); + const harness = mainHarness(path.join(root, 'user')); + return { + read: async () => parsePageDiskRead(await harness.invoke('page:read', file)), + text: () => fs.readFileSync(file, 'utf8'), + write: (text: string) => fs.writeFileSync(file, text), + edit: async (checksum: string, edit: Edit) => + parsePageEditResult( + await harness.invoke('page:edit', { pagePath: file, authoredChecksum: checksum, edit }), + ), + dispose: () => { + harness.dispose(); + fs.rmSync(root, { recursive: true, force: true }); + }, + }; +} + +function named(page: PageDiskRead, name: string): NodeRef { + assert.ok(page.editable); + const pending: { readonly node: PageNode; readonly path: readonly number[] }[] = + page.model.nodes.map((node, index) => ({ node, path: [index] })); + for (let index = 0; index < pending.length; index++) { + assert.ok(pending.length < 100, 'the synthetic fixture is bounded'); + const entry = pending[index]; + assert.ok(entry !== undefined); + const node = entry.node; + if ('name' in node && node.name === name) { + assert.ok(node.start !== undefined && node.end !== undefined); + return { path: entry.path, kind: node.kind, span: { start: node.start, end: node.end } }; + } + if ('children' in node && node.children !== undefined) { + pending.push( + ...node.children.map((child, at) => ({ node: child, path: [...entry.path, at] })), + ); + } + } + assert.fail(`Missing ${name}`); +} + +const HEADING = ''; +const IMAGE = ''; +const ICON = ""; +const CHILDREN = [HEADING, IMAGE, ICON]; +const WRAPPERS = [ + ['{rechtsgebiete.map((gebiet) => (', '))}'], + ['{rechtsgebiete.map((gebiet) => (
{gebiet.visible && (', ')}
))}'], + ['{rechtsgebiete.map((gebiet) => (
{gebiet.items.map((item) => (', '))}
))}'], + ['{rechtsgebiete.map((gebiet) => { const label = gebiet.title; return (', '); })}'], +] as const; +const PLACES = [ + { source: 'Icon', destination: 'Heading', placement: 'before', order: [ICON, HEADING, IMAGE] }, + { source: 'Icon', destination: 'Image', placement: 'before', order: [HEADING, ICON, IMAGE] }, + { source: 'Icon', destination: 'Image', placement: 'after', order: CHILDREN }, + { source: 'Icon', destination: 'Card', placement: 'first-child', order: [ICON, HEADING, IMAGE] }, + { source: 'Icon', destination: 'Card', placement: 'last-child', order: CHILDREN }, + { source: 'Heading', destination: 'Icon', placement: 'after', order: [IMAGE, ICON, HEADING] }, + { source: 'Heading', destination: 'Icon', placement: 'before', order: [IMAGE, HEADING, ICON] }, + { source: 'Image', destination: 'Heading', placement: 'before', order: [IMAGE, HEADING, ICON] }, +] as const; + +for (const [prefix, suffix] of WRAPPERS) { + for (const place of PLACES) { + test(`repeated move ${place.placement} ${place.destination} in ${prefix}`, async (context) => { + const card = (children: readonly string[]) => + `\r\n ${children.join('\r\n ')}\r\n`; + const source = `${prefix}${card(CHILDREN)}${suffix}\r\n\r\n`; + const harness = fixture(source); + context.after(harness.dispose); + const page = await harness.read(); + const moved = await harness.edit(page.checksum, { + tag: 'move-node', + target: named(page, place.source), + destination: named(page, place.destination), + placement: place.placement, + }); + assert.ok(moved.ok, JSON.stringify(moved)); + assert.equal( + harness.text(), + `${prefix}${card(place.order)}${suffix}\r\n\r\n`, + ); + const undone = await harness.edit(moved.value.checksum, { + tag: 'revert', + hunks: moved.value.inverse, + }); + assert.ok(undone.ok, JSON.stringify(undone)); + assert.equal(harness.text(), source, 'Undo restores every original byte'); + }); + } +} + +test('move out of a nested container retains outer and inner loop bindings', async (context) => { + const icon = ''; + const prefix = '{rechtsgebiete.map((gebiet) => (
{gebiet.items.map((item) => ('; + const suffix = '))}
))}'; + const source = `${prefix}${icon}${HEADING}${suffix}`; + const harness = fixture(source); + context.after(harness.dispose); + const page = await harness.read(); + const moved = await harness.edit(page.checksum, { + tag: 'move-node', + target: named(page, 'Icon'), + destination: named(page, 'Heading'), + placement: 'before', + }); + assert.ok(moved.ok, JSON.stringify(moved)); + assert.equal(harness.text(), `${prefix}${icon}${HEADING}${suffix}`); + const undone = await harness.edit(moved.value.checksum, { + tag: 'revert', + hunks: moved.value.inverse, + }); + assert.ok(undone.ok); + assert.equal(harness.text(), source); +}); + +test('repeated moves and Undo preserve an external change elsewhere', async (context) => { + const filler = '

A paragraph separating the move from other writers and Undo context.

'; + const source = + `{rechtsgebiete.map((gebiet) => (${CHILDREN.join('')}))}` + `${filler}