From 9097170b08d3205c87e93777ad28c3982161c04a Mon Sep 17 00:00:00 2001 From: derechopiano <257026466+derechopiano@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:33:57 +0100 Subject: [PATCH] XLSX: preserve carriage returns in saved text --- crates/xlsx/src/tests/roundtrip.rs | 41 ++++++++++++++++++++++++++++++ crates/xlsx/src/xml.rs | 16 +++++++++--- docs/file-format-parity.md | 3 ++- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 901802905..a9425fc74 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -693,6 +693,47 @@ fn empty_workbook_roundtrip() { assert!(crate::sniff(&bytes) == crate::Format::Xlsx); } +#[test] +fn cell_text_carriage_returns_survive_xlsx_roundtrip() { + let mut wb = Workbook::new(); + let values = [("A1", "Line 1\r\nLine 2"), ("A2", "Part A\rPart B"), ("A3", "Item 3\nItem 4"), ("A4", "literal _x000D_ marker")]; + let sheet = wb.sheet_mut(0).unwrap(); + for (address, text) in values { + sheet.set_value(at(address), Value::text(text)); + } + let mut formula = Cell::formula(Formula::new("A1")); + formula.value = Value::text("Cached\r\ntext"); + sheet.cells.set(at("B1"), formula); + sheet.comments.insert( + at("A1"), + Comment { + author: "Tester".into(), + text: "Note\r\ntext".into(), + replies: vec![], + threaded: false, + resolved: false, + visible: false, + dates: vec![], + }, + ); + + let bytes = write_xlsx(&wb).unwrap(); + let mut package = Package::open(&bytes).unwrap(); + let shared_strings = String::from_utf8(package.read("xl/sharedStrings.xml").unwrap().unwrap()).unwrap(); + assert!(shared_strings.contains("Line 1_x000D_\nLine 2"), "CR must be encoded before XML parsing"); + assert!(shared_strings.contains("Part A_x000D_Part B")); + assert!(!shared_strings.contains('\r'), "raw CR is normalized to LF by XML parsers"); + let sheet_xml = String::from_utf8(package.read("xl/worksheets/sheet1.xml").unwrap().unwrap()).unwrap(); + assert!(sheet_xml.contains("Cached_x000D_\ntext"), "formula caches also use ST_Xstring"); + + let (back, _) = read_xlsx(&bytes).unwrap(); + for (address, text) in values { + assert_eq!(back.sheets[0].value(at(address)), Value::text(text), "{address} changed on save/reopen"); + } + assert_eq!(back.sheets[0].value(at("B1")), Value::text("Cached\r\ntext")); + assert_eq!(back.sheets[0].comments.get(&at("A1")).map(|c| c.text.as_str()), Some("Note\r\ntext")); +} + #[test] fn writing_repairs_invalid_models() { let mut wb = Workbook::new(); diff --git a/crates/xlsx/src/xml.rs b/crates/xlsx/src/xml.rs index 43bb5b53e..95c2ce0e8 100644 --- a/crates/xlsx/src/xml.rs +++ b/crates/xlsx/src/xml.rs @@ -253,6 +253,7 @@ pub fn esc(s: &str) -> String { '&' => o.push_str("&"), '<' => o.push_str("<"), '>' => o.push_str(">"), + '\r' => o.push_str(" "), c if is_xml_char(c) => o.push(c), _ => {} } @@ -283,15 +284,16 @@ fn is_xml_char(c: char) -> bool { matches!(c, '\t' | '\n' | '\r' | '\u{20}'..='\u{D7FF}' | '\u{E000}'..='\u{FFFD}' | '\u{10000}'..='\u{10FFFF}') } -/// ST_Xstring encoding for cell text: characters XML can't carry become `_xHHHH_`, and literal -/// `_xHHHH_` sequences are protected with `_x005F_`. The result still needs [`esc`]. +/// ST_Xstring encoding for cell text: characters XML can't carry and carriage returns become +/// `_xHHHH_`, and literal `_xHHHH_` sequences are protected with `_x005F_`. The result still needs +/// [`esc`]. pub fn encode_xstring(s: &str) -> String { let mut o = String::with_capacity(s.len()); let chars: Vec = s.chars().collect(); for (i, &c) in chars.iter().enumerate() { if c == '_' && looks_like_escape(&chars, i) { o.push_str("_x005F_"); - } else if !is_xml_char(c) || (c.is_control() && !matches!(c, '\t' | '\n' | '\r')) { + } else if !is_xml_char(c) || (c.is_control() && !matches!(c, '\t' | '\n')) { o.push_str(&format!("_x{:04X}_", c as u32 & 0xFFFF)); } else { o.push(c); @@ -397,11 +399,19 @@ mod tests { #[test] fn xstring() { assert_eq!(decode_xstring("a_x000D_b"), "a\rb"); + assert_eq!(encode_xstring("a\r\nb\rc"), "a_x000D_\nb_x000D_c"); + assert_eq!(decode_xstring(&encode_xstring("a\r\nb\rc")), "a\r\nb\rc"); assert_eq!(encode_xstring("a\u{1}b"), "a_x0001_b"); assert_eq!(decode_xstring(&encode_xstring("_x0041_")), "_x0041_"); assert_eq!(decode_xstring("_x00"), "_x00"); } + #[test] + fn text_escaping_preserves_carriage_returns() { + assert_eq!(esc("a\rb"), "a b"); + assert_eq!(esc_attr("a\rb"), "a b"); + } + #[test] fn numbers() { assert_eq!(num(1.0), "1"); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 988472414..f66ae2b40 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** trivial (XLSX chart sheets are kept on save (#727); previously minor: CSV save keeps the separator it was opened with) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-15 · **Change:** minor (XLSX text retains carriage returns on save/reopen) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-15 | minor | XLSX text preserves CR and CRLF across save/reopen; shared strings use `_x000D_` and other XML text uses ` ` (#1488) | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | | 2026-10-12 | minor | `.xls` import reads the calculation settings (CalcMode, CalcIter, CalcCount, CalcDelta, CalcPrecision), so circular models calculate instead of showing `#CIRC!` (#724) |