Skip to content

nes: fix: NES cache should respect line endings when applying edits to document contents '#308479

Merged
ulugbekna merged 1 commit intomainfrom
ulugbekna/nes-fix-windows-caching
Apr 8, 2026
Merged

nes: fix: NES cache should respect line endings when applying edits to document contents '#308479
ulugbekna merged 1 commit intomainfrom
ulugbekna/nes-fix-windows-caching

Conversation

@ulugbekna
Copy link
Copy Markdown
Contributor

@ulugbekna ulugbekna commented Apr 8, 2026

otherwise, cached document contents do not match because of those line endings

should fix https://github.com/microsoft/vscode-copilot-issues/issues/429

…o document contents '

otherwise, cached document contents do not match because of those line endings
Copilot AI review requested due to automatic review settings April 8, 2026 10:54
@ulugbekna ulugbekna enabled auto-merge (squash) April 8, 2026 10:54
@ulugbekna ulugbekna self-assigned this Apr 8, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Copilot Next Edit Suggestion (NES) pipeline to preserve a document’s line-ending style (LF vs CRLF) when converting/applying multi-line edits, preventing cached document contents from diverging due to EOL mismatches.

Changes:

  • Normalize replacement text to CRLF when the target document content uses CRLF line endings.
  • Refactor test setup to reuse a shared stateless provider factory.
  • Add a new caching/rebasing test that simulates a CRLF (“Windows”) document and asserts CRLF preservation across cached edits.
Show a summary per file
File Description
extensions/copilot/src/extension/inlineEdits/node/nextEditProvider.ts Adds CRLF-aware normalization when converting line-based edits into string edits.
extensions/copilot/src/extension/inlineEdits/test/node/nextEditProviderCaching.spec.ts Adds a CRLF-focused regression test and refactors provider construction into a helper.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment on lines +87 to +89
return new StringEdit(suggestedEdit.replacements.map(
r => new StringReplacement(r.replaceRange, r.newText.replace(/\n/g, '\r\n'))
));
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

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

The CRLF normalization here is not idempotent: r.newText.replace(/\n/g, '\r\n') will turn any existing \r\n in newText into \r\r\n. Since StringReplacement/StringEdit already provide normalizeEOL(eol) (which replaces \r\n|\n safely), prefer using that to normalize to CRLF when needed (e.g. suggestedEdit.normalizeEOL('\r\n')) rather than manually replacing \n.

This avoids introducing mixed/invalid EOL sequences in replacement text if a provider ever returns \r\n (or if replacement text is composed from sources that already contain CRLF).

Suggested change
return new StringEdit(suggestedEdit.replacements.map(
r => new StringReplacement(r.replaceRange, r.newText.replace(/\n/g, '\r\n'))
));
return suggestedEdit.normalizeEOL('\r\n');

Copilot uses AI. Check for mistakes.
@ulugbekna ulugbekna merged commit 5be5cff into main Apr 8, 2026
27 checks passed
@ulugbekna ulugbekna deleted the ulugbekna/nes-fix-windows-caching branch April 8, 2026 11:17
@vs-code-engineering vs-code-engineering Bot added this to the 1.116.0 milestone Apr 8, 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