Skip to content

Commit

Permalink
Make sure from/to differences are computed from clipped values
Browse files Browse the repository at this point in the history
FIX: Fix an issue where `Text.slice` and `Text.replace` could return objects
with incorrect `length` when the given `from`/`to` values were out of range
for the text.

Closes codemirror/dev#1303
  • Loading branch information
marijnh committed Dec 5, 2023
1 parent 5771dc5 commit 88ecc25
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
11 changes: 11 additions & 0 deletions src/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export abstract class Text implements Iterable<string> {

/// Replace a range of the text with the given content.
replace(from: number, to: number, text: Text): Text {
;[from, to] = clip(this, from, to)
let parts: Text[] = []
this.decompose(0, from, parts, Open.To)
if (text.length) text.decompose(0, text.length, parts, Open.From | Open.To)
Expand All @@ -66,6 +67,7 @@ export abstract class Text implements Iterable<string> {

/// Retrieve the text between the given points.
slice(from: number, to: number = this.length): Text {
;[from, to] = clip(this, from, to)
let parts: Text[] = []
this.decompose(from, to, parts, 0 as Open)
return TextNode.from(parts, to - from)
Expand Down Expand Up @@ -198,13 +200,15 @@ class TextLeaf extends Text {

replace(from: number, to: number, text: Text): Text {
if (!(text instanceof TextLeaf)) return super.replace(from, to, text)
;[from, to] = clip(this, from, to)
let lines = appendText(this.text, appendText(text.text, sliceText(this.text, 0, from)), to)
let newLen = this.length + text.length - (to - from)
if (lines.length <= Tree.Branch) return new TextLeaf(lines, newLen)
return TextNode.from(TextLeaf.split(lines, []), newLen)
}

sliceString(from: number, to = this.length, lineSep = "\n") {
;[from, to] = clip(this, from, to)
let result = ""
for (let pos = 0, i = 0; pos <= to && i < this.text.length; i++) {
let line = this.text[i], end = pos + line.length
Expand Down Expand Up @@ -274,6 +278,7 @@ class TextNode extends Text {
}

replace(from: number, to: number, text: Text): Text {
;[from, to] = clip(this, from, to)
if (text.lines < this.lines) for (let i = 0, pos = 0; i < this.children.length; i++) {
let child = this.children[i], end = pos + child.length
// Fast path: if the change only affects one child and the
Expand All @@ -296,6 +301,7 @@ class TextNode extends Text {
}

sliceString(from: number, to = this.length, lineSep = "\n") {
;[from, to] = clip(this, from, to)
let result = ""
for (let i = 0, pos = 0; i < this.children.length && pos <= to; i++) {
let child = this.children[i], end = pos + child.length
Expand Down Expand Up @@ -569,3 +575,8 @@ export class Line {
/// The length of the line (not including any line break after it).
get length() { return this.to - this.from }
}

function clip(text: Text, from: number, to: number) {
from = Math.max(0, Math.min(text.length, from))
return [from, Math.max(from, Math.min(text.length, to))]
}
8 changes: 8 additions & 0 deletions test/test-text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,12 @@ describe("Text", () => {
ist(doc0.slice(from, to).toString(), text0.slice(from, to))
}
})

it("clips out-of-range boundaries", () => {
ist(doc0.slice(0, -10).length, 0)
ist(Text.empty.slice(0, 10).length, 0)
ist(Text.empty.slice(1000, 1100).length, 0)
ist(doc0.slice(5, 0).length, 0)
ist(doc0.slice(-5, 0).length, 0)
})
})

0 comments on commit 88ecc25

Please sign in to comment.