Skip to content

fix(diffs): Correct line count when edits split or form a CR/LF#956

Open
necolas wants to merge 1 commit into
beta-1.3from
fix-crlf-piece-boundary-linecount
Open

fix(diffs): Correct line count when edits split or form a CR/LF#956
necolas wants to merge 1 commit into
beta-1.3from
fix-crlf-piece-boundary-linecount

Conversation

@necolas

@necolas necolas commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Reproduce:

  1. Open a document whose content is "a\r\nb" (two lines).
  2. Insert "X" between the \r and the \n (offset 2) to make "a\rX\nb".
  3. Ask the document for its line count.

It reports 2 lines instead of 3. In that state positionAt(2) returns {line:0,character:2} instead of {line:1,character:0}, getLineText(2) throws "Line index out of range", and a search for "X" returns the wrong range. The mirror case is also wrong: inserting a "\r" just before an existing "\n" reports an extra line instead of merging the two into one \r\n break.

Each piece derived its line-break count from the shared buffer's offsets, which record a \r\n as one break after the \n. An edit that split a \r\n across a piece boundary, or formed one across two pieces, was then miscounted, corrupting the line count and every position, line-text, and search result that relies on it. The add buffer had the same flaw: appending text that begins with "\n" after a buffer ending in "\r" recorded two breaks for the resulting \r\n.

Count each piece's breaks as if its text stood alone, then reconcile boundary pairs in the tree by folding a \r ending one piece with a \n starting the next into a single break. Drop the stale lone-\r offset in the add buffer when an append merges it into a \r\n. Trim the trailing line break in getTextSlice once at the slice end instead of per piece, so an interior \r split across pieces is preserved.

Reproduce:
1. Open a document whose content is "a\r\nb" (two lines).
2. Insert "X" between the \r and the \n (offset 2) to make
   "a\rX\nb".
3. Ask the document for its line count.

It reports 2 lines instead of 3. In that state positionAt(2)
returns {line:0,character:2} instead of {line:1,character:0},
getLineText(2) throws "Line index out of range", and a search
for "X" returns the wrong range. The mirror case is also wrong:
inserting a "\r" just before an existing "\n" reports an extra
line instead of merging the two into one \r\n break.

Each piece derived its line-break count from the shared buffer's
offsets, which record a \r\n as one break after the \n. An edit
that split a \r\n across a piece boundary, or formed one across
two pieces, was then miscounted, corrupting the line count and
every position, line-text, and search result that relies on it.
The add buffer had the same flaw: appending text that begins
with "\n" after a buffer ending in "\r" recorded two breaks for
the resulting \r\n.

Count each piece's breaks as if its text stood alone, then
reconcile boundary pairs in the tree by folding a \r ending one
piece with a \n starting the next into a single break. Drop the
stale lone-\r offset in the add buffer when an append merges it
into a \r\n. Trim the trailing line break in getTextSlice once
at the slice end instead of per piece, so an interior \r split
across pieces is preserved.
@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pierre-docs-diffs Ready Ready Preview Jul 9, 2026 10:15pm
pierre-docs-diffshub Ready Ready Preview Jul 9, 2026 10:15pm
pierre-docs-trees Ready Ready Preview Jul 9, 2026 10:15pm
pierrejs-diff-demo Ready Ready Preview Jul 9, 2026 10:15pm

Request Review

@ije

ije commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

how can i insert "X" between the \r and the \n (offset 2) to make "a\rX\nb" in the edit file? the fix is great! but seems introduce some extra complex, as we do not expose the piecetable class as API.

@necolas necolas mentioned this pull request Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants