diff --git a/docs/tools/edit.md b/docs/tools/edit.md index 2b35bb44b..6fcda8ac5 100644 --- a/docs/tools/edit.md +++ b/docs/tools/edit.md @@ -63,7 +63,7 @@ The canonical grammar is strict, but the hand parser accepts a few non-dangerous - `delete N..M:` and any body rows under `delete` / `delete block` are rejected. - Empty `replace` / `insert` / `replace block` hunks are rejected. - `-` body rows are rejected with `MINUS_ROW_REJECTED`. -- `replace block N:` / `delete block N` / `insert after block N:` require a wired tree-sitter resolver; `replace block` and `insert after block` additionally need at least one `+TEXT` body row, while `delete block` takes none. An unresolvable block (unsupported language, blank/closing-delimiter line, no node beginning on N, or a syntax error in the resolved block) is rejected on the apply/final-preview path; the streaming preview silently drops it instead. +- `replace block N:` / `delete block N` / `insert after block N:` require a wired tree-sitter resolver; `replace block` and `insert after block` additionally need at least one `+TEXT` body row, while `delete block` takes none. An unresolvable block (unsupported language, blank/closing-delimiter line, no node beginning on N, or a syntax error in the resolved block) is rejected on the apply/final-preview path; the streaming preview silently drops it instead. Exception: `insert after block N:` anchored on a pure closing-delimiter line is lowered to plain `insert after N:` with a warning — line N is the end of a block, and inserting after that end is exactly what the plain form does. ## Outputs - Single-shot tool result; hashline mode does not use a `resolve` preview/apply handshake. diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index d5bfe66d9..777b13908 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -5,6 +5,7 @@ ### Added - Added inward landing correction for `insert after block N:`: a body indented deeper than the block's closing line now slides back across the block's trailing closer lines and lands inside the block at its claimed depth, with a warning naming the landing line. Same conservative guards as the outward shift — comparable indentation only, closers only, abandoned when another hunk targets a crossed line; plain `insert after M:` stays literal +- Added closer-anchor lowering for `insert after block N:`: anchoring on a pure closing-delimiter line (where no block begins, so resolution previously failed the whole patch) now applies as plain `insert after N:` with a warning teaching the opener-only rule. `resolveBlockEdits` gained an `onWarning` callback; apply, preview, and patcher paths surface it on `warnings` ### Changed diff --git a/packages/hashline/src/apply.ts b/packages/hashline/src/apply.ts index 7ae3d6cb8..ca7bba16d 100644 --- a/packages/hashline/src/apply.ts +++ b/packages/hashline/src/apply.ts @@ -123,7 +123,7 @@ function bucketAnchorEditsByLine(edits: IndexedEdit[]): Map void; + /** + * Invoked once per diagnostic produced while resolving — currently only the + * closer-anchor lowering of `insert after block N:`. Hosts should surface + * these on the apply result's `warnings`. + */ + onWarning?: (message: string) => void; } /** True when at least one edit is an unresolved deferred block edit. */ @@ -67,6 +74,22 @@ export function resolveBlockEdits( const op = edit.mode === "insert_after" ? "insert_after" : edit.payloads.length === 0 ? "delete" : "replace"; const span = resolver ? resolver({ path, text, line: edit.anchor.line }) : null; if (span === null) { + // `insert after block N` anchored on a pure closing-delimiter line: + // no block begins there, but line N IS the end of one — and "after + // the end of the block" is exactly plain `insert after N:`. Lower it + // instead of failing the patch; warn so the author learns the + // opener-only rule. + if (op === "insert_after" && resolver) { + const anchorText = text.split("\n")[edit.anchor.line - 1]; + if (anchorText !== undefined && STRUCTURAL_CLOSER_RE.test(anchorText)) { + options.onWarning?.(insertAfterBlockCloserLoweredWarning(edit.anchor.line)); + for (const payload of edit.payloads) { + const cursor: Cursor = { kind: "after_anchor", anchor: { line: edit.anchor.line } }; + resolved.push({ kind: "insert", cursor, text: payload, lineNum: edit.lineNum, index: synthIndex++ }); + } + continue; + } + } if (onUnresolved === "drop") continue; throw new Error( `line ${edit.lineNum}: ${ diff --git a/packages/hashline/src/input.ts b/packages/hashline/src/input.ts index 3513d1aea..b3bc1a134 100644 --- a/packages/hashline/src/input.ts +++ b/packages/hashline/src/input.ts @@ -309,12 +309,16 @@ export class PatchSection { */ applyTo(text: string, blockResolver?: BlockResolver): ApplyResult { const { edits, warnings } = this.parse(); - const resolved = resolveBlockEdits(edits, text, this.path, blockResolver, { onUnresolved: "throw" }); + const resolveWarnings: string[] = []; + const resolved = resolveBlockEdits(edits, text, this.path, blockResolver, { + onUnresolved: "throw", + onWarning: warning => resolveWarnings.push(warning), + }); const result = applyEdits(text, resolved); // Preserve parse warnings so consumers don't need to call `parse()` // separately. - const merged = warnings.length === 0 ? result.warnings : [...warnings, ...(result.warnings ?? [])]; - return merged && merged.length > 0 + const merged = [...warnings, ...resolveWarnings, ...(result.warnings ?? [])]; + return merged.length > 0 ? { ...result, warnings: merged } : { text: result.text, firstChangedLine: result.firstChangedLine }; } @@ -332,10 +336,14 @@ export class PatchSection { */ applyPartialTo(text: string, blockResolver?: BlockResolver): ApplyResult { const { edits, warnings } = parsePatchStreaming(this.diff); - const resolved = resolveBlockEdits(edits, text, this.path, blockResolver, { onUnresolved: "drop" }); + const resolveWarnings: string[] = []; + const resolved = resolveBlockEdits(edits, text, this.path, blockResolver, { + onUnresolved: "drop", + onWarning: warning => resolveWarnings.push(warning), + }); const result = applyEdits(text, resolved); - const merged = warnings.length === 0 ? result.warnings : [...warnings, ...(result.warnings ?? [])]; - return merged && merged.length > 0 + const merged = [...warnings, ...resolveWarnings, ...(result.warnings ?? [])]; + return merged.length > 0 ? { ...result, warnings: merged } : { text: result.text, firstChangedLine: result.firstChangedLine }; } diff --git a/packages/hashline/src/messages.ts b/packages/hashline/src/messages.ts index 075062f3a..33c7429ea 100644 --- a/packages/hashline/src/messages.ts +++ b/packages/hashline/src/messages.ts @@ -116,6 +116,19 @@ export function blockUnresolvedMessage( export const BLOCK_RESOLVER_UNAVAILABLE = "Block-anchored ops (`replace block N:`, `delete block N`, `insert after block N:`) are not available here (no tree-sitter block resolver is configured). Use a concrete line range instead."; +/** + * Warning emitted when an `insert after block N:` anchored on a pure + * closing-delimiter line is lowered to plain `insert after N:`. No block + * begins on a closer, but the closer IS the end of one — and inserting after + * the end of that block is exactly what the plain form does. + */ +export function insertAfterBlockCloserLoweredWarning(line: number): string { + return ( + `\`insert after block ${line}:\` anchors on a closing-delimiter line; no block begins there, so it was applied as plain \`insert after ${line}:\`. ` + + "Anchor `insert after block` on the line that OPENS the construct." + ); +} + /** * Internal invariant error: `applyEdits` received an unresolved `replace block * N:` edit. Block edits must be expanded by `resolveBlockEdits` before reaching diff --git a/packages/hashline/src/patcher.ts b/packages/hashline/src/patcher.ts index d0d7b699f..511119640 100644 --- a/packages/hashline/src/patcher.ts +++ b/packages/hashline/src/patcher.ts @@ -380,6 +380,7 @@ export class Patcher { // When a block edit needs the tagged snapshot but it is unavailable, the // range cannot be placed safely — reject with a MismatchError (re-read). const blockResolutions: BlockResolution[] = []; + const resolveWarnings: string[] = []; let resolved: readonly Edit[] = edits; if (hasBlockEdit(edits)) { const baseText = @@ -390,8 +391,13 @@ export class Patcher { resolved = resolveBlockEdits(edits, baseText, section.path, this.blockResolver, { onUnresolved: "throw", onResolved: resolution => blockResolutions.push(resolution), + onWarning: warning => resolveWarnings.push(warning), }); } + const withResolveWarnings = (result: ApplyResult): ApplyResult => + resolveWarnings.length === 0 + ? result + : { ...result, warnings: [...resolveWarnings, ...(result.warnings ?? [])] }; // No tag, or the tag still names the live content: an edit anchored at any // line is safe to apply, and the resolved block spans line up with what @@ -399,7 +405,7 @@ export class Patcher { // recovery below, where line numbers shift, so resolutions are dropped.) if (expected === undefined || liveMatches) { const result = applyEdits(normalized, resolved); - return blockResolutions.length > 0 ? { ...result, blockResolutions } : result; + return withResolveWarnings(blockResolutions.length > 0 ? { ...result, blockResolutions } : result); } // Head/tail-only inserts are position-stable: "start"/"end" cannot move // with content drift, so a stale tag is non-fatal. Apply onto the live @@ -407,7 +413,7 @@ export class Patcher { // mismatch, which cannot be safely relocated and must reject. if (!hasAnchorScopedEdit(resolved)) { const result = applyEdits(normalized, resolved); - return { ...result, warnings: [HEADTAIL_DRIFT_WARNING, ...(result.warnings ?? [])] }; + return withResolveWarnings({ ...result, warnings: [HEADTAIL_DRIFT_WARNING, ...(result.warnings ?? [])] }); } // File drifted: try to replay the edit against the version the tag // names and 3-way-merge it onto the live content. @@ -417,7 +423,7 @@ export class Patcher { fileHash: expected, edits: resolved, }); - if (recovered) return recoveryToApplyResult(recovered); + if (recovered) return withResolveWarnings(recoveryToApplyResult(recovered)); const hashRecognized = this.snapshots.byHash(canonicalPath, expected) !== null; throw this.#mismatchError(section, canonicalPath, normalized, expected, hashRecognized); } diff --git a/packages/hashline/test/block.test.ts b/packages/hashline/test/block.test.ts index b634f536b..6ed417aea 100644 --- a/packages/hashline/test/block.test.ts +++ b/packages/hashline/test/block.test.ts @@ -340,21 +340,37 @@ describe("insert after block", () => { expect(() => resolveBlockEdits(edits, "ignored", PATH, () => null)).toThrow("`insert after block 7:`"); }); - it("rejects a closing-delimiter line as an insert-after-block anchor", () => { + it("lowers a closing-delimiter anchor to plain `insert after N:` with a warning", () => { const section = Patch.parseSingle(`[${PATH}#1A2B]\ninsert after block 3:\n+ done();`); const resolver: BlockResolver = ({ line }) => (line === 2 ? { start: 2, end: 3 } : null); - let error: Error | undefined; - try { - section.applyTo(text, resolver); - } catch (err) { - error = err instanceof Error ? err : new Error(String(err)); - } - expect(error?.message).toContain( - "`insert after block 3:` could not resolve a syntactic block beginning on line 3", + const result = section.applyTo(text, resolver); + + // line 3 is ` }` — no block begins there, but it ends one; the body + // lands after it, exactly where `insert after block` would have put it. + expect(result.text).toBe("function x() {\n if (y) {\n }\n done();\n}\n"); + expect(result.warnings?.some(w => /applied as plain `insert after 3:`/.test(w))).toBe(true); + }); + + it("still rejects an unresolvable blank-line anchor (lowering is closer-only)", () => { + const blankAnchored = Patch.parseSingle(`[${PATH}#1A2B]\ninsert after block 2:\n+done();`); + + expect(() => blankAnchored.applyTo("function x() {\n\n}\n", () => null)).toThrow( + "`insert after block 2:` could not resolve a syntactic block beginning on line 2", ); - expect(error?.message).toContain("Use `insert after M:` with the block's explicit last line instead"); - expect(error?.message).toContain("*3: }"); + }); + + it("Patcher surfaces the closer-anchor lowering warning", async () => { + const fs = new InMemoryFilesystem([[PATH, text]]); + const snapshots = new InMemorySnapshotStore(); + const tag = snapshots.record(PATH, text); + const resolver: BlockResolver = ({ line }) => (line === 2 ? { start: 2, end: 3 } : null); + const patcher = new Patcher({ fs, snapshots, blockResolver: resolver }); + + const result = await patcher.apply(Patch.parse(`[${PATH}#${tag}]\ninsert after block 3:\n+ done();`)); + + expect(fs.get(PATH)).toBe("function x() {\n if (y) {\n }\n done();\n}\n"); + expect(result.sections[0]?.warnings.some(w => /applied as plain `insert after 3:`/.test(w))).toBe(true); }); it("applyTo inserts the body after the resolved block's last line", () => {