Skip to content

Commit 0c56804

Browse files
committed
fix[apply-patch]: preserve original CRLF/LF line endings on text updates
Context: * Windows edits sometimes flipped CRLF files to LF or created mixed endings. Change: * Detect the original file EOL (treat any \r\n as CRLF; else LF) and normalize the final text to match only for Update File writes. * Added small helper (detect_eol/normalize_to_eol) and a single call at the final write site. * Added focused tests to verify CRLF/LF preservation. Risk: * Minimal and localized. Only affects the text update write path; binary paths and new-file adds are unchanged. Tests: * cargo fmt * cargo clippy -p codex-apply-patch -- -D warnings (clean) * cargo test -p codex-apply-patch (all tests passed; includes new EOL tests) Verification: * Verified on local runs that updating CRLF files preserves CRLF and LF files preserve LF. New-file behavior remains unchanged.
1 parent 7ff142d commit 0c56804

File tree

1 file changed

+88
-4
lines changed
  • codex-rs/apply-patch/src

1 file changed

+88
-4
lines changed

codex-rs/apply-patch/src/lib.rs

Lines changed: 88 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -597,8 +597,16 @@ fn apply_hunks_to_files(hunks: &[Hunk]) -> anyhow::Result<AffectedPaths> {
597597
move_path,
598598
chunks,
599599
} => {
600-
let AppliedPatch { new_contents, .. } =
601-
derive_new_contents_from_chunks(path, chunks)?;
600+
let AppliedPatch {
601+
original_contents,
602+
new_contents,
603+
} = derive_new_contents_from_chunks(path, chunks)?;
604+
let target_eol = detect_eol(&original_contents);
605+
let final_contents = if target_eol == Eol::Crlf {
606+
normalize_to_eol(&new_contents, Eol::Crlf)
607+
} else {
608+
new_contents
609+
};
602610
if let Some(dest) = move_path {
603611
if let Some(parent) = dest.parent()
604612
&& !parent.as_os_str().is_empty()
@@ -607,13 +615,13 @@ fn apply_hunks_to_files(hunks: &[Hunk]) -> anyhow::Result<AffectedPaths> {
607615
format!("Failed to create parent directories for {}", dest.display())
608616
})?;
609617
}
610-
std::fs::write(dest, new_contents)
618+
std::fs::write(dest, final_contents)
611619
.with_context(|| format!("Failed to write file {}", dest.display()))?;
612620
std::fs::remove_file(path)
613621
.with_context(|| format!("Failed to remove original {}", path.display()))?;
614622
modified.push(dest.clone());
615623
} else {
616-
std::fs::write(path, new_contents)
624+
std::fs::write(path, final_contents)
617625
.with_context(|| format!("Failed to write file {}", path.display()))?;
618626
modified.push(path.clone());
619627
}
@@ -840,6 +848,32 @@ pub fn print_summary(
840848
Ok(())
841849
}
842850

851+
// Line ending handling for text files written by apply_patch.
852+
// Detect the original file's EOL and normalize the final text to match.
853+
#[derive(Copy, Clone, Eq, PartialEq)]
854+
enum Eol {
855+
Lf,
856+
Crlf,
857+
}
858+
859+
fn detect_eol(s: &str) -> Eol {
860+
if s.contains("\r\n") {
861+
Eol::Crlf
862+
} else {
863+
Eol::Lf
864+
}
865+
}
866+
867+
fn normalize_to_eol(s: &str, e: Eol) -> String {
868+
match e {
869+
Eol::Lf => s.to_string(),
870+
Eol::Crlf => {
871+
let collapsed = s.replace("\r\n", "\n");
872+
collapsed.replace('\n', "\r\n")
873+
}
874+
}
875+
}
876+
843877
#[cfg(test)]
844878
mod tests {
845879
use super::*;
@@ -1338,6 +1372,56 @@ PATCH"#,
13381372
assert_eq!(String::from_utf8(stderr).unwrap(), "");
13391373
}
13401374

1375+
#[test]
1376+
fn test_update_preserves_crlf_line_endings() {
1377+
let dir = tempdir().unwrap();
1378+
let path = dir.path().join("eol_crlf.txt");
1379+
// Original uses CRLF endings
1380+
std::fs::write(&path, "line1\r\nline2\r\n").unwrap();
1381+
1382+
let patch = wrap_patch(&format!(
1383+
r#"*** Update File: {}
1384+
@@
1385+
line1
1386+
-line2
1387+
+line2-replacement
1388+
+added"#,
1389+
path.display()
1390+
));
1391+
1392+
let mut stdout = Vec::new();
1393+
let mut stderr = Vec::new();
1394+
apply_patch(&patch, &mut stdout, &mut stderr).unwrap();
1395+
1396+
let contents = std::fs::read_to_string(&path).unwrap();
1397+
// All lines should use CRLF after update
1398+
assert_eq!(contents, "line1\r\nline2-replacement\r\nadded\r\n");
1399+
}
1400+
1401+
#[test]
1402+
fn test_update_preserves_lf_line_endings() {
1403+
let dir = tempdir().unwrap();
1404+
let path = dir.path().join("eol_lf.txt");
1405+
std::fs::write(&path, "line1\nline2\n").unwrap();
1406+
1407+
let patch = wrap_patch(&format!(
1408+
r#"*** Update File: {}
1409+
@@
1410+
line1
1411+
-line2
1412+
+line2-replacement
1413+
+added"#,
1414+
path.display()
1415+
));
1416+
1417+
let mut stdout = Vec::new();
1418+
let mut stderr = Vec::new();
1419+
apply_patch(&patch, &mut stdout, &mut stderr).unwrap();
1420+
1421+
let contents = std::fs::read_to_string(&path).unwrap();
1422+
assert_eq!(contents, "line1\nline2-replacement\nadded\n");
1423+
}
1424+
13411425
#[test]
13421426
fn test_unified_diff() {
13431427
// Start with a file containing four lines.

0 commit comments

Comments
 (0)