Skip to content

Fix line offsets when incremental edits join CRLF - #1882

Merged
Dirk Bäumer (dbaeumer) merged 3 commits into
microsoft:mainfrom
agammann:fix-crlf-incremental-line-offsets
Oct 20, 2026
Merged

Dirk Bäumer (dbaeumer) merged 3 commits into
microsoft:mainfrom
agammann:fix-crlf-incremental-line-offsets

Conversation

@agammann

Copy link
Copy Markdown
Contributor

Bug

An incremental edit can join a carriage return and line feed across the edit boundary while leaving the cached line offsets stale. For example, inserting \n at line 1, character 0 in a\rb produces a\r\nb, but lineCount returns 3 instead of 2. Line and position lookups then disagree with a fresh document containing the same text.

Fix

Invalidate cached line offsets when an edit leaves a CRLF pair across either edit boundary. The next lookup rebuilds them from the updated content; other edits retain the incremental update path.

Verification

  • The new regression failed on unmodified b4f995b (3 !== 2) and passes with this fix. It covers insertion at either side of the boundary, removal of an intervening character, and a batch of edits.
  • textDocument: npm run all (63 tests passing).
  • Repository: npm run symlink, npm run compile, npm run lint, and npm test passed locally on Node 22.
  • On this Ubuntu 26.04 host, Playwright's browser suites used its Ubuntu 24.04 Chromium build via PLAYWRIGHT_HOST_PLATFORM_OVERRIDE=ubuntu24.04-x64.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The targeted invalidation is correct and the regression tests cover both boundaries and batched updates.

0 open findings

What changed in this PR

Fixes stale line offsets when incremental edits create CRLF pairs across edit boundaries.

Changes:

  • Invalidates cached offsets when edits join CRLF.
  • Adds regression coverage for insertions, removals, and batched edits.
File Description
textDocument/​src/​main.ts Detects boundary-spanning CRLF pairs and rebuilds offsets.
textDocument/​src/​test/​textdocument.test.ts Tests affected line and position APIs.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix correctly handles both edit boundaries and includes comprehensive regression coverage.

0 open findings

🧠 Review effort: Balanced

@dbaeumer
Dirk Bäumer (dbaeumer) enabled auto-merge (squash) October 14, 2026 11:58
Copilot AI balanced review requested due to automatic review settings October 20, 2026 16:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused fix and regression coverage address the reported cache inconsistency, with no unresolved findings.

No open issues 🎉

Review effort: Balanced

@dbaeumer
Dirk Bäumer (dbaeumer) merged commit 8f2cc71 into microsoft:main Oct 20, 2026
5 checks passed
@dbaeumer Dirk Bäumer (dbaeumer) added this to the 3.18.5 milestone Oct 21, 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.

5 participants