Skip to content

fix(core): preserve CRLF endings when splicing patch hunks - #45931

Closed
pengzh1 wants to merge 2 commits into
anomalyco:devfrom
pengzh1:fix/issue-45926
Closed

pengzh1 wants to merge 2 commits into
anomalyco:devfrom
pengzh1:fix/issue-45926

Conversation

@pengzh1

@pengzh1 pengzh1 commented Aug 28, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #45926

Type of change

  • Bug fix

What does this PR do?

Patch.derive split the original on \n, so a CRLF file's lines kept trailing \r while the model's new lines are LF-only (read never shows \r). Splicing the hunk replaced the touched CRLF lines with LF lines, turning a consistently-CRLF file into mixed endings. derive now checks the splice point: when the replaced region (or, for pure insertions, the adjacent line) is CRLF, the inserted lines carry \r too. LF files and mixed files keep their previous behavior.

How did you verify your code works?

Added three unit tests in packages/core/test/patch.test.ts, including the exact repro from the issue (derive("f", [{oldLines:["foo"], newLines:["FOO"]}], "foo\r\nbar\r\n") now yields "FOO\r\nbar\r\n"); they fail on unfixed dev. Existing patch.test.ts + tool-apply-patch.test.ts (18 tests) green, tsgo --noEmit clean.

Screenshots / recordings

Tool behavior fix, no UI.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

read shows the model LF-only text, and apply_patch spliced those LF
lines into CRLF files, converting the touched region to mixed endings.
When the splice point is CRLF, derive now carries the file's endings
onto the inserted lines. Replacement regions use their own majority;
insertions reference the adjacent line.
@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Based on my search, I found one potentially related PR:

The other results (#4604 and #20217) appear to be older, related PRs about CRLF handling but are not current duplicates of this PR.

@pengzh1

pengzh1 commented Aug 28, 2026

Copy link
Copy Markdown
Author

Related but distinct: #45888 fixes the edit tool's matching in mixed-ending files, this one fixes apply_patch's splice (Patch.derive) writing LF lines over CRLF regions — different tool, different code path (packages/core/src/patch.ts), and #45926 notes the edit fix doesn't cover the patch path.

@pengzh1

pengzh1 commented Sep 5, 2026

Copy link
Copy Markdown
Author

Closing this for now — I want to focus on a time-sensitive upstream fix. The branch stays available; I may revisit if the underlying CRLF handling changes.

@pengzh1 pengzh1 closed this Sep 5, 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.

apply_patch turns CRLF files into mixed line endings

1 participant