From 819a5565e6c082d0c93a41b7b0a6296ef39e8580 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 07:40:52 +0000 Subject: [PATCH] fix(fields): refuse edits that would flatten or split a field (#42 step 1) A whole-paragraph edit of a paragraph holding a Word field silently turned the field into frozen text (exit 0), and a span replace that covered a field left its `end` fldChar behind as an orphan that validate called valid. - core/fields: XML-side field parser (begin/separate/end stack per story, positions in the span walkers' offset space) plus refusals. - edit (markdown/--text/--runs/code, range, batch, tracked or not) refuses a paragraph holding a field with FIELD_IN_PARAGRAPH, nothing written. - span edit / replace / cross-paragraph replace refuse a span that cuts a field; a span covering a whole field removes its markers too. - paragraph removal refuses to take one end of a multi-paragraph field. - tracked deletion writes w:delInstrText; reject renames it back. - validate reports unbalanced fldChar markers as an error. Refs #42 Triage-Routine: true Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01LkctELRxzFrjMAGvfnY1BE --- CLAUDE.md | 1 + REFERENCE.md | 4 +- src/cli/delete/batch.ts | 9 + src/cli/delete/index.tsx | 9 + src/cli/edit/batch.ts | 13 + src/cli/edit/index.tsx | 3 + src/cli/replace/apply-matches.ts | 27 +- src/cli/respond.ts | 2 + src/cli/validate/index.ts | 2 + src/core/edit/index.tsx | 57 ++++ src/core/fields/audit.ts | 44 +++ src/core/fields/index.ts | 9 + src/core/fields/parse.ts | 398 ++++++++++++++++++++++ src/core/fields/refusals.ts | 195 +++++++++++ src/core/fields/remove-covered-fields.tsx | 107 ++++++ src/core/find/index.ts | 52 ++- src/core/find/replace-across.tsx | 9 +- src/core/find/replace-matches.tsx | 17 +- src/core/find/replace-span.tsx | 1 + src/core/markdown/inline.tsx | 1 + src/core/raw/package-consistency.ts | 38 ++- src/core/track-changes/apply.ts | 1 + src/core/track-changes/index.tsx | 5 +- tests/cli/fields.test.ts | 334 ++++++++++++++++++ tests/cli/invariants.test.ts | 25 +- 25 files changed, 1308 insertions(+), 55 deletions(-) create mode 100644 src/core/fields/audit.ts create mode 100644 src/core/fields/index.ts create mode 100644 src/core/fields/parse.ts create mode 100644 src/core/fields/refusals.ts create mode 100644 src/core/fields/remove-covered-fields.tsx create mode 100644 tests/cli/fields.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 8859b5d..2fb6293 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,6 +39,7 @@ These invariants are NOT SUGGESTIONS. These MUST be followed. - **Comments are never anything but hints.** Every `` comment `read` writes into Markdown — locators (``) and the structural visibility annotations (`docx:section` — rendered at the section's START with an `applies-to="pX..pY (below)"` scope on deviating sections so the "which side of the boundary?" off-by-one is explicit, `docx:page`, `docx:table` (widths/borders + the `tables format` properties align/style/cell-margins/repeat-header/row-heights), `docx:cell` merge/shading/vAlign/halign/borders/margins, `docx:track-changes on|off pending=N` ALWAYS at the head (the one hint that states its default too — weak agents can't reliably read "no hint" as "off," and a wrong tracking guess is high-cost, so it's stated outright; `pending=N` likewise always, `pending=0` included, since the default accepted view hides a pending deletion outright, plus a compact `(hdr0: 1, fn2: 1)` when some pending changes live outside the body), `docx:header`/`docx:footer` for page headers/footers (content in a `text` attr, fields as `{page}`/`{date}`/… tokens, a `pending="tcN …"` attr naming the part's tracked changes in every view, and the text following the view — CriticMarkup under `--current`, safe inside the comment because `htmlAttr` escapes every `>` so a deletion's `--` can never form `-->` — authored via `docx headers`/`docx footers`; a UNIFORM marginal rides the head, a marginal that DIFFERS by section renders at that section's START, next to the content it governs), `docx:textbox tbxN anchor="pM"` … `docx:textbox-end tbxN` bracketing a text box's story, printed right after the paragraph that anchors it (the story's own paragraphs carry bare `tbxN:pK` locators), `docx:layout` flagging tab-aligned content that wraps in render — inside a multi-column section, OR a line whose trailing content rides a right-edge LEFT tab so a long value overflows the margin (the résumé `San`/`Francisco` split); the latter is also consolidated into ONE top-of-read `fix-all` summary naming the single-call cure `edit --at pN-pM --tabs right` (a render-only break Markdown can't show), and future image/style hints, all via `formatNote` in `cli/read/annotations.ts`) — is a READ-TIME VISIBILITY HINT. **Naming rule: a BARE comment is a locator (an address you pass to `--at`: ``, ``; a plain empty cell uses `` so it can be filled directly); a `docx:TYPE` comment is docx-cli METADATA** (structure/formatting). Metadata never rides a bare locator — it's its own `docx:` annotation, which may carry the relevant locator as a bare leading token (``). So `docx:` is greppable as "everything docx-cli added beyond addressing," and locators stay short. The Markdown importer DROPS them all (`walker.tsx`'s `case "html"` returns `[]`; the inline walker drops inline comments); they NEVER drive reconstruction or carry data through `read → create`. A comment can't silently corrupt the round-trip or smuggle structure back — what an agent sees is a hint, nothing more. The lossless view is `read --ast`; in-place `edit` preserves structure; the authoring verbs (`docx sections`, `docx tables …`) change it. Emit hints **deviation-only** (only what differs from the document default — even columns, default geometry, `single` borders, `align=left` emit nothing), with TWO deliberate exceptions that always emit even at their default: `docx:section` (every section gets a marker so the `sN` is always addressable) and `docx:track-changes` (states `off` and `pending=0` too, so a weak agent never infers state from a missing hint). Run FORMATTING is the one thing that's Markdown-representable and DOES round-trip, but it rides HTML ELEMENTS (``/``/``/``, via `gatherHtmlSpans`), NOT comments. Even `` (the dominant font/size note) is a hint: `read` omits the baseline from runs and shows the note so an agent can match it, but the importer drops it — a full `read → create` rebuild falls back to the template docDefaults (Calibri 11pt) for the dominant font/size. The footnote-shaped read annotations are hints too: the `[^tcN]` (`--current`) and `[^cN]` (`--comments`) labels and their definition lines are DROPPED on import (`remarkReadHintLabels` in `core/markdown/read-hint-labels.ts`; real notes always read as `[^fnN]`/`[^enN]`), so a pasted `--current` line behaves exactly like its label-free CriticMarkup. No comment, anywhere, is parse-back. - **Everything we parse MUST be represented in the AST**. Markdown is always either a subset or equal to what is in the AST, never less than it. `RUN_BEARING_WRAPPER_TAGS` in `src/core/parser/run-ops.ts` is the AST↔XML offset bridge; every offset-aware walker reads it. See [src/core](src/core/CLAUDE.md). - **`` is transparent at EVERY level, and text boxes are first-class.** ECMA-376 Part 3 lets a producer wrap a block, a paragraph's runs, or a run's children in a markup-compatibility wrapper and nest wrappers inside branches; Word writes every modern shape/text box that way (`wps` under ``, VML under ``, SAME story in both). Every walker resolves it through `alternateContentBranch` in `src/core/mc.ts` (first Choice, else Fallback) — the reader at block/paragraph/run level, the offset walkers via `wrapperContent(node)` in `parser/run-ops.ts` (NEVER `node.children` for a wrapper — that is what keeps a multi-Choice wrapper counted exactly once), and the text-box collector while descending a shape. Dropping the wrapper was issue #4: `replace --all` reported 2 and exited 0 while a third "Acme" sat in a text box a human reads first. A `` story is a block container like a cell (`TextBoxRun` on the anchor paragraph, `tbxN:pK` ids, `blockReferences` parents inside the Choice copy), so `find`/`replace`/`edit`/`insert`/`delete`/`comments` reach it by default; `Document.save` (and the raw gate, BEFORE validating) mirrors each Choice story onto its Fallback twin (`syncTextBoxFallbacks`, post-order so nested boxes sync inner-first) — verified against Word for Mac 365: Word regenerates a box's Fallback when IT edits the box and leaves an untouched box's Fallback verbatim (a stale one stays stale forever), so mirroring on our writes is exactly its behavior. `wc` counts what `read` shows — text boxes included, everywhere. **Word discards comments and footnotes/endnotes inside a text box on its next save** (verified: the markers vanish from the story and the comment from comments.xml, regardless of the Fallback), so `comments add` (`--at` AND `--anchor`) and `footnotes`/`endnotes add` REFUSE a `tbxN` target with `UNSUPPORTED` + a pointer to the anchor paragraph; tracked changes and hyperlinks inside a box survive Word and are allowed. The box rides its anchor RUN: EVERY whole-paragraph `edit` lifts object-bearing runs (drawing/pict/object/AlternateContent) out and puts them back — `--text` around the token diff (`extractObjectRuns`), `--markdown`/`--runs` around the node replace (`liftObjectRuns` → `reattachObjectRuns` in `track-changes/replace.tsx`, untracked carriers even under tracking; a `--markdown` that re-states an image via `![alt](sha256.ext)` drops the lifted DRAWING so `img0` isn't duplicated, while a text box always returns), a `replace` on the anchor run's text keeps a trailing zero-width shape (`sliceRun`), and a tracked delete of the anchor wraps the run in `` WITHOUT rewriting the story's `` (`TextBoxRun.trackedChange` hides it in the accepted view; `iterateBlocks({ view })`). An UNTRACKED removal of the anchor paragraph (`delete --at pN`, `edit --at pN --text ""`) re-anchors the box on the neighboring paragraph — previous, else next — via `relocateObjectRuns` in `track-changes/replace.tsx`; deleting the invoice's Notes line used to take the stamp box with it, silently. +- **A field is atomic structure** (issue #42). A complex field is three zero-width `` markers (begin/separate/end, possibly in different paragraphs) plus ``; `src/core/fields/parse.ts` pairs them per story with a stack in the span walkers' offset space. A mutation carries a field through intact, removes it whole, or refuses before writing: whole-paragraph rebuilds (`Edit.paragraph`/`Edit.range`) refuse a paragraph holding one (`FIELD_IN_PARAGRAPH`), span walkers refuse a span that CUTS one (`fieldSpanRefusal`) and take a COVERED one's markers out first (`removeCoveredFields` — a marker on a span edge otherwise stays behind), paragraph removal refuses to take one end of a multi-paragraph field (`fieldRemovalRefusal`, judged against a batch's whole removal set), a tracked deletion writes `` (reject renames it back), and `validate` reports unbalanced markers. Carrying fields THROUGH a whole-paragraph edit (and the AST/read model) is #41 / #42 step 2 — until then the refusal is the contract. Never add an exit-0 path that drops a `fldChar`. - **Every element must have stable positional ids for locators** (`p0`, `t0`, `c0`, `img0`, `link0`, `tc0`, `hdr0`/`ftr0` for page headers/footers, `tbx0` for a text box — its story chains like a cell: `tbx0:p1`, `tbx0:t0:r0c0:p0`). Block ids shift after structural edits. Re-read between non-trivial mutations. To not force agents to re-read on every modification, we create batch versions of any editing call such that the already made locators can be used. - **Locators are the backbone of the system.** Agents use these to reliably identify elements, ranges (both bounded and one-side bounded). If you're introducing a new concept, it is CRITICAL that these work and work well. - **The CLI surface should be thin.** The CLI surface is concerned with parsing user input. The core is where the actual domain logic happens. diff --git a/REFERENCE.md b/REFERENCE.md index 263489d..b2f7d92 100644 --- a/REFERENCE.md +++ b/REFERENCE.md @@ -420,12 +420,14 @@ The CLI is built for non-interactive agents. **Exit code is the success signal** | `0` | success | — | | `2` | usage / bad locator | `USAGE`, `INVALID_LOCATOR`, `INVALID_XML` | | `3` | addressed thing not found | `FILE_NOT_FOUND`, `PART_NOT_FOUND`, `BLOCK_NOT_FOUND`, `COMMENT_NOT_FOUND`, `IMAGE_NOT_FOUND`, `HYPERLINK_NOT_FOUND`, `TRACKED_CHANGE_NOT_FOUND`, `MATCH_NOT_FOUND` | -| `1` | general failure | `NOT_A_ZIP`, `TRACKED_CHANGE_CONFLICT`, `TABLE_STRUCTURE`, `VALIDATION_FAILED`, `IMAGE_SOURCE`, `RENDER_ENGINE`, `RENDER_FAILED`, `UNSUPPORTED`, `UNHANDLED` | +| `1` | general failure | `NOT_A_ZIP`, `TRACKED_CHANGE_CONFLICT`, `TABLE_STRUCTURE`, `VALIDATION_FAILED`, `IMAGE_SOURCE`, `RENDER_ENGINE`, `RENDER_FAILED`, `UNSUPPORTED`, `FIELD_IN_PARAGRAPH`, `UNHANDLED` | **Errors** print `{code, error, hint?}` JSON to stdout with a nonzero exit — note there is **no `ok` field**; the exit code plus `code` are the unambiguous signal. **An equation is its `$…$` token in text.** Wherever text is matched or addressed — `find`/`replace` (single, `--batch`, cross-paragraph), `edit --at pN:S-E` spans, a whole-paragraph `edit --text`/markdown, `comments add --anchor`/`--at`, `hyperlinks add --at`, `wc pN:S-E` — an equation occupies exactly the token `read` prints for it: `$latex$` inline, `$$latex$$` for a display block (`Energy is $E=mc^2$ in physics.` puts `physics` at `p0:22-29`). An equation a view hides (a pending deletion in the default accepted view) occupies nothing. What a change does to an equation follows from how it meets the token: covering the WHOLE token deletes the equation (a `` when tracked); staying strictly INSIDE it rewrites its LaTeX (`docx replace FILE 'mc^2' 'mc^3'`; tracked, an old/new equation pair like `equations edit --track`); CUTTING a `$` delimiter is refused with `UNSUPPORTED` and nothing is written. The replacement text is always literal — `$5` stays `$5`. A whole-paragraph `edit --text` (or a word-only markdown rewrite) is diffed against that token view: a token left in place keeps the original equation object untouched, a token left out deletes it, a `$…$` span where one stood with different LaTeX edits it, and a rewrite that can't be read one way (two spans where one equation was, an inline token turned `$$…$$`, a token moved to where another equation was) is refused. A comment range opening or closing inside a token snaps outward to the whole equation; a link may hold a whole equation, never part of one. `--runs`, and a `--text` that must rebuild from literal text (run-format flags, `--no-formatting`, or a tracked paragraph with pending revisions), refuse a paragraph holding an equation. `--dry-run` refuses exactly what the real call would. +**A field is one unit.** A Word field — a cross-reference, page number, TOC, merge field — is markup around the text `read` shows (its cached result: `[1, 3, 4]` in a citation is three `REF` fields whose results are `1`, `3`, `4]`). No mutation silently flattens one to plain text or leaves half of one: a whole-paragraph rebuild (`edit --at pN` with markdown, `--text`, `--runs`, or `code edit`; a `pN-pM` range edit; single or `--batch`, tracked or not) of a paragraph holding a field is refused with `FIELD_IN_PARAGRAPH`, naming the fields, and nothing is written — change the text around them with a span instead (`docx find` → `edit --at pN:S-E`, or `docx replace`). A span beside a field keeps it; a span inside its result edits the cached text and keeps the field; a span covering the whole field plus text on either side deletes the field whole (tracked: one `` per marker with ``, so reject restores it); a span covering only part of it is refused with `UNSUPPORTED`. Removing paragraphs (`delete --at pN`/`pN-pM`/`--batch`, `edit --at pN --text ""`) that hold one end of a field spanning paragraphs but not the other is refused too; removing every paragraph it spans takes it whole. `docx validate` reports an unbalanced field (an orphan `fldChar`, e.g. from an older build) as an error. + **`$` and the shell.** Bash expands `$` inside double quotes: `"Energy is $E=mc^2$ now"` reaches the CLI as `Energy is =mc^2$ now`, which reads as "delete the equation." `edit` (text and markdown, single and `--batch`) and `replace` refuse an edit that drops an equation AND carries the shell's fingerprint — a stray unpaired `$`, or the token as bash leaves it (`=mc^2$`, a PID where `$$` was) — with `USAGE`, and the hint shows the line to send instead. Single-quote `$`-bearing arguments, or put them in a `--batch` file (`{"at":"p0","markdown":"Energy is $E=mc^2$ now"}`), written with a QUOTED heredoc (`cat > edits.jsonl <<'EOF'` … `EOF`) — an unquoted heredoc expands `$` too. **The `ok` field appears in exactly one place: the `--verbose` success ack** (`{ok:true, operation, path, …}`). Without `-v`, success output is shaped for the next command: diff --git a/src/cli/delete/batch.ts b/src/cli/delete/batch.ts index f8ef8e9..d6e623d 100644 --- a/src/cli/delete/batch.ts +++ b/src/cli/delete/batch.ts @@ -4,6 +4,7 @@ import { LocatorResolveError, type XmlNode, } from "@core"; +import { fieldRemovalRefusal } from "@core/fields"; import { removeParagraphLine } from "@core/track-changes/replace"; import { readJsonlObjects } from "../parse-helpers"; import { @@ -123,6 +124,14 @@ export async function runDeleteBatch( refs.push({ locator: at, ref: blockRef }); } + // Judged against the whole set, so a batch removing every paragraph a + // field spans (both of its ends) goes through. + const refusal = fieldRemovalRefusal( + document.body, + refs.map((entry) => entry.ref.node), + ); + if (refusal) return fail(refusal.code, refusal.message, refusal.hint); + if (dryRun) { await respond({ operation: "delete", diff --git a/src/cli/delete/index.tsx b/src/cli/delete/index.tsx index aa91d55..f390a0b 100644 --- a/src/cli/delete/index.tsx +++ b/src/cli/delete/index.tsx @@ -5,6 +5,7 @@ import { removeInlineSectPr, } from "@core"; import { findContainingParagraph } from "@core/comments"; +import { fieldRemovalRefusal } from "@core/fields"; import { XmlNode } from "@core/parser"; import { applyTrackedRangeDelete, @@ -154,6 +155,12 @@ async function commitRangeDelete( const rangeRef = await resolveBlockRangeOrFail(document, opts.locator); if (typeof rangeRef === "number") return rangeRef; + const refusal = fieldRemovalRefusal( + document.body, + rangeRef.parent.slice(rangeRef.startIndex, rangeRef.endIndex + 1), + ); + if (refusal) return fail(refusal.code, refusal.message, refusal.hint); + const tracked = resolveTracked(document, opts.trackFlag); if (tracked) { try { @@ -255,6 +262,8 @@ async function commitBlockDelete( "Block reference is stale (parent does not contain it)", ); } + const refusal = fieldRemovalRefusal(document.body, [blockRef.node]); + if (refusal) return fail(refusal.code, refusal.message, refusal.hint); if (opts.dryRun) return respondDryRun(opts); diff --git a/src/cli/edit/batch.ts b/src/cli/edit/batch.ts index d7872c8..5435de8 100644 --- a/src/cli/edit/batch.ts +++ b/src/cli/edit/batch.ts @@ -18,6 +18,7 @@ import { resolveCellParagraphReference, } from "@core"; import type { ParagraphOptions } from "@core/blocks"; +import { fieldRemovalRefusal } from "@core/fields"; import type { XmlNode } from "@core/parser"; import { ensureCellEndsWithParagraph, type Grid } from "@core/table"; import { removeParagraphLine } from "@core/track-changes/replace"; @@ -128,6 +129,14 @@ export async function runEditBatch( throw error; } + const fieldRefusal = fieldRemovalRefusal( + document.body, + resolved.filter((entry) => entry.removesLine).map((entry) => entry.node), + ); + if (fieldRefusal) { + return fail(fieldRefusal.code, fieldRefusal.message, fieldRefusal.hint); + } + // Entries apply in memory even on --dry-run (only the save is skipped), so // the preview refuses exactly what the real call would — as `replace // --batch` already does. @@ -285,6 +294,9 @@ type ResolvedEntry = { paragraphLocator: string; /** Performs the mutation via the `Edit` lens. Throws `EditError`. */ apply: () => void; + /** A line removal (empty `text` / `delete: true`) — checked as one set + * against fields that span paragraphs before anything applies. */ + removesLine?: true; /** Filled by `apply` for a whole-paragraph content edit: the other authors * whose pending revisions it resolved (warned after the save, issue #17). */ resolvedAuthors: string[]; @@ -453,6 +465,7 @@ async function resolveEntry( ...(cellParent ? { cellParent } : {}), paragraphLocator, apply, + removesLine: true, resolvedAuthors: [], }; } diff --git a/src/cli/edit/index.tsx b/src/cli/edit/index.tsx index 3fef6ef..1a33b9c 100644 --- a/src/cli/edit/index.tsx +++ b/src/cli/edit/index.tsx @@ -19,6 +19,7 @@ import { type XmlNode, } from "@core"; import type { ParagraphOptions, TabStop } from "@core/blocks"; +import { fieldRemovalRefusal } from "@core/fields"; import { ensureCellEndsWithParagraph } from "@core/table"; import { removeParagraphLine } from "@core/track-changes/replace"; import type { parseArgs } from "util"; @@ -482,6 +483,8 @@ async function commitBlockEdit( // Empty `--text` on a whole paragraph removes the line (cell-safe — a // table cell's last paragraph is blanked, not deleted). Same path as // `docx delete --at pN`. + const refusal = fieldRemovalRefusal(document.body, [blockRef.node]); + if (refusal) return fail(refusal.code, refusal.message, refusal.hint); removeParagraphLine(document, blockRef, { track, author: opts.authorFlag, diff --git a/src/cli/replace/apply-matches.ts b/src/cli/replace/apply-matches.ts index 93c5db2..694fcfe 100644 --- a/src/cli/replace/apply-matches.ts +++ b/src/cli/replace/apply-matches.ts @@ -1,5 +1,6 @@ import type { Body } from "@core"; import { EquationParseError } from "@core/equation"; +import { fieldSpanRefusal, parseFields } from "@core/fields"; import { equationSpanRefusal, type ReplacementFormatting, @@ -12,7 +13,8 @@ import { fail } from "../respond"; /** Refuse the selected matches when any meets an equation in a way replace * can't carry out (`equationSpanRefusal`: a cut token, or a shell-mangled - * replacement that would delete one), else apply them — whole tokens + * replacement that would delete one) or cuts a field (`fieldSpanRefusal`: + * half a field would stay behind), else apply them — whole tokens * deleted, inside-token spans recompiled — and return undefined. Shared by * single-shot and `--batch` replace; `entry` prefixes a batch entry's error. * Every refusal comes before any mutation, and the caller applies even on @@ -38,14 +40,23 @@ export async function applyMatchesOrFail( equations: match.equations, matched: match.text, })); + const fields = parseFields([body.body], options.view); for (const replacement of replacements) { - const refusal = equationSpanRefusal( - spanLocator(replacement), - replacement.equations, - replacement.replacement, - "replace", - { pattern: replacement.matched }, - ); + const paragraph = body.resolveBlock(replacement.blockId).node; + const refusal = + equationSpanRefusal( + spanLocator(replacement), + replacement.equations, + replacement.replacement, + "replace", + { pattern: replacement.matched }, + ) ?? + fieldSpanRefusal( + fields, + { paragraph, offset: replacement.start }, + { paragraph, offset: replacement.end }, + spanLocator(replacement), + ); if (refusal) { return fail(refusal.code, `${prefix}${refusal.message}`, refusal.hint); } diff --git a/src/cli/respond.ts b/src/cli/respond.ts index a08f7a7..25721c0 100644 --- a/src/cli/respond.ts +++ b/src/cli/respond.ts @@ -39,6 +39,7 @@ export type ErrorCode = | "RENDER_FAILED" | "UPGRADE_FAILED" | "UNSUPPORTED" + | "FIELD_IN_PARAGRAPH" | "UNHANDLED"; // All CLI output goes through these sinks so in-process tests can capture it. @@ -295,6 +296,7 @@ function exitCodeFor(code: ErrorCode): number { case "RENDER_FAILED": case "UPGRADE_FAILED": case "UNSUPPORTED": + case "FIELD_IN_PARAGRAPH": case "UNHANDLED": return EXIT.GENERAL_ERROR; } diff --git a/src/cli/validate/index.ts b/src/cli/validate/index.ts index 0ed530c..6087748 100644 --- a/src/cli/validate/index.ts +++ b/src/cli/validate/index.ts @@ -33,6 +33,8 @@ graded by what Word does on open: warning a comment range with no commentReference (Word drops the comment) warning a comment with no marker in the document at all (Word deletes it) warning a reply whose thread parent names no comment (Word drops the link) + error an unbalanced field — a fldChar begin/separate/end with no match + (half a field: an orphan end, or a field never closed) These list under the part they concern, tagged "(consistency)"; one Word handles them harmlessly (a stale commentsIds entry, a duplicate durableId) is not reported, but --fix and every save clean it up anyway. diff --git a/src/core/edit/index.tsx b/src/core/edit/index.tsx index e60e403..c9c3db7 100644 --- a/src/core/edit/index.tsx +++ b/src/core/edit/index.tsx @@ -37,6 +37,12 @@ import { batchFileHint, shellMangledEquationLoss, } from "../equation/shell-mangle"; +import { + fieldSpanRefusal, + holdsFieldMarkup, + paragraphFieldRefusal, + parseFields, +} from "../fields"; import { equationOverlaps, equationSpanRefusal, @@ -176,6 +182,7 @@ export class Edit { "Block reference is stale (parent does not contain it)", ); } + this.refuseFieldRebuild([blockRef.node], this.blockIdOf(blockRef.node)); if (spec.kind === "runs") this.refuseRunsOverEquations(blockRef.node); const tracked = opts.track ?? this.document.isTrackChangesEnabled(); // Decided before anything mutates, so a refusal leaves the paragraph as @@ -408,6 +415,11 @@ export class Edit { if (refusal) { throw new EditError(refusal.code, refusal.message, refusal.hint); } + this.refuseFieldCut( + blockRef.node, + span, + `${blockId}:${span.start}-${span.end}`, + ); const tracked: TrackedReplaceOptions | undefined = (opts.track ?? this.document.isTrackChangesEnabled()) ? { @@ -433,6 +445,40 @@ export class Edit { } } + /** A whole-paragraph rebuild re-emits fresh runs from text, so a field in + * the rebuilt paragraphs would come back as frozen plain text — refuse + * before anything mutates (`paragraphFieldRefusal`, issue #42). */ + private refuseFieldRebuild(blocks: XmlNode[], locator: string): void { + if (!holdsFieldMarkup(blocks)) return; + const refusal = paragraphFieldRefusal( + parseFields([this.document.body.body]), + blocks, + locator, + ); + if (refusal) { + throw new EditError(refusal.code, refusal.message, refusal.hint); + } + } + + /** Refuse a `pN:S-E` span that cuts a field (`fieldSpanRefusal`). */ + private refuseFieldCut( + paragraph: XmlNode, + span: { start: number; end: number }, + locator: string, + ): void { + // A span can only cut a field whose marker sits in this paragraph. + if (!holdsFieldMarkup([paragraph])) return; + const refusal = fieldSpanRefusal( + parseFields([this.document.body.body]), + { paragraph, offset: span.start }, + { paragraph, offset: span.end }, + locator, + ); + if (refusal) { + throw new EditError(refusal.code, refusal.message, refusal.hint); + } + } + /** `--runs` states each run byte for byte and has no way to say where an * equation goes, so rewriting a paragraph that holds one would drop it * silently. Refuse; `--text` and markdown both carry it as its `$…$` @@ -671,6 +717,16 @@ export class Edit { if (spec.kind === "code") { ensureCodeBlockStyles(this.document, spec.language); } + const [first, last] = [ + rangeRef.parent[rangeRef.startIndex], + rangeRef.parent[rangeRef.endIndex], + ]; + this.refuseFieldRebuild( + rangeRef.parent.slice(rangeRef.startIndex, rangeRef.endIndex + 1), + first && last + ? `${this.blockIdOf(first)}-${this.blockIdOf(last)}` + : "the range", + ); const tracked = opts.track ?? this.document.isTrackChangesEnabled(); if (tracked) { @@ -780,6 +836,7 @@ export type EditErrorCode = | "INVALID_LOCATOR" | "BLOCK_NOT_FOUND" | "TRACKED_CHANGE_CONFLICT" + | "FIELD_IN_PARAGRAPH" | "UNSUPPORTED"; export class EditError extends Error { diff --git a/src/core/fields/audit.ts b/src/core/fields/audit.ts new file mode 100644 index 0000000..8b4dcd1 --- /dev/null +++ b/src/core/fields/audit.ts @@ -0,0 +1,44 @@ +import type { PackagePart } from "../raw/package-consistency"; +import { parseFields } from "./parse"; + +/** `validate`'s field-balance check: every `` the begin/separate/ + * end stack can't pair, per story part. A part that holds several stories + * (footnotes, endnotes, comments) is parsed one story per note/comment — a + * field never spans two. Nothing is fixable: which half to drop, or where the + * missing end belongs, isn't knowable from the XML. */ +export function auditFieldBalance(parts: PackagePart[]): FieldIssue[] { + const issues: FieldIssue[] = []; + for (const part of parts) { + const stories = MULTI_STORY_ROOTS.has(part.root.tag) + ? part.root.children.filter((child) => !child.isText) + : [part.root]; + const { strays } = parseFields(stories, "current"); + if (strays.length === 0) continue; + const counts = new Map(); + for (const stray of strays) { + const type = stray.node.getAttribute("w:fldCharType") ?? "?"; + counts.set(type, (counts.get(type) ?? 0) + 1); + } + const described = [...counts] + .map(([type, count]) => `${count} "${type}"`) + .join(", "); + issues.push({ + part: part.name, + message: `unbalanced field: ${described} fldChar marker${strays.length === 1 ? "" : "s"} with no matching begin/end — half a field (Word may show the instruction as text, or swallow the text after an unclosed field)`, + severity: "error", + fixable: false, + hint: "Find the paragraph with `docx raw get FILE --at pN` (look for w:fldChar) and repair it with `docx raw edit`, or re-insert the field in Word.", + }); + } + return issues; +} + +const MULTI_STORY_ROOTS = new Set(["w:footnotes", "w:endnotes", "w:comments"]); + +type FieldIssue = { + part: string; + message: string; + severity: "error"; + fixable: false; + hint: string; +}; diff --git a/src/core/fields/index.ts b/src/core/fields/index.ts new file mode 100644 index 0000000..c5361db --- /dev/null +++ b/src/core/fields/index.ts @@ -0,0 +1,9 @@ +export { holdsFieldMarkup, type ParsedFields, parseFields } from "./parse"; +export { + type FieldRefusal, + fieldRemovalRefusal, + fieldSpanRefusal, + paragraphFieldRefusal, + type SpanEnd, +} from "./refusals"; +export { removeCoveredFields } from "./remove-covered-fields"; diff --git a/src/core/fields/parse.ts b/src/core/fields/parse.ts new file mode 100644 index 0000000..1d311e5 --- /dev/null +++ b/src/core/fields/parse.ts @@ -0,0 +1,398 @@ +import type { FindView } from "../find"; +import { isWrapperVisibleInView } from "../find/replace-span"; +import { + alternateContentBranch, + collectTextBoxContents, + isAlternateContent, +} from "../mc"; +import { + equationWidth, + inlineMarkerWidth, + isEquation, + isRunBearingWrapper, + wrapperContentNode, + type XmlNode, +} from "../parser"; + +/** Every field in a story, read straight off the XML — no AST involved. + * + * A COMPLEX field is three zero-width `` markers that form one + * unit — `begin`, an optional `separate`, `end` — with its instruction in the + * `` runs between begin and separate and its cached result in the + * runs between separate and end. The markers can sit in different runs, + * different wrappers, even different paragraphs (a TOC opens in its first + * entry and closes after its last), and fields nest, so they're paired with a + * stack per story — the body, each text box's ``, each + * header/footer/note/comment. A SIMPLE field (``) is one + * wrapper holding its result runs. + * + * Each marker carries its position in the offset space every span walker + * uses (`collectRunSlots`/`sumVisibleTextLength` in find/replace-span.tsx): + * the ordinal of its paragraph in document order plus the paragraph-local + * offset in `view`. Markers the stack can't pair (an `end` with no open + * field, a `begin` never closed) are `strays` — a damaged file. */ +export function parseFields( + storyRoots: XmlNode[], + view: FindView = "accepted", +): ParsedFields { + const state: WalkState = { + view, + ordinal: 0, + fields: [], + strays: [], + stack: [], + stories: [], + ordinals: new Map(), + }; + for (const root of storyRoots) walkStory(state, root); + // Text-box stories are queued as they're met (inside an anchor's run) and + // parsed afterward, each with its own stack — a field never spans a story. + while (state.stories.length > 0) { + const story = state.stories.shift(); + if (story) walkStory(state, story); + } + return { + fields: state.fields, + strays: state.strays, + ordinals: state.ordinals, + }; +} + +function walkStory(state: WalkState, root: XmlNode): void { + const outer = state.stack; + state.stack = []; + walkBlocks(state, root.children); + for (const open of state.stack) { + state.strays.push(open.begin); + } + state.stack = outer; +} + +/** Block-level descent in document order: paragraphs, then anything that + * holds paragraphs (tables, rows, cells, content controls, custom XML). A + * block-level `` is read through its ONE branch. */ +function walkBlocks(state: WalkState, children: XmlNode[]): void { + for (const child of children) { + if (child.isText) continue; + if (child.tag === "w:p") { + walkParagraph(state, child); + continue; + } + if (isAlternateContent(child)) { + const branch = alternateContentBranch(child); + if (branch) walkBlocks(state, branch.children); + continue; + } + walkBlocks(state, child.children); + } +} + +function walkParagraph(state: WalkState, paragraph: XmlNode): void { + const cursor: Cursor = { + paragraph, + ordinal: state.ordinal++, + offset: 0, + }; + state.ordinals.set(paragraph, cursor.ordinal); + walkRunContainer(state, cursor, paragraph, { visible: true, measured: true }); +} + +/** Walk a run container (the paragraph, or a run-bearing wrapper's content + * node) the way the span walkers do: runs by `runTextLength`, equations by + * their token width, visible wrappers descended, hidden ones contributing no + * width. Anything else (a run-level content control, custom XML) is opaque to + * the walkers — zero width — but its markers still count for pairing. */ +function walkRunContainer( + state: WalkState, + cursor: Cursor, + container: XmlNode, + scope: Scope, +): void { + for (const child of container.children) { + if (child.isText) continue; + if (child.tag === "w:r") { + walkRun(state, cursor, container, child, scope); + continue; + } + if (isEquation(child)) { + if (counts(scope)) cursor.offset += equationWidth(child, state.view); + continue; + } + if (child.tag === "w:fldSimple") { + walkSimpleField(state, cursor, container, child, scope); + continue; + } + if (isRunBearingWrapper(child.tag)) { + walkRunContainer(state, cursor, wrapperContentNode(child), { + ...scope, + visible: scope.visible && isWrapperVisibleInView(child.tag, state.view), + }); + continue; + } + if (child.tag === "w:pPr" || child.tag === "w:rPr") continue; + walkRunContainer(state, cursor, child, { ...scope, measured: false }); + } +} + +/** Whether text met in this scope advances the offset: visible in the view + * AND inside containers the span walkers measure. */ +const counts = (scope: Scope): boolean => scope.visible && scope.measured; + +function walkSimpleField( + state: WalkState, + cursor: Cursor, + container: XmlNode, + node: XmlNode, + scope: Scope, +): void { + const start = position(cursor); + const resultStart = cursor.offset; + walkRunContainer(state, cursor, node, scope); + state.fields.push({ + kind: "simple", + node, + container, + paragraph: cursor.paragraph, + start, + end: position(cursor), + visible: scope.visible, + instruction: (node.getAttribute("w:instr") ?? "").trim(), + result: resultText(node), + resultWidth: cursor.offset - resultStart, + }); +} + +function walkRun( + state: WalkState, + cursor: Cursor, + container: XmlNode, + run: XmlNode, + scope: Scope, +): void { + const { visible } = scope; + for (const child of run.children) { + if (child.isText) continue; + if (child.tag === "w:fldChar") { + const marker: FieldMarker = { + node: child, + run, + container, + paragraph: cursor.paragraph, + position: position(cursor), + visible, + }; + placeMarker(state, marker, child.getAttribute("w:fldCharType")); + continue; + } + if (child.tag === "w:instrText" || child.tag === "w:delInstrText") { + const open = state.stack.at(-1); + if (open && !open.separate) { + open.instructionRuns.push({ + node: child, + run, + container, + paragraph: cursor.paragraph, + position: position(cursor), + visible, + }); + open.instructionText += child.collectText(); + } + continue; + } + if (child.tag === "w:t" || child.tag === "w:delText") { + const text = child.collectText(); + for (const open of state.stack) { + if (open.separate) open.resultText += text; + } + if (counts(scope)) cursor.offset += text.length; + continue; + } + if (child.tag === "w:rPr") continue; + for (const story of collectTextBoxContents(child)) { + state.stories.push(story); + } + if (counts(scope)) cursor.offset += inlineMarkerWidth(child); + } +} + +/** Pair a `` with the open field it belongs to. */ +function placeMarker( + state: WalkState, + marker: FieldMarker, + type: string | undefined, +): void { + if (type === "begin") { + state.stack.push({ + begin: marker, + instructionRuns: [], + instructionText: "", + resultText: "", + }); + return; + } + const open = state.stack.at(-1); + if (!open) { + state.strays.push(marker); + return; + } + if (type === "separate") { + if (open.separate) state.strays.push(marker); + else open.separate = marker; + return; + } + if (type !== "end") return; + state.stack.pop(); + state.fields.push({ + kind: "complex", + begin: open.begin, + ...(open.separate ? { separate: open.separate } : {}), + end: marker, + instructionRuns: open.instructionRuns, + instruction: open.instructionText.trim(), + result: open.resultText, + }); +} + +function position(cursor: Cursor): FieldPosition { + return { ordinal: cursor.ordinal, offset: cursor.offset }; +} + +function resultText(node: XmlNode): string { + let out = ""; + for (const child of node.children) { + if (child.tag === "w:t" || child.tag === "w:delText") { + out += child.collectText(); + continue; + } + if (!child.isText) out += resultText(child); + } + return out; +} + +/** Whether any field markup sits anywhere under `nodes` — the cheap test that + * lets a mutator skip `parseFields` (a whole-document walk) for the common + * paragraph with no field at all. */ +export function holdsFieldMarkup(nodes: XmlNode[]): boolean { + return nodes.some( + (node) => + FIELD_TAGS.has(node.tag) || + [...FIELD_TAGS].some((tag) => node.findDescendant(tag) !== undefined), + ); +} + +const FIELD_TAGS = new Set([ + "w:fldChar", + "w:fldSimple", + "w:instrText", + "w:delInstrText", +]); + +/** Every paragraph that holds one of the field's markers (its fldChars and + * instruction runs, or the `` itself) — the paragraphs a + * rebuild or removal can't touch without touching the field. */ +export function fieldParagraphs(field: Field): Set { + if (field.kind === "simple") return new Set([field.paragraph]); + return new Set(fieldMarkers(field).map((marker) => marker.paragraph)); +} + +/** A complex field's markers in document order: begin, instruction runs, + * separate, end. */ +export function fieldMarkers(field: ComplexField): FieldMarker[] { + return [ + field.begin, + ...field.instructionRuns, + ...(field.separate ? [field.separate] : []), + field.end, + ]; +} + +/** The field's kind, as Word names it: the instruction's first word (`REF`, + * `PAGE`, `TOC`, `HYPERLINK`, …), upper-cased. */ +export function fieldType(field: Field): string { + return (field.instruction.split(/\s+/)[0] ?? "").toUpperCase() || "FIELD"; +} + +/** Whether any of the field's markers is visible in the view the positions + * were measured in — a field wholly inside a pending deletion has no width + * and nothing for an edit to carry. */ +export function isFieldVisible(field: Field): boolean { + if (field.kind === "simple") return field.visible; + return fieldMarkers(field).some((marker) => marker.visible); +} + +/** `a` before `b` in document order (`-1`), at the same point (`0`), after (`1`). */ +export function comparePositions(a: FieldPosition, b: FieldPosition): number { + if (a.ordinal !== b.ordinal) return a.ordinal < b.ordinal ? -1 : 1; + if (a.offset !== b.offset) return a.offset < b.offset ? -1 : 1; + return 0; +} + +export type ParsedFields = { + fields: Field[]; + strays: FieldMarker[]; + /** Each walked paragraph's document-order ordinal — how a caller turns a + * `pN:S-E` span into the positions fields are measured in. */ + ordinals: Map; +}; + +export type Field = ComplexField | SimpleField; + +export type ComplexField = { + kind: "complex"; + begin: FieldMarker; + separate?: FieldMarker; + end: FieldMarker; + instructionRuns: FieldMarker[]; + instruction: string; + /** The cached result's text (every view — for naming the field). */ + result: string; +}; + +export type SimpleField = { + kind: "simple"; + node: XmlNode; + container: XmlNode; + paragraph: XmlNode; + start: FieldPosition; + end: FieldPosition; + visible: boolean; + instruction: string; + result: string; + resultWidth: number; +}; + +/** A zero-width field marker (`` or ``), with the run + * that holds it and the node whose child list holds that run. */ +export type FieldMarker = { + node: XmlNode; + run: XmlNode; + container: XmlNode; + paragraph: XmlNode; + position: FieldPosition; + /** Inside no view-hidden wrapper. */ + visible: boolean; +}; + +export type FieldPosition = { ordinal: number; offset: number }; + +type OpenField = { + begin: FieldMarker; + separate?: FieldMarker; + instructionRuns: FieldMarker[]; + instructionText: string; + resultText: string; +}; + +type Scope = { visible: boolean; measured: boolean }; + +type Cursor = { paragraph: XmlNode; ordinal: number; offset: number }; + +type WalkState = { + view: FindView; + ordinal: number; + fields: Field[]; + strays: FieldMarker[]; + stack: OpenField[]; + stories: XmlNode[]; + ordinals: Map; +}; diff --git a/src/core/fields/refusals.ts b/src/core/fields/refusals.ts new file mode 100644 index 0000000..7393796 --- /dev/null +++ b/src/core/fields/refusals.ts @@ -0,0 +1,195 @@ +import type { Body } from "../ast/document/body"; +import type { XmlNode } from "../parser"; +import { + comparePositions, + type Field, + type FieldPosition, + fieldParagraphs, + fieldType, + holdsFieldMarkup, + isFieldVisible, + type ParsedFields, + parseFields, +} from "./parse"; + +/** A field is atomic structure: a mutation carries it through intact, removes + * it whole, or refuses before writing anything (issue #42). These are the + * three refusals, each `null` when the mutation is safe. */ + +/** Refuse a whole-paragraph rebuild (`edit` markdown/`--text`/`--runs`/code, + * a `pN-pM` range edit) of paragraphs that hold a field: the rebuild emits + * fresh runs from text, so every field in them would come back as frozen + * plain text — instruction gone, exit 0, `read` unchanged. */ +export function paragraphFieldRefusal( + parsed: ParsedFields, + blocks: XmlNode[], + locator: string, +): FieldRefusal | null { + const targets = paragraphsIn(blocks); + const held = parsed.fields.filter( + (field) => + isFieldVisible(field) && + [...fieldParagraphs(field)].some((paragraph) => targets.has(paragraph)), + ); + if (held.length === 0) return null; + const count = `${held.length} field${held.length === 1 ? "" : "s"}`; + return { + code: "FIELD_IN_PARAGRAPH", + message: `${locator} contains ${count} (${describeFields(held)}) that this edit would turn into plain text. Nothing was changed.`, + hint: `A field (cross-reference, page number, TOC entry, merge field, …) is kept only by edits around it: change the text beside it with a span edit (\`docx find FILE "TEXT"\` gives the pN:S-E locator for \`docx edit FILE --at pN:S-E "…"\`) or with \`docx replace\` — fields outside the span are kept, a span inside a field's result edits that result, and a span covering a whole field deletes it.`, + }; +} + +/** Refuse a span that CUTS a field: one covering some of {begin … end} but + * not all of it. A span strictly inside the result (edits the cached text) + * and one covering the whole field (removes it — see `removeCoveredFields`) + * are both fine. The `equationSpanRefusal` analog. */ +export function fieldSpanRefusal( + parsed: ParsedFields, + start: SpanEnd, + end: SpanEnd, + locator: string, +): FieldRefusal | null { + const span = positionSpan(parsed, start, end); + if (!span) return null; + const cut = parsed.fields.find( + (field) => isFieldVisible(field) && spanRelation(field, span) === "cut", + ); + if (!cut) return null; + return { + code: "UNSUPPORTED", + message: `${locator}: the span cuts a field (${describeFields([cut])}) — it covers part of the field but not all of it, which would leave half a field behind. Nothing was changed.`, + hint: `A field is one unit whose visible text is its result${cut.result ? ` (${JSON.stringify(cut.result)})` : ""}. Match only text beside it, only text inside its result to edit that result, or the whole result plus text on either side to delete the field.`, + }; +} + +/** Refuse removing paragraphs (`delete --at pN`/`pN-pM`, `edit --at pN + * --text ""`, a batch's line removals) that hold one end of a field but not + * the other — a TOC-shaped field that opens in one paragraph and closes in + * another. Removing a paragraph that holds a whole field is fine: the field + * goes with it. Positions don't matter here, so any view's parse will do. */ +export function fieldRemovalRefusal( + body: Body, + removedBlocks: XmlNode[], +): FieldRefusal | null { + if (!holdsFieldMarkup(removedBlocks)) return null; + const removed = paragraphsIn(removedBlocks); + const locatorOf = (paragraph: XmlNode): string => + body.findBlockByNode(paragraph)?.id ?? "a paragraph"; + for (const field of parseFields([body.body]).fields) { + const paragraphs = [...fieldParagraphs(field)]; + const taken = paragraphs.filter((paragraph) => removed.has(paragraph)); + if (taken.length === 0 || taken.length === paragraphs.length) continue; + const first = paragraphs[0]; + const last = paragraphs.at(-1); + const extent = + first && last ? `${locatorOf(first)}–${locatorOf(last)}` : ""; + return { + code: "UNSUPPORTED", + message: `Removing ${taken.map(locatorOf).join(", ")} would split a field (${describeFields([field])}) that spans ${extent}: the removal takes one end of it and leaves the other, half a field. Nothing was changed.`, + hint: `Remove every paragraph the field spans in one call (\`docx delete FILE --at ${extent.replace("–", "-")}\`), or change the text inside it with a span edit instead.`, + }; + } + return null; +} + +/** A span given as paragraph-local offsets (`pN:S-E`, or across paragraphs + * `pN:S-pM:E`), in the positions `parsed` measured — null when a paragraph + * wasn't walked (it's in no story `parsed` covers). */ +export function positionSpan( + parsed: ParsedFields, + start: SpanEnd, + end: SpanEnd, +): PositionSpan | null { + const startOrdinal = parsed.ordinals.get(start.paragraph); + const endOrdinal = parsed.ordinals.get(end.paragraph); + if (startOrdinal === undefined || endOrdinal === undefined) return null; + return { + start: { ordinal: startOrdinal, offset: start.offset }, + end: { ordinal: endOrdinal, offset: end.offset }, + }; +} + +/** How a span meets a field, positions compared in document order. A field + * occupies [begin, end] (zero-width markers) and its result [separate, end]. + * `inside` wins a tie: a span exactly over the result edits it, keeping the + * field — the walkers leave a marker sitting on a span edge in place. */ +export function spanRelation( + field: Field, + span: PositionSpan, +): "disjoint" | "inside" | "covered" | "cut" { + const begin = field.kind === "simple" ? field.start : field.begin.position; + const end = field.kind === "simple" ? field.end : field.end.position; + const resultStart = + field.kind === "simple" + ? field.start + : (field.separate?.position ?? field.begin.position); + const overlaps = + comparePositions(span.start, end) < 0 && + comparePositions(span.end, begin) > 0; + if (!overlaps) return "disjoint"; + if ( + comparePositions(resultStart, span.start) <= 0 && + comparePositions(span.end, end) <= 0 + ) { + return "inside"; + } + if ( + comparePositions(span.start, begin) <= 0 && + comparePositions(end, span.end) <= 0 + ) { + return "covered"; + } + return "cut"; +} + +/** `REF ×3: "1", "3", "4]"` — each kind with a count and the cached results, + * the text an agent sees in `read`. */ +function describeFields(fields: Field[]): string { + const byType = new Map(); + for (const field of fields) { + const type = fieldType(field); + byType.set(type, [...(byType.get(type) ?? []), field]); + } + return [...byType] + .map(([type, group]) => { + const count = group.length > 1 ? ` ×${group.length}` : ""; + const results = group + .map((field) => JSON.stringify(truncate(field.result))) + .join(", "); + return `${type}${count}: ${results}`; + }) + .join("; "); +} + +function truncate(text: string): string { + return text.length > 40 ? `${text.slice(0, 39)}…` : text; +} + +/** Every `` in (or being) one of `blocks` — a removed or rebuilt table + * takes its cells' paragraphs with it. A text box's story is NOT the anchor + * paragraph's content: removal re-anchors the box and a rebuild lifts it out + * and puts it back, so its paragraphs (and their fields) survive both. */ +function paragraphsIn(blocks: XmlNode[]): Set { + const out = new Set(); + const visit = (node: XmlNode): void => { + if (node.tag === "w:txbxContent") return; + if (node.tag === "w:p") out.add(node); + for (const child of node.children) { + if (!child.isText) visit(child); + } + }; + for (const block of blocks) visit(block); + return out; +} + +export type PositionSpan = { start: FieldPosition; end: FieldPosition }; + +/** One end of a span: a paragraph and a paragraph-local offset. */ +export type SpanEnd = { paragraph: XmlNode; offset: number }; + +export type FieldRefusal = { + code: "FIELD_IN_PARAGRAPH" | "UNSUPPORTED"; + message: string; + hint: string; +}; diff --git a/src/core/fields/remove-covered-fields.tsx b/src/core/fields/remove-covered-fields.tsx new file mode 100644 index 0000000..3ff015f --- /dev/null +++ b/src/core/fields/remove-covered-fields.tsx @@ -0,0 +1,107 @@ +import type { FindView } from "../find"; +import { XmlNode } from "../parser"; +import { convertTextToDelText, type TrackedMeta } from "../track-changes"; +import { Del } from "../track-changes/emit"; +import { + type FieldMarker, + fieldMarkers, + type ParsedFields, + parseFields, +} from "./parse"; +import { positionSpan, type SpanEnd, spanRelation } from "./refusals"; + +/** `removeCoveredFields` for a span inside one paragraph (`pN:S-E`, a + * `replace` match): a field the span covers whole has both ends in this + * paragraph, so the paragraph alone is parsed — LIFO pairing makes its local + * pairs exactly the document's. */ +export function removeFieldsCoveredInParagraph( + paragraph: XmlNode, + span: { start: number; end: number }, + view: FindView, + mintMeta?: () => TrackedMeta, +): void { + const story = new XmlNode("w:body"); + story.children = [paragraph]; + removeCoveredFields( + parseFields([story], view), + { paragraph, offset: span.start }, + { paragraph, offset: span.end }, + mintMeta, + ); +} + +/** Before a span walker deletes `span`, take out the markers of every complex + * field the span COVERS whole, so the field goes as one unit. The walkers + * own text, not structure: a zero-width `` sitting on a span edge + * stays where it is, so a span from before a field to the end of its result + * took the begin and separate and left the `end` behind — an orphan + * `validate` couldn't see (issue #42). The result runs are text inside the + * span; the walker deletes those itself. + * + * Untracked, the markers are dropped. Tracked (`mintMeta`), each marker's run + * is wrapped in its own `` (`convertTextToDelText` renames + * `` to ``; reject renames it back), so Accept All removes the + * field whole and Reject All restores it. A marker sharing its run with other + * content is first moved into a run of its own. Markers already hidden in a + * pending deletion are left alone. A covered `` needs nothing: + * the walker removes it as the wrapper of the text it deletes. */ +export function removeCoveredFields( + parsed: ParsedFields, + start: SpanEnd, + end: SpanEnd, + mintMeta?: () => TrackedMeta, +): void { + const span = positionSpan(parsed, start, end); + if (!span) return; + const markers = parsed.fields.flatMap((field) => + field.kind === "complex" && spanRelation(field, span) === "covered" + ? fieldMarkers(field).filter((marker) => marker.visible) + : [], + ); + const byRun = new Map(); + for (const marker of markers) { + byRun.set(marker.run, [...(byRun.get(marker.run) ?? []), marker]); + } + for (const [run, runMarkers] of byRun) { + const [first] = runMarkers; + if (!first) continue; + const target = isolateMarkers( + run, + first.container, + new Set(runMarkers.map((marker) => marker.node)), + ); + const index = first.container.children.indexOf(target); + if (index === -1) continue; + if (!mintMeta) { + first.container.children.splice(index, 1); + continue; + } + first.container.children.splice( + index, + 1, + {convertTextToDelText(target)}, + ); + } +} + +/** The run holding exactly `nodes` (plus its rPr): `run` itself when they're + * all it holds, else a new run — same attributes and rPr — that they're moved + * into, placed just before `run` in `container`. */ +function isolateMarkers( + run: XmlNode, + container: XmlNode, + nodes: Set, +): XmlNode { + const content = run.children.filter( + (child) => !child.isText && child.tag !== "w:rPr", + ); + if (content.every((child) => nodes.has(child))) return run; + const isolated = new XmlNode("w:r", { ...run.attributes }); + const properties = run.findChild("w:rPr"); + if (properties) isolated.children.push(properties.clone()); + isolated.children.push(...content.filter((child) => nodes.has(child))); + run.children = run.children.filter((child) => !nodes.has(child)); + const index = container.children.indexOf(run); + container.children.splice(Math.max(index, 0), 0, isolated); + return isolated; +} diff --git a/src/core/find/index.ts b/src/core/find/index.ts index 54dbec6..070ea83 100644 --- a/src/core/find/index.ts +++ b/src/core/find/index.ts @@ -6,6 +6,13 @@ import type { TrackedChange, } from "../ast/types"; import { formatEquationToken } from "../equation/token"; +import { + type FieldRefusal, + fieldSpanRefusal, + parseFields, + removeCoveredFields, + type SpanEnd, +} from "../fields"; import { isRevisionVisible, type RevisionView, @@ -299,7 +306,7 @@ export type AcrossReplaceResult = { /** Set when a match can't be applied over the equations it touches (it * cuts one, or would put a paragraph break inside one — see * `equationSpanRefusal`). When set, NOTHING was applied. */ - refusal?: EquationRefusal | undefined; + refusal?: EquationRefusal | FieldRefusal | undefined; normalizedQuery?: string; normalizationApplied?: NormalizationKind[]; }; @@ -328,19 +335,32 @@ export function applyAcrossReplace( ); const replaced = selectMatches(matches, spec); const expand = replacementExpander(spec); + const fields = parseFields([doc.body], spec.view); const refusal = replaced - .map((match) => - equationSpanRefusal( - acrossLocator(match), - match.equations, - expand(match.text), - "replace", - { multiline: true, pattern: match.text }, - ), + .map( + (match) => + equationSpanRefusal( + acrossLocator(match), + match.equations, + expand(match.text), + "replace", + { multiline: true, pattern: match.text }, + ) ?? + fieldSpanRefusal( + fields, + ...matchEnds(doc, match), + acrossLocator(match), + ), ) .find((found) => found !== null); if (!dryRun && !refusal) { for (const match of [...replaced].reverse()) { + // A field the match covers goes whole: its markers out first (the + // boundary paragraphs' edge markers would otherwise stay behind). + removeCoveredFields( + parseFields([doc.body], spec.view), + ...matchEnds(doc, match), + ); replaceAcrossParagraphs( doc, match, @@ -353,6 +373,20 @@ export function applyAcrossReplace( return { totalMatches: matches.length, replaced, refusal, ...normalization }; } +/** A cross-paragraph match's two ends, as the fields module takes them. */ +function matchEnds(doc: Body, match: ParagraphSpanMatch): [SpanEnd, SpanEnd] { + return [ + { + paragraph: doc.resolveBlock(match.startBlockId).node, + offset: match.startOffset, + }, + { + paragraph: doc.resolveBlock(match.endBlockId).node, + offset: match.endOffset, + }, + ]; +} + /** `pN:S-E`, or `pN:S-pM:E` across paragraphs — `spanLocator`'s spelling. */ function acrossLocator(match: ParagraphSpanMatch): string { return match.startBlockId === match.endBlockId diff --git a/src/core/find/replace-across.tsx b/src/core/find/replace-across.tsx index 37f9817..f6da338 100644 --- a/src/core/find/replace-across.tsx +++ b/src/core/find/replace-across.tsx @@ -183,7 +183,10 @@ function splitChildrenAt( before.push(child); continue; } - if (child.tag === "w:r") { + // A run with no width (a field's ``, a lone drawing or note + // reference) is a marker: it goes by `edge` below, like the others, so a + // field marker sitting exactly on a span edge stays outside the cut. + if (child.tag === "w:r" && runTextLength(child) > 0) { const length = runTextLength(child); if (cursor + length <= offset) { before.push(child); @@ -230,8 +233,8 @@ function splitChildrenAt( cursor += length; continue; } - // Zero-width: comment/bookmark markers, view-invisible tracked wrappers, - // a hidden equation. + // Zero-width: comment/bookmark markers, empty runs, view-invisible + // tracked wrappers, a hidden equation. const inBefore = cursor < offset || (cursor === offset && edge === "before"); (inBefore ? before : after).push(child); diff --git a/src/core/find/replace-matches.tsx b/src/core/find/replace-matches.tsx index ada6398..5630211 100644 --- a/src/core/find/replace-matches.tsx +++ b/src/core/find/replace-matches.tsx @@ -5,6 +5,7 @@ import { shellMangledEquationLoss, } from "../equation/shell-mangle"; import { formatEquationToken } from "../equation/token"; +import { removeFieldsCoveredInParagraph } from "../fields/remove-covered-fields"; import type { XmlNode } from "../parser"; import { Del, Ins } from "../track-changes/emit"; import type { EquationOverlap, FindView } from "./index"; @@ -93,15 +94,25 @@ export function replaceInParagraph( } operations.push({ start: match.start, - apply: () => + apply: () => { + const span = { start: match.start, end: match.end }; + // A field the span covers goes whole — its markers first, then + // the walker takes its result with the rest of the text. + removeFieldsCoveredInParagraph( + paragraph, + span, + view, + tracked && (() => mintMeta(tracked)), + ); replaceSpanInParagraph( paragraph, - { start: match.start, end: match.end }, + span, match.replacement, tracked, view, formatting, - ), + ); + }, }); } for (const { overlap, changes } of rewrites.values()) { diff --git a/src/core/find/replace-span.tsx b/src/core/find/replace-span.tsx index 3be0b80..78f3006 100644 --- a/src/core/find/replace-span.tsx +++ b/src/core/find/replace-span.tsx @@ -814,6 +814,7 @@ function mintMeta(tracked: TrackedReplaceOptions): TrackedMeta { function convertRunTextToDelText(run: XmlNode): void { for (const child of run.children) { if (child.tag === "w:t") child.tag = "w:delText"; + if (child.tag === "w:instrText") child.tag = "w:delInstrText"; } } diff --git a/src/core/markdown/inline.tsx b/src/core/markdown/inline.tsx index 77d6600..3b2290c 100644 --- a/src/core/markdown/inline.tsx +++ b/src/core/markdown/inline.tsx @@ -483,6 +483,7 @@ function wrapTracked( function renameTextToDelText(nodes: XmlNode[]): void { for (const node of nodes) { if (node.tag === "w:t") node.tag = "w:delText"; + if (node.tag === "w:instrText") node.tag = "w:delInstrText"; if (node.children.length > 0) renameTextToDelText(node.children); } } diff --git a/src/core/raw/package-consistency.ts b/src/core/raw/package-consistency.ts index 685129b..4c0a84b 100644 --- a/src/core/raw/package-consistency.ts +++ b/src/core/raw/package-consistency.ts @@ -6,6 +6,7 @@ import { threadRootsOf, } from "../ast/document/comments/reconcile-sidecars"; import { auditCommentMarkers, type StoryPart } from "../comments/marker-audit"; +import { auditFieldBalance } from "../fields/audit"; import type { XmlNode } from "../parser"; /** Package-level consistency audit — the cross-part invariants no per-part @@ -26,6 +27,10 @@ import type { XmlNode } from "../parser"; * handles them harmlessly). * - **Thread links** — `auditThreadLinks`: a `w15:paraIdParent` naming no * comment (warning; Word drops the thread). + * - **Field balance** — `auditFieldBalance`: a `` the begin/ + * separate/end stack can't pair (an orphan `end`, a field never closed) is + * an ERROR — the XSDs can't see it, and an unclosed field swallows the text + * after it as instruction. * * Because the reports come from the very functions `validate --fix` (and * every save) applies, a finding is marked fixable exactly when the fix cures @@ -62,19 +67,26 @@ export function checkPackageConsistency( findings.push(...reconcileSidecars(cloneRoots(roots))); findings.push(...auditThreadLinks(roots)); } - return findings.flatMap((finding) => - finding.severity === "silent" - ? [] - : [ - { - part: finding.part, - message: finding.message, - severity: finding.severity, - fixable: finding.fix !== undefined, - ...(finding.hint ? { hint: finding.hint } : {}), - }, - ], - ); + const fieldIssues = auditFieldBalance([ + ...stories, + ...parts.filter((part) => part.root.tag === "w:comments"), + ]); + return [ + ...fieldIssues, + ...findings.flatMap((finding) => + finding.severity === "silent" + ? [] + : [ + { + part: finding.part, + message: finding.message, + severity: finding.severity, + fixable: finding.fix !== undefined, + ...(finding.hint ? { hint: finding.hint } : {}), + }, + ], + ), + ]; } const STORY_ROOTS = new Set([ diff --git a/src/core/track-changes/apply.ts b/src/core/track-changes/apply.ts index 21af655..b53ede5 100644 --- a/src/core/track-changes/apply.ts +++ b/src/core/track-changes/apply.ts @@ -639,6 +639,7 @@ function renameDelTextToText(node: XmlNode): void { // (see `mutateTextToDelText`), so it isn't renamed back either. if (node.tag === "w:txbxContent") return; if (node.tag === "w:delText") node.tag = "w:t"; + if (node.tag === "w:delInstrText") node.tag = "w:instrText"; for (const child of node.children) renameDelTextToText(child); } diff --git a/src/core/track-changes/index.tsx b/src/core/track-changes/index.tsx index b9563fd..5e34f26 100644 --- a/src/core/track-changes/index.tsx +++ b/src/core/track-changes/index.tsx @@ -365,7 +365,9 @@ function walkXml(nodes: XmlNode[], visit: (node: XmlNode) => void): void { } } -/** Rewrite `` → `` under a tracked deletion. A text box's +/** Rewrite `` → `` (and a field's `` → + * ``, its deleted-instruction twin) under a tracked + * deletion. A text box's * story (``) is NOT descended: the anchor run's `` * already deletes the whole shape as one unit (what Word records), and * rewriting the story would leave bare delText outside any `` that @@ -374,6 +376,7 @@ function mutateTextToDelText(nodes: XmlNode[]): void { for (const node of nodes) { if (node.tag === "w:txbxContent") continue; if (node.tag === "w:t") node.tag = "w:delText"; + if (node.tag === "w:instrText") node.tag = "w:delInstrText"; mutateTextToDelText(node.children); } } diff --git a/tests/cli/fields.test.ts b/tests/cli/fields.test.ts new file mode 100644 index 0000000..0e9feff --- /dev/null +++ b/tests/cli/fields.test.ts @@ -0,0 +1,334 @@ +import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; +import { runCli, tempWorkspace } from "./harness"; +import { freshFixture } from "./helpers"; + +/** Issue #42 step 1: a field is atomic structure. Every mutator either keeps + * it intact, removes it whole, or refuses with nothing written — never a + * silent flatten to frozen text, never half a field. */ + +const PAPER = "tests/fixtures/multi-column.docx"; +// p34: "…referenced by numbers in brackets [1, 3, 4]." — three complex +// `REF _Ref… \r \h` fields whose cached results are "1", "3", "4]". +const NEW_TEXT = + "Use a numbered list of references at the end of the article, ordered alphabetically by surname of first author, and referenced by numbers in brackets [1, 3, 4]."; + +async function paragraphXml(path: string, locator: string): Promise { + return (await runCli("raw", "get", path, "--at", locator)).stdout; +} + +const count = (xml: string, needle: string): number => + xml.split(needle).length - 1; + +/** begin/separate/end counts — a balanced field contributes one of each. */ +function fieldChars(xml: string): Record { + return { + begin: count(xml, 'w:fldCharType="begin"'), + separate: count(xml, 'w:fldCharType="separate"'), + end: count(xml, 'w:fldCharType="end"'), + }; +} + +async function expectUnchanged( + path: string, + run: () => Promise<{ exitCode: number; stdout: string }>, + code: string, +): Promise { + const before = Buffer.from(await Bun.file(path).arrayBuffer()); + const result = await run(); + expect(result.exitCode).not.toBe(0); + expect(result.stdout).toContain(code); + const after = Buffer.from(await Bun.file(path).arrayBuffer()); + expect(after.equals(before)).toBe(true); +} + +describe("whole-paragraph rebuilds refuse a paragraph holding fields", () => { + const cases: [string, string[]][] = [ + ["markdown (the default channel)", [NEW_TEXT]], + ["--text", ["--text", NEW_TEXT]], + ["--track --text", ["--track", "--text", NEW_TEXT]], + ["--track markdown", ["--track", NEW_TEXT]], + ["--runs", ["--runs", JSON.stringify([{ text: NEW_TEXT }])]], + ]; + for (const [label, flags] of cases) { + test(label, async () => { + const path = await freshFixture(`fields-${label}`, PAPER); + await expectUnchanged( + path, + () => runCli("edit", path, "--at", "p34", ...flags), + "FIELD_IN_PARAGRAPH", + ); + }); + } + + test("the refusal names the fields by kind and cached result", async () => { + const path = await freshFixture("fields-message", PAPER); + const result = await runCli("edit", path, "--at", "p34", NEW_TEXT); + expect(result.stdout).toContain( + '3 fields (REF ×3: \\"1\\", \\"3\\", \\"4]\\")', + ); + }); + + test("--batch refuses the whole batch, nothing written", async () => { + const path = await freshFixture("fields-batch", PAPER); + const batch = join(tempWorkspace("fields-batch"), "edits.jsonl"); + await Bun.write( + batch, + [ + JSON.stringify({ at: "p0", text: "Changed title" }), + JSON.stringify({ at: "p34", text: NEW_TEXT }), + ].join("\n"), + ); + await expectUnchanged( + path, + () => runCli("edit", path, "--batch", batch), + "FIELD_IN_PARAGRAPH", + ); + }); + + test("a range edit over a field paragraph refuses", async () => { + const path = await freshFixture("fields-range", PAPER); + await expectUnchanged( + path, + () => runCli("edit", path, "--at", "p33-p34", "--text", "x"), + "FIELD_IN_PARAGRAPH", + ); + }); +}); + +describe("span edits keep, cover, or refuse fields", () => { + test("replace beside the fields keeps all three", async () => { + const path = await freshFixture("fields-beside", PAPER); + const result = await runCli("replace", path, "last name", "surname"); + expect(result.exitCode).toBe(0); + expect(count(await paragraphXml(path, "p34"), " REF _Ref")).toBe(3); + }); + + test("a span inside a field's result edits the cached text, field kept", async () => { + const path = await freshFixture("fields-inside", PAPER); + const result = await runCli( + "edit", + path, + "--at", + "p34:159-161", + "--text", + "5]", + ); + expect(result.exitCode).toBe(0); + const xml = await paragraphXml(path, "p34"); + expect(count(xml, " REF _Ref")).toBe(3); + expect(fieldChars(xml)).toEqual({ begin: 3, separate: 3, end: 3 }); + expect(xml).toContain( + '>5]', + ); + }); + + test("a span covering whole fields removes them whole — no orphan end", async () => { + const path = await freshFixture("fields-covered", PAPER); + const result = await runCli("replace", path, "[1, 3, 4]", "[2]"); + expect(result.exitCode).toBe(0); + const xml = await paragraphXml(path, "p34"); + expect(fieldChars(xml)).toEqual({ begin: 0, separate: 0, end: 0 }); + expect(xml).not.toContain("instrText"); + const validate = await runCli("validate", path); + expect(validate.stdout.startsWith("valid")).toBe(true); + }); + + test("tracked, the covered fields are deleted whole: accept removes, reject restores", async () => { + const path = await freshFixture("fields-covered-tracked", PAPER); + const result = await runCli("replace", path, "[1, 3, 4]", "[2]", "--track"); + expect(result.exitCode).toBe(0); + const tracked = await paragraphXml(path, "p34"); + expect(count(tracked, " { + const path = await freshFixture("fields-cut", PAPER); + await expectUnchanged( + path, + () => runCli("replace", path, "3, 4", "9"), + "cuts a field", + ); + await expectUnchanged( + path, + () => runCli("edit", path, "--at", "p34:156-160", "--text", "9"), + "cuts a field", + ); + }); +}); + +/** A fresh document holding one paragraph per entry of `lines`. */ +async function createParagraphs( + label: string, + lines: string[], +): Promise { + const workspace = tempWorkspace(label); + const source = join(workspace, "source.md"); + await Bun.write(source, lines.join("\n\n")); + const path = join(workspace, "doc.docx"); + expect((await runCli("create", path, "--from", source)).exitCode).toBe(0); + return path; +} + +/** A TOC-shaped field: begin + instruction + separate in one paragraph, its + * `end` in the next. */ +async function twoParagraphField(label: string): Promise { + const path = await createParagraphs(label, [ + "Contents", + "placeholder", + "placeholder", + "Body text", + ]); + const begin = + ' TOC \\o "1-3" Introduction 1'; + const end = + 'Methods 2'; + expect( + (await runCli("raw", "replace", path, "--at", "p1", "--xml", begin)) + .exitCode, + ).toBe(0); + expect( + (await runCli("raw", "replace", path, "--at", "p2", "--xml", end)).exitCode, + ).toBe(0); + return path; +} + +describe("paragraph removal never splits a multi-paragraph field", () => { + test("delete --at one end refuses", async () => { + const path = await twoParagraphField("fields-split-delete"); + await expectUnchanged( + path, + () => runCli("delete", path, "--at", "p2"), + "split a field", + ); + await expectUnchanged( + path, + () => runCli("delete", path, "--at", "p0-p1"), + "split a field", + ); + }); + + test('edit --text "" (line removal) refuses, single and --batch', async () => { + const path = await twoParagraphField("fields-split-edit"); + await expectUnchanged( + path, + () => runCli("edit", path, "--at", "p1", "--text", ""), + "split a field", + ); + const batch = join(tempWorkspace("fields-split-batch"), "b.jsonl"); + await Bun.write(batch, JSON.stringify({ at: "p2", delete: true })); + await expectUnchanged( + path, + () => runCli("edit", path, "--batch", batch), + "split a field", + ); + }); + + test("rebuilding one end refuses (it holds a field marker)", async () => { + const path = await twoParagraphField("fields-split-rebuild"); + await expectUnchanged( + path, + () => runCli("edit", path, "--at", "p1", "Introduction 3"), + "FIELD_IN_PARAGRAPH", + ); + }); + + test("removing both ends takes the whole field", async () => { + const path = await twoParagraphField("fields-split-both"); + expect((await runCli("delete", path, "--at", "p1-p2")).exitCode).toBe(0); + const batchPath = await twoParagraphField("fields-split-both-batch"); + const batch = join(tempWorkspace("fields-split-both-b"), "b.jsonl"); + await Bun.write( + batch, + `${JSON.stringify({ at: "p1" })}\n${JSON.stringify({ at: "p2" })}`, + ); + expect((await runCli("delete", batchPath, "--batch", batch)).exitCode).toBe( + 0, + ); + for (const done of [path, batchPath]) { + const validate = await runCli("validate", done); + expect(validate.stdout.startsWith("valid")).toBe(true); + } + }); +}); + +describe("cross-paragraph replace keeps a multi-paragraph field whole", () => { + test("a match inside the field's result is fine", async () => { + const path = await twoParagraphField("fields-across-inside"); + const result = await runCli( + "replace", + path, + "Introduction 1\nMethods", + "Overview", + ); + expect(result.exitCode).toBe(0); + const validate = await runCli("validate", path); + expect(validate.stdout.startsWith("valid")).toBe(true); + expect(fieldChars(await paragraphXml(path, "p1"))).toEqual({ + begin: 1, + separate: 1, + end: 1, + }); + }); + + test("a match taking the field's begin but not its end is refused", async () => { + const path = await twoParagraphField("fields-across-cut"); + await expectUnchanged( + path, + () => runCli("replace", path, "Contents\nIntroduction", "Contents"), + "cuts a field", + ); + }); +}); + +describe("tracked paragraph deletion writes delInstrText", () => { + test("delete --track; reject restores ", async () => { + const path = await freshFixture("fields-tracked-delete", PAPER); + expect( + (await runCli("delete", path, "--at", "p34", "--track")).exitCode, + ).toBe(0); + const deleted = await paragraphXml(path, "p34"); + expect(count(deleted, " { + test("an orphan end is an error", async () => { + const path = await createParagraphs("fields-orphan", ["one", "two"]); + await runCli( + "raw", + "replace", + path, + "--at", + "p1", + "--xml", + 'two', + "--no-validate", + ); + const result = await runCli("validate", path); + expect(result.exitCode).toBe(1); + expect(result.stdout).toContain("unbalanced field"); + }); +}); diff --git a/tests/cli/invariants.test.ts b/tests/cli/invariants.test.ts index b29e294..db8bda5 100644 --- a/tests/cli/invariants.test.ts +++ b/tests/cli/invariants.test.ts @@ -433,25 +433,16 @@ describe("transparent wrappers — replace inside the wrapper", () => { expect(p1Text).toBe("Hello Bob."); }); - test("replace span crossing INTO a wrapper splits it cleanly", async () => { + test("replace span cutting INTO a is refused (half a field)", async () => { const docPath = await freshCopy("transparent-replace-cross"); - // "is 2026" crosses the fldSimple boundary; replacing it should leave - // the rest of the date inside a (now-shorter) fldSimple half. + const before = await Bun.file(docPath).arrayBuffer(); + // "is 2026" covers part of the field's result and text outside it — + // splitting the fldSimple would duplicate its instruction (issue #42). const result = await runCli("replace", docPath, "is 2026", "was 2025"); - expect(result.exitCode).toBe(0); - - const read = await runCli("read", docPath, "--ast"); - const doc = read.parsed as { - blocks: Array<{ - type: string; - runs?: Array<{ type: string; text?: string }>; - }>; - }; - const p0Text = (doc.blocks[0]?.runs ?? []) - .filter((run) => run.type === "text") - .map((run) => run.text ?? "") - .join(""); - expect(p0Text).toBe("Today was 2025-05-05."); + expect(result.exitCode).not.toBe(0); + expect(result.stdout).toContain("cuts a field"); + const after = await Bun.file(docPath).arrayBuffer(); + expect(Buffer.from(after).equals(Buffer.from(before))).toBe(true); }); test("the fldSimple wrapper survives a fully-internal replace", async () => {