From b13ea3e2f37d25a1dee1507117f5bd5dce156428 Mon Sep 17 00:00:00 2001 From: Vedran Dobos <19259909+vedran-dobos@users.noreply.github.com> Date: Sun, 11 Oct 2026 23:38:53 -0700 Subject: [PATCH] CSV: saving keeps the separator the file was opened with (#426) A `;`-separated CSV (the usual CSV where `,` is the decimal separator) was rewritten with `,` on save: read_csv sniffed the delimiter but the result was dropped, and save_bytes passed a hard-coded b','. file.save, Save As .csv, file.exportCsv, file.saveBytes and gridcraft-cli convert all went through it. The workbook now remembers the separator it was read with (Workbook::csv_delimiter, set by read_csv) and a CSV save writes it back. Workbooks not read from CSV, and .tsv/.txt sources saved as .csv, keep `,`. Regression tests: `;`, `,` and tab .csv round trips byte for byte, new and .xlsx-origin workbooks save with `,`, and open-by-path + edit + file.save on a `;` file keeps `;` (also through exportCsv and saveBytes). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/io.rs | 46 +++++++++++++++++++++++++++++- crates/engine/src/tests.rs | 27 ++++++++++++++++++ crates/model/src/workbook.rs | 5 ++++ crates/xlsx/src/csv.rs | 1 + crates/xlsx/src/tests/csv_tests.rs | 5 ++++ docs/file-format-parity.md | 5 ++-- 6 files changed, 86 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index e388f87..6a8804b 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -85,6 +85,8 @@ pub fn open_bytes(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { Some(FileKind::Tsv) => { let opts = gridcraft_xlsx::CsvOptions { delimiter: b'\t', ..Default::default() }; let mut wb = gridcraft_xlsx::read_csv(bytes, &opts).map_err(|e| EngineError::Other(e.to_string()))?; + // Tab is what makes it a .tsv/.txt; saved as .csv it gets the CSV default. + wb.csv_delimiter = None; rename_first_sheet(&mut wb, name); Ok((wb, vec![])) } @@ -126,7 +128,8 @@ pub fn save_bytes(wb: &Workbook, path: &str) -> Result> { } FileKind::Xlsb => Err(EngineError::Other("XLSB is supported for data import only. Save as .xlsx or another supported export format.".into())), FileKind::Ods => Err(EngineError::Other("ODS is supported for data import only. Save as .xlsx or another supported export format.".into())), - FileKind::Csv => Ok(wb.sheet(sheet).map(|sh| gridcraft_xlsx::write_csv(sh, wb, b',')).unwrap_or_default()), + // A workbook read from CSV keeps the separator it was read with (a `;` file stays `;`). + FileKind::Csv => Ok(wb.sheet(sheet).map(|sh| gridcraft_xlsx::write_csv(sh, wb, wb.csv_delimiter.unwrap_or(b','))).unwrap_or_default()), FileKind::Tsv => Ok(wb.sheet(sheet).map(|sh| gridcraft_xlsx::write_csv(sh, wb, b'\t')).unwrap_or_default()), FileKind::Json => serde_json::to_vec_pretty(wb).map_err(|e| EngineError::Other(e.to_string())), FileKind::Html => Ok(to_html(wb, sheet).into_bytes()), @@ -410,6 +413,47 @@ mod tests { b } + /// Cut-list rows as a CAD/CAM program exports them: `;`-separated, CRLF, part codes in the last + /// two columns, a part name containing a comma. + const CUT_LIST: &str = "1;511;184;P3_L1_led;2162PE;16;1MM_2162PE;1MM_2162PE;;;1244121406FB;\r\n\ + 1;562.5;510;Komoda, lijevo;2162PE;19;1MM_2162PE;;;;12441214071A;124412140710\r\n"; + + #[test] + fn csv_save_keeps_the_separator_it_was_opened_with() { + let (wb, _) = open_bytes("cut list.csv", CUT_LIST.as_bytes()).unwrap(); + assert_eq!(wb.csv_delimiter, Some(b';')); + assert_eq!(String::from_utf8(save_bytes(&wb, "cut list.csv").unwrap()).unwrap(), CUT_LIST); + // Save As another .csv name keeps it too; a comma file stays a comma file (quoting only + // fields that contain the separator in use). + assert_eq!(String::from_utf8(save_bytes(&wb, "copy.CSV").unwrap()).unwrap(), CUT_LIST); + let commas = "\"Komoda, lijevo\",b\r\n1,x;y\r\n"; + let (wb, _) = open_bytes("plain.csv", commas.as_bytes()).unwrap(); + assert_eq!(String::from_utf8(save_bytes(&wb, "plain.csv").unwrap()).unwrap(), commas); + // A tab-separated file named .csv round-trips with tabs. + let tabs = "a\tb\r\n1\t2\r\n"; + let (wb, _) = open_bytes("tabs.csv", tabs.as_bytes()).unwrap(); + assert_eq!(String::from_utf8(save_bytes(&wb, "tabs.csv").unwrap()).unwrap(), tabs); + } + + #[test] + fn csv_save_of_a_workbook_not_read_from_csv_uses_commas() { + let mut wb = Workbook::new(); + if let Some(sh) = wb.sheet_mut(0) { + sh.set_value(CellRef::new(0, 0), gridcraft_core::Value::text("a")); + sh.set_value(CellRef::new(0, 1), gridcraft_core::Value::Number(1.0)); + } + assert_eq!(save_bytes(&wb, "new.csv").unwrap(), b"a,1\r\n"); + // Through .xlsx the CSV origin is gone, as in Excel. + let (from_csv, _) = open_bytes("cut list.csv", CUT_LIST.as_bytes()).unwrap(); + let (reopened, _) = open_bytes("book.xlsx", &save_bytes(&from_csv, "book.xlsx").unwrap()).unwrap(); + assert_eq!(reopened.csv_delimiter, None); + assert!(String::from_utf8(save_bytes(&reopened, "again.csv").unwrap()).unwrap().starts_with("1,511,184,")); + // A .tsv/.txt saved as .csv gets commas; as .tsv it keeps tabs. + let (tsv, _) = open_bytes("list.txt", b"a\tb\r\n").unwrap(); + assert_eq!(save_bytes(&tsv, "list.csv").unwrap(), b"a,b\r\n"); + assert_eq!(save_bytes(&tsv, "list.tsv").unwrap(), b"a\tb\r\n"); + } + #[test] fn encrypted_workbook_is_reported_not_misread() { let err = open_bytes("secret.xlsx", &cfb(&["EncryptionInfo", "EncryptedPackage"])).unwrap_err(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c9f4d0a..b89f2f4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1257,6 +1257,33 @@ fn ods_import_never_reuses_source_as_save_target() { std::fs::remove_dir_all(dir).unwrap(); } +/// Open a `;` CSV by path (a double-click), edit, Save: the file keeps its `;` separator. +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn saving_an_opened_semicolon_csv_keeps_semicolons() { + #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("gridcraft-csv-{}-{nonce}", std::process::id())); + std::fs::create_dir(&dir).unwrap(); + let path = dir.join("cut list.csv"); + std::fs::write(&path, "1;511;184;P3_L1_led;2162PE;16;;1244121406FB\r\n1;562;510;P3_pod;2162PE;19;;124412140710\r\n").unwrap(); + let mut s = s(); + s.execute("file.open", json!({"path": path})).unwrap(); + s.execute("cell.set", json!({"cell": "B2", "input": "600"})).unwrap(); + s.execute("file.save", json!({})).unwrap(); + assert_eq!( + std::fs::read_to_string(&path).unwrap(), + "1;511;184;P3_L1_led;2162PE;16;;1244121406FB\r\n1;600;510;P3_pod;2162PE;19;;124412140710\r\n" + ); + let exported = dir.join("export.csv"); + s.execute("file.exportCsv", json!({"path": exported})).unwrap(); + assert!(std::fs::read_to_string(&exported).unwrap().starts_with("1;511;184;")); + let saved = s.execute("file.saveBytes", json!({"format": "csv"})).unwrap(); + let bytes = crate::io::base64_decode(saved["base64"].as_str().unwrap()).unwrap(); + assert!(bytes.starts_with(b"1;511;184;")); + std::fs::remove_dir_all(dir).unwrap(); +} + #[test] fn validation_formulas_are_relative_and_see_the_new_value() { let mut s = s(); diff --git a/crates/model/src/workbook.rs b/crates/model/src/workbook.rs index e4be016..82ddd04 100644 --- a/crates/model/src/workbook.rs +++ b/crates/model/src/workbook.rs @@ -79,6 +79,10 @@ pub struct Workbook { /// macros it arrived with. base64 in JSON. `None` = the workbook has no macros. #[serde(default, with = "crate::b64::opt")] pub vba_project: Option>, + /// Field separator of the CSV this workbook was read from, so saving it as CSV writes the same + /// separator back (a `;` file stays a `;` file). `None` = not read from CSV: CSV saves use `,`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub csv_delimiter: Option, /// Unmodelled file content kept for saving (package root and workbook level). Not in JSON. #[serde(skip)] pub preserved: Option>, @@ -107,6 +111,7 @@ impl Workbook { custom_lists: vec![], cell_styles: vec![], vba_project: None, + csv_delimiter: None, preserved: None, } } diff --git a/crates/xlsx/src/csv.rs b/crates/xlsx/src/csv.rs index 315bc96..7b5ed45 100644 --- a/crates/xlsx/src/csv.rs +++ b/crates/xlsx/src/csv.rs @@ -139,6 +139,7 @@ pub fn read_csv(bytes: &[u8], opts: &CsvOptions) -> Result { } let mut wb = Workbook::new(); wb.date_system = opts.date_system; + wb.csv_delimiter = u8::try_from(delim).ok(); let mut sheet = Sheet::new("Sheet1"); let mut styles = std::mem::take(&mut wb.styles); let mut fmt_cache: Vec<(&'static str, StyleId)> = vec![]; diff --git a/crates/xlsx/src/tests/csv_tests.rs b/crates/xlsx/src/tests/csv_tests.rs index 480380b..cb0012b 100644 --- a/crates/xlsx/src/tests/csv_tests.rs +++ b/crates/xlsx/src/tests/csv_tests.rs @@ -80,6 +80,11 @@ fn delimiter_sniffing() { assert_eq!(sniff_delimiter("a;b\r\n\r\nc;d\r\n", '"'), b';'); let wb = read_csv(b"x;y\n1,5;2", &CsvOptions { delimiter: 0, ..Default::default() }).unwrap(); assert_eq!(v(&wb, 1, 1), Value::Number(2.0)); + // The workbook remembers the separator it was read with, sniffed or given. + assert_eq!(wb.csv_delimiter, Some(b';')); + assert_eq!(read_csv(b"a\tb", &CsvOptions { delimiter: 0, ..Default::default() }).unwrap().csv_delimiter, Some(b'\t')); + assert_eq!(read_csv(b"a;b", &CsvOptions::default()).unwrap().csv_delimiter, Some(b',')); + assert_eq!(Workbook::new().csv_delimiter, None); assert!(read_csv(b"a", &CsvOptions { delimiter: b'"', ..Default::default() }).is_err()); } diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 06dd452..0442bd3 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-11 · **Change:** minor (`.xls` BIFF8 import, #121) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (CSV save keeps the separator it was opened with) · **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** @@ -34,7 +34,7 @@ support comes from `crates/engine/src/io.rs` (`FileKind`), `crates/xlsx/src` and | Format | Excel | GridCraft read | GridCraft write | Notes | |---|---|---|---|---| -| `.csv` UTF-8 / Windows / Mac / MS-DOS | R/W | yes (UTF-8 and UTF-16 BOMs, UTF-8, Windows-1252 fallback; delimiter detection) | yes (UTF-8 without BOM, CRLF) | Excel writes the **displayed** values; we write raw values. No encoding choice on save. Re-import of a single-column export splits text (#127) | +| `.csv` UTF-8 / Windows / Mac / MS-DOS | R/W | yes (UTF-8 and UTF-16 BOMs, UTF-8, Windows-1252 fallback; delimiter detection) | yes (UTF-8 without BOM, CRLF; a file opened as CSV is saved with the separator it was read with, so a `;` file stays `;`; otherwise `,`) | Excel writes the **displayed** values; we write raw values. No encoding choice on save. Re-import of a single-column export splits text (#127) | | `.txt` tab-delimited, Unicode text | R/W | yes (`.txt`, `.tsv`, `.tab`) | yes | Text Import Wizard options partly via Text to Columns | | `.prn` formatted text (space-delimited) | R/W | no | no | | | `.dif`, `.slk` (SYLK) | R/W | no | no | Small, legacy | @@ -76,6 +76,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | CSV: saving a file opened as CSV keeps its detected separator (`;` files were rewritten with `,`) | | 2026-10-11 | minor | `.xls` (BIFF8) import: values, formulas, number formats, merges, sizes, names (#121) | | 2026-10-11 | minor | XLSX pass-through: custom XML, external links, connections, query tables, printer settings, form/ActiveX controls, OLE objects, sheet backgrounds kept on save (#330, #324) | | 2026-10-11 | minor | XLSX: threaded comments and persons parts written (they came back as notes before); note visibility read from VML (#300) |