fix(hashline-edit): detect overlapping ranges and prevent false unwrap of blank-line spans
- Add detectOverlappingRanges() to reject edits with overlapping pos..end ranges instead of crashing with undefined.match() - Add bounds guard (?? "") in edit-operation-primitives for out-of-range line access - Add null guard in leadingWhitespace() for undefined/empty input - Fix restoreOldWrappedLines false unwrap: skip candidate spans containing blank/whitespace-only lines, preventing incorrect collapse of structural blank lines and indentation (the "애국가 bug") - Improve tool description for range replace clarity - Add tests: overlapping range detection, false unwrap prevention
This commit is contained in:
parent
c7efe8f002
commit
60cf2de16f
@ -15,6 +15,7 @@ export function stripMergeOperatorChars(text: string): string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function leadingWhitespace(text: string): string {
|
function leadingWhitespace(text: string): string {
|
||||||
|
if (!text) return ""
|
||||||
const match = text.match(/^\s*/)
|
const match = text.match(/^\s*/)
|
||||||
return match ? match[0] : ""
|
return match ? match[0] : ""
|
||||||
}
|
}
|
||||||
@ -36,7 +37,9 @@ export function restoreOldWrappedLines(originalLines: string[], replacementLines
|
|||||||
const candidates: { start: number; len: number; replacement: string; canonical: string }[] = []
|
const candidates: { start: number; len: number; replacement: string; canonical: string }[] = []
|
||||||
for (let start = 0; start < replacementLines.length; start += 1) {
|
for (let start = 0; start < replacementLines.length; start += 1) {
|
||||||
for (let len = 2; len <= 10 && start + len <= replacementLines.length; len += 1) {
|
for (let len = 2; len <= 10 && start + len <= replacementLines.length; len += 1) {
|
||||||
const canonicalSpan = stripAllWhitespace(replacementLines.slice(start, start + len).join(""))
|
const span = replacementLines.slice(start, start + len)
|
||||||
|
if (span.some((line) => line.trim().length === 0)) continue
|
||||||
|
const canonicalSpan = stripAllWhitespace(span.join(""))
|
||||||
const original = canonicalToOriginal.get(canonicalSpan)
|
const original = canonicalToOriginal.get(canonicalSpan)
|
||||||
if (original && original.count === 1 && canonicalSpan.length >= 6) {
|
if (original && original.count === 1 && canonicalSpan.length >= 6) {
|
||||||
candidates.push({ start, len, replacement: original.line, canonical: canonicalSpan })
|
candidates.push({ start, len, replacement: original.line, canonical: canonicalSpan })
|
||||||
|
|||||||
@ -63,7 +63,7 @@ export function applyReplaceLines(
|
|||||||
const corrected = autocorrectReplacementLines(originalRange, stripped)
|
const corrected = autocorrectReplacementLines(originalRange, stripped)
|
||||||
const restored = corrected.map((entry, idx) => {
|
const restored = corrected.map((entry, idx) => {
|
||||||
if (idx !== 0) return entry
|
if (idx !== 0) return entry
|
||||||
return restoreLeadingIndent(lines[startLine - 1], entry)
|
return restoreLeadingIndent(lines[startLine - 1] ?? "", entry)
|
||||||
})
|
})
|
||||||
result.splice(startLine - 1, endLine - startLine + 1, ...restored)
|
result.splice(startLine - 1, endLine - startLine + 1, ...restored)
|
||||||
return result
|
return result
|
||||||
|
|||||||
@ -236,6 +236,22 @@ describe("hashline edit operations", () => {
|
|||||||
expect(result).toEqual(["if (x) {", " return 3", " return 4", "}"])
|
expect(result).toEqual(["if (x) {", " return 3", " return 4", "}"])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("preserves blank lines and indentation in range replace (no false unwrap)", () => {
|
||||||
|
//#given — reproduces the 애국가 bug where blank+indented lines collapse
|
||||||
|
const lines = ["", "동해물과 백두산이 마르고 닳도록", "하느님이 보우하사 우리나라 만세", "", "무궁화 삼천리 화려강산", "대한사람 대한으로 길이 보전하세", ""]
|
||||||
|
|
||||||
|
//#when — replace the range with indented version (blank lines preserved)
|
||||||
|
const result = applyReplaceLines(
|
||||||
|
lines,
|
||||||
|
anchorFor(lines, 1),
|
||||||
|
anchorFor(lines, 7),
|
||||||
|
["", " 동해물과 백두산이 마르고 닳도록", " 하느님이 보우하사 우리나라 만세", "", " 무궁화 삼천리 화려강산", " 대한사람 대한으로 길이 보전하세", ""]
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then — all 7 lines preserved with indentation, not collapsed to 3
|
||||||
|
expect(result).toEqual(["", " 동해물과 백두산이 마르고 닳도록", " 하느님이 보우하사 우리나라 만세", "", " 무궁화 삼천리 화려강산", " 대한사람 대한으로 길이 보전하세", ""])
|
||||||
|
})
|
||||||
|
|
||||||
it("collapses wrapped replacement span back to unique original single line", () => {
|
it("collapses wrapped replacement span back to unique original single line", () => {
|
||||||
//#given
|
//#given
|
||||||
const lines = [
|
const lines = [
|
||||||
@ -353,4 +369,33 @@ describe("hashline edit operations", () => {
|
|||||||
//#then
|
//#then
|
||||||
expect(result).toEqual(["const a = 10;", "const b = 20;"])
|
expect(result).toEqual(["const a = 10;", "const b = 20;"])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("throws on overlapping range edits", () => {
|
||||||
|
//#given
|
||||||
|
const content = "line 1\nline 2\nline 3\nline 4\nline 5"
|
||||||
|
const lines = content.split("\n")
|
||||||
|
const edits: HashlineEdit[] = [
|
||||||
|
{ op: "replace", pos: anchorFor(lines, 1), end: anchorFor(lines, 3), lines: "replaced A" },
|
||||||
|
{ op: "replace", pos: anchorFor(lines, 2), end: anchorFor(lines, 4), lines: "replaced B" },
|
||||||
|
]
|
||||||
|
|
||||||
|
//#when / #then
|
||||||
|
expect(() => applyHashlineEdits(content, edits)).toThrow(/overlapping/i)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("allows non-overlapping range edits", () => {
|
||||||
|
//#given
|
||||||
|
const content = "line 1\nline 2\nline 3\nline 4\nline 5"
|
||||||
|
const lines = content.split("\n")
|
||||||
|
const edits: HashlineEdit[] = [
|
||||||
|
{ op: "replace", pos: anchorFor(lines, 1), end: anchorFor(lines, 2), lines: "replaced A" },
|
||||||
|
{ op: "replace", pos: anchorFor(lines, 4), end: anchorFor(lines, 5), lines: "replaced B" },
|
||||||
|
]
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = applyHashlineEdits(content, edits)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toEqual("replaced A\nline 3\nreplaced B")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@ -1,5 +1,5 @@
|
|||||||
import { dedupeEdits } from "./edit-deduplication"
|
import { dedupeEdits } from "./edit-deduplication"
|
||||||
import { collectLineRefs, getEditLineNumber } from "./edit-ordering"
|
import { collectLineRefs, detectOverlappingRanges, getEditLineNumber } from "./edit-ordering"
|
||||||
import type { HashlineEdit } from "./types"
|
import type { HashlineEdit } from "./types"
|
||||||
import {
|
import {
|
||||||
applyAppend,
|
applyAppend,
|
||||||
@ -36,6 +36,9 @@ export function applyHashlineEditsWithReport(content: string, edits: HashlineEdi
|
|||||||
const refs = collectLineRefs(sortedEdits)
|
const refs = collectLineRefs(sortedEdits)
|
||||||
validateLineRefs(lines, refs)
|
validateLineRefs(lines, refs)
|
||||||
|
|
||||||
|
const overlapError = detectOverlappingRanges(sortedEdits)
|
||||||
|
if (overlapError) throw new Error(overlapError)
|
||||||
|
|
||||||
for (const edit of sortedEdits) {
|
for (const edit of sortedEdits) {
|
||||||
switch (edit.op) {
|
switch (edit.op) {
|
||||||
case "replace": {
|
case "replace": {
|
||||||
|
|||||||
@ -27,3 +27,30 @@ export function collectLineRefs(edits: HashlineEdit[]): string[] {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function detectOverlappingRanges(edits: HashlineEdit[]): string | null {
|
||||||
|
const ranges: { start: number; end: number; idx: number }[] = []
|
||||||
|
for (let i = 0; i < edits.length; i++) {
|
||||||
|
const edit = edits[i]
|
||||||
|
if (edit.op !== "replace" || !edit.end) continue
|
||||||
|
const start = parseLineRef(edit.pos).line
|
||||||
|
const end = parseLineRef(edit.end).line
|
||||||
|
ranges.push({ start, end, idx: i })
|
||||||
|
}
|
||||||
|
if (ranges.length < 2) return null
|
||||||
|
|
||||||
|
ranges.sort((a, b) => a.start - b.start || a.end - b.end)
|
||||||
|
for (let i = 1; i < ranges.length; i++) {
|
||||||
|
const prev = ranges[i - 1]
|
||||||
|
const curr = ranges[i]
|
||||||
|
if (curr.start <= prev.end) {
|
||||||
|
return (
|
||||||
|
`Overlapping range edits detected: ` +
|
||||||
|
`edit ${prev.idx + 1} (lines ${prev.start}-${prev.end}) overlaps with ` +
|
||||||
|
`edit ${curr.idx + 1} (lines ${curr.start}-${curr.end}). ` +
|
||||||
|
`Use pos-only replace for single-line edits.`
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
|||||||
@ -7,6 +7,7 @@ function equalsIgnoringWhitespace(a: string, b: string): boolean {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function leadingWhitespace(text: string): string {
|
function leadingWhitespace(text: string): string {
|
||||||
|
if (!text) return ""
|
||||||
const match = text.match(/^\s*/)
|
const match = text.match(/^\s*/)
|
||||||
return match ? match[0] : ""
|
return match ? match[0] : ""
|
||||||
}
|
}
|
||||||
|
|||||||
@ -34,8 +34,8 @@ FILE CREATION:
|
|||||||
CRITICAL: only unanchored append/prepend can create a missing file.
|
CRITICAL: only unanchored append/prepend can create a missing file.
|
||||||
|
|
||||||
OPERATION CHOICE:
|
OPERATION CHOICE:
|
||||||
replace with pos only -> replace one line at pos
|
replace with pos only -> replace one line at pos (MOST COMMON for single-line edits)
|
||||||
replace with pos+end -> replace range pos..end
|
replace with pos+end -> replace ENTIRE range pos..end as a block (ranges MUST NOT overlap across edits)
|
||||||
append with pos/end anchor -> insert after that anchor
|
append with pos/end anchor -> insert after that anchor
|
||||||
prepend with pos/end anchor -> insert before that anchor
|
prepend with pos/end anchor -> insert before that anchor
|
||||||
append/prepend without anchors -> EOF/BOF insertion
|
append/prepend without anchors -> EOF/BOF insertion
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user