Repository navigation
fix: don't treat chunks emptied by an overwrite as removed in replace - #367
Merged
antfu merged 2 commits intoOct 26, 2026
Merged
Conversation
`update()` and `overwrite()` over a range that spans several chunks store the replacement on the first chunk and empty the others. `_hasRemovedContent()` took those for removed content, so `replace()` and `replaceAll()` skipped a match over the range whenever an earlier edit had split it, even an insert of nothing. The first chunk now records where the overwritten range ends, and the others where it starts. An emptied chunk counts as present while the match includes the first chunk and that chunk's replacement still reaches it. A match that leaves the first chunk out is still skipped, so the replacement is never doubled.
This was referenced Oct 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since #352,
replace()andreplaceAll()skip a match that lands on removed content, and_hasRemovedContent()decides that by looking for an empty chunk in the matched range.update()andoverwrite()leave empty chunks too: when the range spans several chunks, the replacement goes on the first one and the others are emptied. Their text was replaced, not removed, but a match over them is skipped all the same. So whether a match is replaced depends on whether some earlier edit happened to split the range, which the caller cannot see.Reproductions
All checked against current master (
[email protected]).The only difference between these two is a split that inserts nothing:
The same happens with
replace(), with a string pattern, and withoverwrite()in place ofupdate(). Any earlier edit that splits the range has this effect, not only an empty insert:A range that an earlier
replaceAll()overwrote cannot be matched again either:The fix
update()now records on the chunks it empties where the overwritten range starts, and on the first chunk where the range ends (replacedFromandreplacesUntilonChunk).remove()andreset()clear both, a split keepsreplacedFromon both halves, andclone()copies them._hasRemovedContent()treats an emptied chunk as present when the match includes the first chunk of that overwrite and that chunk's replacement still reaches it. Removed chunks, trimmed ones and ranges overwritten with''are unaffected.Both conditions matter:
update(0, 3, 'Q')on'abc'split at 1,replace('bc', 'P')still gives'Q'. Overwriting only the emptied chunks would leave'Q'in place and add'P'after it. Without the split, the same call throwscannot split a chunk that has already been edited.update(0, 1, 'R')orreset(0, 1)followed byreplace('abc', 'P')still gives'R'or'a'. Recording only where the range starts would bring those chunks back. With an empty replacement it would even make the result depend on the split again, sinceupdate(0, 2, '')followed byupdate(0, 1, 'Y')can be reached with or without it.One consequence to be aware of: a match that includes the first chunk but ends inside the range is now replaced, and the rest of the range loses its replacement, as it does when
overwrite()is called on that part directly. Master already did this when the match ended exactly where the first chunk ends, and skipped the match when it ended further in. So on'abc',update(0, 3, 'Q')thenreplace('ab', 'P')gives'P'with a split at 2 and'Q'with a split at 1 (and throws with none). This branch gives'P'for both. Skipping both would also be consistent, but it changes far more existing results (2,588 against 144 in the comparison below), because the first case is the common one.Evidence
The same metamorphic fuzz as in #361: random sequences of edits on random strings, each run with and without a few
appendLeft(k, '')splits, comparingtoString(),slice(),lastChar(),lastLine(),isEmpty(),hasChanged()andlength(). Divergences fromreplace()/replaceAll():replace/replaceAllupdate,overwrite,remove,replace,replaceAllWith #361 merged, these are the only divergences left on master in both runs.
The fuzz never uses an empty replacement and never trims one away, so I also checked exhaustively on
'abcde': every sequence of one or two edits (remove,updatewith content,updatewith'',reset) over every range, under every placement of zero, one or two empty inserts, followed byreplace/replaceAllof every substring, as a string and as a RegExp, 1,811,700 cases in all. Each result is compared with the one without the inserts, and each skip decision with a per-character model of which text is still represented in the output (the model's rendering matchedtoString()in every case):The 596 cases that now throw are matches that master skipped and that also end inside another overwritten chunk, so overwriting them throws
cannot split a chunk that has already been edited. The same calls throw on master as well when the range is not split.A second exhaustive check runs
trim(),trimStart(),trimEnd()ortrimLines()between the edits and the replace, with whitespace replacements that the trim can empty: 228,830 of 6,436,950 comparisons depend on the inserts on master, 0 on this branch.On the same random trials, 145 of 355,590 runs give a different result than on master. 142 of them are matches that master skipped and this branch replaces, as described above. In one trial (2 runs) a later
replaceAll('a', 'P')now throwscannot split a chunk that has already been edited, because it falls inside a range that a partial match before it now replaces. One run no longer throws.3 tests added: two fail on master, and the third pins down the matches that must still be skipped. The 454 existing tests pass unchanged. Lint is clean, and
pnpm run typecheckreports the sameTS2341errors as on master.This touches
_hasRemovedContent()like #366, which fixes empty matches at a chunk boundary inside removed content. The two only conflict in the test file, where both add a test at the end of the same block. I checked the combination: all 460 tests pass, the fuzz runs above show no divergences, and both exhaustive checks (and the one from #366) report 0.