Skip to content

Preserve line endings when applying patches#30882

Open
charliemarsh-oai wants to merge 3 commits into
mainfrom
charlie/fix-apply-patch-crlf
Open

Preserve line endings when applying patches#30882
charliemarsh-oai wants to merge 3 commits into
mainfrom
charlie/fix-apply-patch-crlf

Conversation

@charliemarsh-oai

@charliemarsh-oai charliemarsh-oai commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • preserve each untouched source line's existing LF or CRLF terminator when applying a patch
  • use the first detected source terminator for inserted and replaced lines, defaulting to LF when the file has none
  • retain parsed context-line identity so repeated or fuzzily matched lines preserve the correct source occurrence
  • rebuild updated files from sorted replacement ranges in one forward pass

Why

apply_patch currently splits source contents on \n, which leaves \r attached to existing CRLF lines, while replacement lines parsed from the patch contain no \r. Joining the result with \n therefore preserves CRLF on untouched lines but emits LF on changed lines, producing mixed-ending files.

The previous fix in #7515 intentionally applied only to Windows. It stripped terminators with str::lines, classified the entire file as CRLF only when every newline was CRLF, and then regenerated the whole file with one separator. That fixed uniform CRLF files, but it coupled behavior to the host OS and normalized mixed-ending files instead of preserving untouched source. The change was reverted wholesale by #7903, leaving the original split/join behavior in place.

Design

This follows the line-ending design we use in Ruff. Ruff's universal newline model recognizes LF, CRLF, and CR and exposes the actual terminator for each source line. Ruff's LineEnding::Auto uses the first detected newline as the preferred style and defaults to LF.

SourceFile applies the same preferred-style rule for new text while remaining more conservative than a formatter: untouched lines retain their exact original terminators, including in mixed-ending files. The patch parser also records which old/new line pairs were context rather than trying to infer equality after parsing. Replacement ranges can then exclude those exact context lines, even when adjacent changed lines have identical text or matching required whitespace normalization.

Closes #4003.

@charliemarsh-oai charliemarsh-oai marked this pull request as ready for review July 2, 2026 00:57
@charliemarsh-oai charliemarsh-oai added bug Something isn't working windows-os Issues related to Codex on Windows systems labels Jul 2, 2026
@charliemarsh-oai charliemarsh-oai marked this pull request as draft July 2, 2026 13:58
@charliemarsh-oai charliemarsh-oai marked this pull request as ready for review July 2, 2026 19:49
@charliemarsh-oai charliemarsh-oai force-pushed the charlie/fix-apply-patch-crlf branch from ef44e33 to ecd347e Compare July 2, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working windows-os Issues related to Codex on Windows systems

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Patched files have mixed line endings on Windows

1 participant