From fe3b9829761ebada4d68779bcbbf9cebe41fb8be Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 27 May 2026 12:57:35 +0200 Subject: [PATCH] refactor(hashline): removed delete operation support from hashline format - Removed the `!` delete sigil and all associated parsing, validation, and tokenization logic. The delete operation is no longer a supported edit kind. - Updated grammar, format constants, and error messages to reference only insert and replace operations. - Simplified the executor's overlap validation to handle only replace operations. --- packages/coding-agent/src/edit/streaming.ts | 1 - .../coding-agent/test/core/hashline.test.ts | 47 +------------- packages/hashline/CHANGELOG.md | 4 ++ packages/hashline/README.md | 1 - packages/hashline/src/apply.ts | 3 +- packages/hashline/src/format.ts | 4 +- packages/hashline/src/grammar.lark | 3 +- packages/hashline/src/parser.ts | 64 ++++++------------- packages/hashline/src/prompt.md | 7 +- packages/hashline/src/tokenizer.ts | 41 +----------- 10 files changed, 32 insertions(+), 143 deletions(-) diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts index 639ca15e1..c5bd9c12e 100644 --- a/packages/coding-agent/src/edit/streaming.ts +++ b/packages/coding-agent/src/edit/streaming.ts @@ -385,7 +385,6 @@ function buildHashlineNaturalOrderPreviews( case "envelope-begin": case "envelope-end": case "abort": - case "op-delete": continue; case "blank": case "raw": diff --git a/packages/coding-agent/test/core/hashline.test.ts b/packages/coding-agent/test/core/hashline.test.ts index 9a04dd236..c944a19ec 100644 --- a/packages/coding-agent/test/core/hashline.test.ts +++ b/packages/coding-agent/test/core/hashline.test.ts @@ -212,11 +212,6 @@ describe("hashline parser — suffix-op syntax", () => { expect(applyDiff(content, diff)).toBe("aaa\nbbb\nccc\ntail"); }); - it("deletes one line or an inclusive range with `!`", () => { - expect(applyDiff(content, `${sameLineRange(tag(2, "bbb"))}!`)).toBe("aaa\nccc"); - expect(applyDiff(content, `${tag(2, "bbb")}-${tag(3, "ccc")}!`)).toBe("aaa"); - }); - it("blanks a line in place with `A:` when given an explicit empty payload", () => { const explicit = `${sameLineRange(tag(2, "bbb"))}:`; expect(applyDiff(content, explicit)).toBe("aaa\n\nccc"); @@ -569,9 +564,7 @@ describe("hashline parser — suffix-op syntax", () => { }); it("describes the new sigil shape on unknown-op lines", () => { - expect(() => parseHashline(`-${sameLineRange(tag(2, "bbb"))}`).edits).toThrow( - /Use LINE↑.*LINE↓.*LINE: \/ A-B:.*LINE! \/ A-B!/, - ); + expect(() => parseHashline(`-${sameLineRange(tag(2, "bbb"))}`).edits).toThrow(/Use LINE↑.*LINE↓.*LINE: \/ A-B:/); }); it("accepts `LINE:TEXT` copied verbatim from read output with a deprecation warning", () => { @@ -616,9 +609,6 @@ describe("hashline parser — suffix-op syntax", () => { const inline = parseHashline(`BOF↓HEAD`); expect(inline.warnings.some(w => /Accepted inline payload on the op line/.test(w))).toBe(true); expect(applyDiff(content, `BOF↓HEAD`)).toBe("HEAD\naaa\nbbb\nccc"); - expect(() => parseHashline(`2!keep`).edits).toThrow( - /deletes only\. Payload is forbidden after !; use : to replace/, - ); }); it("coalesces two replace ops targeting the same single line (last wins)", () => { @@ -675,16 +665,6 @@ describe("hashline parser — suffix-op syntax", () => { expect(warnings).toEqual([]); }); - it("rejects a replace overlapping a later delete", () => { - const diff = `${tag(2, "bbb")}-${tag(4, "ddd")}:\n${extra("X")}\n${tag(3, "ccc")}!`; - expect(() => parseHashline(diff).edits).toThrow(/anchor line 3 is already targeted by the .+ op on line 1/); - }); - - it("rejects two deletes on the same line", () => { - const diff = `${tag(2, "bbb")}!\n${tag(2, "bbb")}!`; - expect(() => parseHashline(diff).edits).toThrow(/anchor line 2 is already targeted by the .+ op on line 1/); - }); - it("accepts multiple inserts at the same anchor (sequential, not duplicates)", () => { // Two ↑ at the same line is a legitimate accumulation pattern — both // inserts above land in source order. Only deletes/replaces are @@ -1272,31 +1252,6 @@ describe("hashline parser — bare ':' replaces with a single blank line", () => }); }); -describe("hashline apply — brace-delete soft warning", () => { - it("deleting a line with unbalanced brace emits a warning", () => { - const text = "if (x) {\n doThing();\n} else {\n doOther();\n}\n"; - const { diff } = splitHashlineInput(`${header("a.ts", text)}\n3!\n`); - const result = applyHashlineEdits(text, parseHashline(diff).edits); - expect(result.warnings).toBeDefined(); - expect(result.warnings![0]).toContain("structural bracket/brace boundary"); - expect(result.warnings![0]).toContain("} else {"); - }); - - it("deleting a balanced line emits no warning", () => { - const text = "line1\nline2\nline3\n"; - const { diff } = splitHashlineInput(`${header("a.ts", text)}\n2!\n`); - const result = applyHashlineEdits(text, parseHashline(diff).edits); - expect(result.warnings).toBeUndefined(); - }); - - it("replace operation that includes a brace line does NOT warn", () => { - const text = "if (x) {\n body\n}\n"; - const { diff } = splitHashlineInput(`${header("a.ts", text)}\n3:}\n`); - const result = applyHashlineEdits(text, parseHashline(diff).edits); - expect(result.warnings).toBeUndefined(); - }); -}); - describe("hashline parser — plus-prefixed blank payload lines", () => { it("raw blank lines between ops are ignored", () => { const text = "a\nb\nc\nd\ne\n"; diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 5ed3bd363..67fea7761 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Removed + +- Removed the `A-B!` / `A!` deletion operator. Use `A-B:` with the desired payload (or empty payload to blank the range) instead. + ### Fixed - Parser now skips markdown-style `# ...` lines when they directly precede a hashline operation, making model-generated explanatory rows in prompt examples non-blocking. diff --git a/packages/hashline/README.md b/packages/hashline/README.md index 92ae8d2fc..303eafd77 100644 --- a/packages/hashline/README.md +++ b/packages/hashline/README.md @@ -50,7 +50,6 @@ Inside a hunk: |`LINE↑`|Insert before LINE (or `BOF↑` for the beginning of file)| |`LINE↓`|Insert after LINE (or `EOF↓` for the end of file)| |`A-B:`|Replace lines A..B (single-anchor `A:` is sugar for `A-A:`)| -|`A-B!`|Delete lines A..B (single-anchor `A!` is sugar for `A-A!`)| |`+TEXT`|Payload continuation. The `+` prefix is stripped| ## Abstractions diff --git a/packages/hashline/src/apply.ts b/packages/hashline/src/apply.ts index 7654ab9dc..262d213b2 100644 --- a/packages/hashline/src/apply.ts +++ b/packages/hashline/src/apply.ts @@ -320,8 +320,7 @@ function countMatchingSingleStructuralSuffixBoundary( * groups. Mirrors the same boundary check the pure-insert absorber uses for * `ANCHOR↓` (leading) / `ANCHOR↑` (trailing) inserts, but applied to the * top/bottom edges of an `A-B:payload` range. Catches mistakes like - * `103-138:const X = …` where line 102 already reads `const X = …` and the - * user really meant `103-138!` (delete only). + * `103-138:const X = …` where line 102 already reads `const X = …`. * * Gated by `options.autoDropPureInsertDuplicates`: the existing 2+-line block * absorb already runs unconditionally, and the structural single-line diff --git a/packages/hashline/src/format.ts b/packages/hashline/src/format.ts index ba0925c12..fa4b5a131 100644 --- a/packages/hashline/src/format.ts +++ b/packages/hashline/src/format.ts @@ -10,11 +10,9 @@ export const HL_OP_INSERT_BEFORE = "↑"; export const HL_OP_INSERT_AFTER = "↓"; /** Op sigil used after a range (or single anchor) to replace its lines. */ export const HL_OP_REPLACE = ":"; -/** Op sigil used after a range (or single anchor) to delete its lines. */ -export const HL_OP_DELETE = "!"; /** All hashline edit op sigils, concatenated for fast membership tests. */ -export const HL_OP_CHARS = `${HL_OP_INSERT_BEFORE}${HL_OP_INSERT_AFTER}${HL_OP_REPLACE}${HL_OP_DELETE}`; +export const HL_OP_CHARS = `${HL_OP_INSERT_BEFORE}${HL_OP_INSERT_AFTER}${HL_OP_REPLACE}`; /** Prefix for payload continuation lines. The prefix itself is not written. */ export const HL_PAYLOAD_PREFIX = "+"; diff --git a/packages/hashline/src/grammar.lark b/packages/hashline/src/grammar.lark index 9f0ef23ad..ede25538e 100644 --- a/packages/hashline/src/grammar.lark +++ b/packages/hashline/src/grammar.lark @@ -8,11 +8,10 @@ update_hunk: "¶" filename ("#" file_hash)? LF line_op* filename: /([^\s#]+)/ file_hash: /[0-9a-f]{4}/ -line_op: insert_before | insert_after | replace | delete +line_op: insert_before | insert_after | replace insert_before: anchor "↑" LF payload* insert_after: anchor "↓" LF payload* replace: range ":" LF payload* -delete: range "!" LF payload: "+" /[^\n]*/ LF anchor: LID | "EOF" | "BOF" diff --git a/packages/hashline/src/parser.ts b/packages/hashline/src/parser.ts index 7a229608e..3434b0e46 100644 --- a/packages/hashline/src/parser.ts +++ b/packages/hashline/src/parser.ts @@ -13,14 +13,7 @@ * * Convenience entry point: {@link parsePatch}. */ -import { - HL_OP_CHARS, - HL_OP_DELETE, - HL_OP_INSERT_AFTER, - HL_OP_INSERT_BEFORE, - HL_OP_REPLACE, - HL_PAYLOAD_PREFIX, -} from "./format"; +import { HL_OP_CHARS, HL_OP_INSERT_AFTER, HL_OP_INSERT_BEFORE, HL_OP_REPLACE, HL_PAYLOAD_PREFIX } from "./format"; import { ABORT_WARNING, IMPLICIT_CONTINUATION_WARNING, @@ -28,7 +21,7 @@ import { PAYLOAD_LINE_PREFIX_DEMOTED_WARNING, REPLACE_PAIR_COALESCED_WARNING, } from "./messages"; -import { cloneCursor, isDeleteOpWithPayload, type ParsedRange, type Token, Tokenizer } from "./tokenizer"; +import { cloneCursor, type ParsedRange, type Token, Tokenizer } from "./tokenizer"; import type { Anchor, Cursor, Edit } from "./types"; function validateRangeOrder(range: ParsedRange, lineNum: number): void { @@ -143,19 +136,6 @@ export class Executor { this.#consumePendingSkippableComments(); this.#handleRaw(token.text, token.lineNum); return; - case "op-delete": - this.#discardPendingSkippableComments(); - this.#flushPending(); - if (token.trailingPayload) { - throw new Error( - `line ${token.lineNum}: ${HL_OP_DELETE} deletes only. Payload is forbidden after ${HL_OP_DELETE}; use ${HL_OP_REPLACE} to replace.`, - ); - } - validateRangeOrder(token.range, token.lineNum); - for (const anchor of expandRange(token.range)) { - this.#edits.push({ kind: "delete", anchor, lineNum: token.lineNum, index: this.#editIndex++ }); - } - return; case "op-insert": this.#discardPendingSkippableComments(); this.#flushPending(); @@ -179,8 +159,8 @@ export class Executor { if (rangesEqual(outer, inner)) { // Identical-range before/after pair. Drop the "before" payload // silently; the second op proceeds as the lone winner. Other - // overlap shapes (different ranges, replace+delete, delete+delete) - // still hit the post-hoc validator. + // overlap shapes (different ranges) still hit the post-hoc + // validator. this.#pending = undefined; if (!this.#warnings.includes(REPLACE_PAIR_COALESCED_WARNING)) { this.#warnings.push(REPLACE_PAIR_COALESCED_WARNING); @@ -221,10 +201,10 @@ export class Executor { * warnings. The executor is single-use; {@link reset} is required for * reuse. * - * Throws if two replace/delete ops target the same line with non-identical - * shapes (different ranges, replace+delete, delete+delete). Identical-range - * `A-B:` pairs in the same hunk are coalesced last-wins by `feed()` with a - * warning, so they never reach the validator. + * Throws if two replace ops target the same line with non-identical + * ranges. Identical-range `A-B:` pairs in the same hunk are coalesced + * last-wins by `feed()` with a warning, so they never reach the + * validator. */ end(): { edits: Edit[]; warnings: string[] } { this.#consumePendingSkippableComments(); @@ -232,6 +212,7 @@ export class Executor { this.#validateNoOverlappingDeletes(); return { edits: this.#edits, warnings: this.#warnings }; } + /** Reset to a fresh state so the same instance can drive another parse. */ reset(): void { this.#edits = []; @@ -243,14 +224,14 @@ export class Executor { } /** - * Each `:` / `!` op contributes a delete edit per line in its range; if - * any line ends up targeted by deletes originating from two different - * source ops (distinguished by their `lineNum`), the patch is internally + * Each `:` op contributes a delete edit per line in its range; if any + * line ends up targeted by deletes originating from two different source + * ops (distinguished by their `lineNum`), the patch is internally * inconsistent. Identical-range `A-B:` pairs are already collapsed by * `feed()`; remaining shapes here are an `A-B:` that overlaps a later - * `N!`/`N:` with a different range, or two `!` deletes on the same line. - * The applier would run both literally and the file would end up with two - * copies of the line, not a chosen winner. + * `N:` with a different range. The applier would run both literally and + * the file would end up with two copies of the line, not a chosen + * winner. */ #validateNoOverlappingDeletes(): void { const sourceLinesByAnchor = new Map(); @@ -267,7 +248,7 @@ export class Executor { if (sourceLines.length < 2) continue; const [firstOp, secondOp] = [...sourceLines].sort((a, b) => a - b); throw new Error( - `line ${secondOp}: anchor line ${anchorLine} is already targeted by the ${HL_OP_REPLACE}/${HL_OP_DELETE} op on line ${firstOp}. ` + + `line ${secondOp}: anchor line ${anchorLine} is already targeted by the ${HL_OP_REPLACE} op on line ${firstOp}. ` + `Issue ONE op per range; payload is only the final desired content, never a before/after pair.`, ); } @@ -280,7 +261,7 @@ export class Executor { } throw new Error( - `line ${lineNum}: payload line has no preceding ${HL_OP_INSERT_BEFORE}, ${HL_OP_INSERT_AFTER}, ${HL_OP_REPLACE}, or ${HL_OP_DELETE} operation. ` + + `line ${lineNum}: payload line has no preceding ${HL_OP_INSERT_BEFORE}, ${HL_OP_INSERT_AFTER}, or ${HL_OP_REPLACE} operation. ` + `Got ${JSON.stringify(`${HL_PAYLOAD_PREFIX}${text}`)}.`, ); } @@ -304,25 +285,18 @@ export class Executor { // Whitespace-only raw lines outside any pending op are silently dropped; // fully empty lines arrive as `blank` tokens. if (text.trim().length === 0) return; - // Orphan raw text outside any pending op: pick the most specific - // diagnostic so the user sees the actionable hint. - if (isDeleteOpWithPayload(text)) { - throw new Error( - `line ${lineNum}: ${HL_OP_DELETE} deletes only. Payload is forbidden after ${HL_OP_DELETE}; use ${HL_OP_REPLACE} to replace.`, - ); - } const firstChar = text[0]; const startsWithOp = firstChar !== undefined && HL_OP_CHARS.includes(firstChar); if (startsWithOp || firstChar === "-" || firstChar === "@" || firstChar === "«" || firstChar === "»") { throw new Error( - `line ${lineNum}: unrecognized op. Use LINE${HL_OP_INSERT_BEFORE} (insert before), LINE${HL_OP_INSERT_AFTER} (insert after), LINE${HL_OP_REPLACE} / A-B${HL_OP_REPLACE} (replace), or LINE${HL_OP_DELETE} / A-B${HL_OP_DELETE} (delete). ` + + `line ${lineNum}: unrecognized op. Use LINE${HL_OP_INSERT_BEFORE} (insert before), LINE${HL_OP_INSERT_AFTER} (insert after), or LINE${HL_OP_REPLACE} / A-B${HL_OP_REPLACE} (replace). ` + `Got ${JSON.stringify(text)}.`, ); } throw new Error( - `line ${lineNum}: payload line has no preceding ${HL_OP_INSERT_BEFORE}, ${HL_OP_INSERT_AFTER}, ${HL_OP_REPLACE}, or ${HL_OP_DELETE} operation. ` + + `line ${lineNum}: payload line has no preceding ${HL_OP_INSERT_BEFORE}, ${HL_OP_INSERT_AFTER}, or ${HL_OP_REPLACE} operation. ` + `Got ${JSON.stringify(text)}.`, ); } diff --git a/packages/hashline/src/prompt.md b/packages/hashline/src/prompt.md index 9d0ebb74e..0c217f428 100644 --- a/packages/hashline/src/prompt.md +++ b/packages/hashline/src/prompt.md @@ -12,7 +12,6 @@ Patch payload is a series of hunks: `¶PATH#HASH` header followed by any number LINE↑ insert before (or BOF↑) — anchor SURVIVES LINE↓ insert after (or EOF↓) — anchor SURVIVES A-B: replace A..B (or A: == A..A) — anchor DELETED, then payload written in its place -A-B! delete A..B (or A! == A..A) — anchor DELETED +PAYLOAD payload line for the preceding op @@ -24,7 +23,6 @@ A-B! delete A..B (or A! == A..A) — anchor DELETED - **Pick the op for your intent.** Does the anchor's existing content SURVIVE? - Survives + new lines next to it → `↑` / `↓`. Go small: prefer `↑`/`↓` over `:` whenever you can. - Changes in place → `:` - - Goes away → `!` When unsure: you wanted `↓`. `:` is destructive — it deletes the anchor line. - **`A-B:` deletes EXACTLY A..B. Payload length never extends the deletion.** `1:` with 10 payload lines still deletes only line 1, then writes 10 lines there. To prepend without deleting, use `1↑` (or `BOF↑`). - **Line numbers are frozen references to what you have seen.** Later ops in the same hunk still use original line numbers; they do NOT shift as earlier ops apply. @@ -45,7 +43,7 @@ A-B! delete A..B (or A! == A..A) — anchor DELETED 4:f(); ``` -# replace one line, insert after, delete +# replace one line, insert after ``` ¶a.ts#1a2b 1: @@ -53,7 +51,6 @@ A-B! delete A..B (or A! == A..A) — anchor DELETED +export const Y = X; 1↓ +const Z = Y; -4! ``` @@ -93,7 +90,7 @@ A-B! delete A..B (or A! == A..A) — anchor DELETED - One op per range, ever. -- Pick op precisely. Update: `:`, add: `↑`/`↓`, remove: `!`. +- Pick op precisely. Update: `:`, add: `↑`/`↓`. - Payload always lives on its own `+`-prefixed line — never inline with the op. - Payload is only what's NEW; never repeat anchor lines or neighbors. - Anchor exactly; don't anchor neighbors. diff --git a/packages/hashline/src/tokenizer.ts b/packages/hashline/src/tokenizer.ts index f50471061..4b1437857 100644 --- a/packages/hashline/src/tokenizer.ts +++ b/packages/hashline/src/tokenizer.ts @@ -17,7 +17,6 @@ import { describeAnchorExamples, HL_FILE_HASH_SEP, HL_FILE_PREFIX, - HL_OP_DELETE, HL_OP_INSERT_AFTER, HL_OP_INSERT_BEFORE, HL_OP_REPLACE, @@ -214,13 +213,7 @@ interface ParsedReplaceOp { inlineBody: string | undefined; } -interface ParsedDeleteOp { - kind: "delete"; - range: ParsedRange; - trailingPayload: boolean; -} - -type ParsedOp = ParsedInsertOp | ParsedReplaceOp | ParsedDeleteOp; +type ParsedOp = ParsedInsertOp | ParsedReplaceOp; function tryParseInsertOp(line: string, sigil: string, kind: "before" | "after"): ParsedInsertOp | null { const end = trimEndIndex(line); @@ -264,20 +257,11 @@ function tryParseReplaceOp(line: string): ParsedReplaceOp | null { }; } -function tryParseDeleteOp(line: string): ParsedDeleteOp | null { - const end = trimEndIndex(line); - const range = scanRange(line, end); - if (range === null || range.nextIndex >= end || line[range.nextIndex] !== HL_OP_DELETE) return null; - const afterSigil = range.nextIndex + HL_OP_DELETE.length; - return { kind: "delete", range: range.range, trailingPayload: afterSigil !== end }; -} - function tryParseOp(line: string): ParsedOp | null { return ( tryParseInsertOp(line, HL_OP_INSERT_BEFORE, "before") ?? tryParseInsertOp(line, HL_OP_INSERT_AFTER, "after") ?? - tryParseReplaceOp(line) ?? - tryParseDeleteOp(line) + tryParseReplaceOp(line) ); } @@ -323,21 +307,6 @@ function tryParseHeader(line: string): { path: string; fileHash?: string } | nul return fileHash !== undefined ? { path, fileHash } : { path }; } -/** - * Returns true when the line scans as `LINE!payload` (delete sigil followed - * by additional content). The parser uses this for the dedicated "deletes - * only" diagnostic, separate from the standard "unrecognized op" path. - */ -export function isDeleteOpWithPayload(line: string): boolean { - const range = scanRange(line, line.length); - return ( - range !== null && - range.nextIndex < line.length && - line[range.nextIndex] === HL_OP_DELETE && - range.nextIndex + HL_OP_DELETE.length < line.length - ); -} - interface TokenBase { /** 1-indexed line number in the original input stream. */ lineNum: number; @@ -351,7 +320,6 @@ export type Token = | (TokenBase & { kind: "header"; path: string; fileHash?: string }) | (TokenBase & { kind: "op-insert"; cursor: Cursor; inlineBody: string | undefined }) | (TokenBase & { kind: "op-replace"; range: ParsedRange; inlineBody: string | undefined }) - | (TokenBase & { kind: "op-delete"; range: ParsedRange; trailingPayload: boolean }) | (TokenBase & { kind: "payload"; text: string }) | (TokenBase & { kind: "raw"; text: string }); @@ -378,10 +346,7 @@ function classifyLine(line: string, lineNum: number): Token { if (op.kind === "insert") { return { kind: "op-insert", lineNum, cursor: op.cursor, inlineBody: op.inlineBody }; } - if (op.kind === "replace") { - return { kind: "op-replace", lineNum, range: op.range, inlineBody: op.inlineBody }; - } - return { kind: "op-delete", lineNum, range: op.range, trailingPayload: op.trailingPayload }; + return { kind: "op-replace", lineNum, range: op.range, inlineBody: op.inlineBody }; } return { kind: "raw", lineNum, text: line };