feat(hashline): allowed closer-line insert after block anchors with warning fallback
- Updated block resolution to detect structural-closing-line anchors and convert unresolvable `insert after block` edits into plain `insert after` inserts while emitting a dedicated warning. - Propagated these resolver warnings through `PatchSection` and `Patcher` results so parse and application callers receive them. - Added tests covering the closer-anchor fallback and confirming non-closer unresolved block anchors still throw.
This commit is contained in:
can1357
+1
-1
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -123,7 +123,7 @@ function bucketAnchorEditsByLine(edits: IndexedEdit[]): Map<number, IndexedEdit[
|
||||
// wrapper" mistake.
|
||||
|
||||
/** A line that is nothing but closing delimiters: `}`, `)`, `];`, `})`, `},`. */
|
||||
const STRUCTURAL_CLOSER_RE = /^\s*[)\]}]+[;,]?\s*$/;
|
||||
export const STRUCTURAL_CLOSER_RE = /^\s*[)\]}]+[;,]?\s*$/;
|
||||
|
||||
interface DelimiterBalance {
|
||||
paren: number;
|
||||
|
||||
@@ -12,7 +12,8 @@
|
||||
* it runs, no `block` edits remain, so {@link applyEdits} (and recovery) only
|
||||
* ever see resolved edits.
|
||||
*/
|
||||
import { BLOCK_RESOLVER_UNAVAILABLE, blockUnresolvedMessage } from "./messages";
|
||||
import { STRUCTURAL_CLOSER_RE } from "./apply";
|
||||
import { BLOCK_RESOLVER_UNAVAILABLE, blockUnresolvedMessage, insertAfterBlockCloserLoweredWarning } from "./messages";
|
||||
import type { BlockResolution, BlockResolver, Cursor, Edit } from "./types";
|
||||
|
||||
export interface ResolveBlockEditsOptions {
|
||||
@@ -31,6 +32,12 @@ export interface ResolveBlockEditsOptions {
|
||||
* edits.
|
||||
*/
|
||||
onResolved?: (resolution: BlockResolution) => 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}: ${
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user