From e23994b7cb990416caee69b871dd140c1fba63d7 Mon Sep 17 00:00:00 2001 From: Hector Villarreal Date: Tue, 3 Nov 2026 01:17:35 -0600 Subject: [PATCH] #3166: a web page's table keeps its characters, its shown text and its header formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit review on #3223: - "Decode HTML before parsing its cells": a page in Windows-1252 read as lossy UTF-8, so "Café" lost its é. The page is now decoded as a CSV is (BOM, UTF-8, else Windows-1252/1256); decode_text is exported for it. - "Keep visible cell text when csv_as_text is enabled": opened as text, a cell with x:num took the recorded number ("4,50" became "4.5"). The recorded number is used only when values are read. - "Preserve the imported format when styling a header": a date in a lost its date format when made bold and centred. Header styling now builds on the style the CSV reader gave the cell. Test: a_web_page_table_keeps_its_characters_text_and_header_formats. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/io.rs | 36 ++++++++++++++++++++++------------- crates/engine/src/io/tests.rs | 20 +++++++++++++++++++ crates/xlsx/src/lib.rs | 2 +- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 9e3c7f7ae..f62a55e40 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -355,7 +355,8 @@ fn part_too_large_message(name: &str, part: &str, size: u64) -> String { /// header cells (``) are bold and centred, and the sheet takes the name the page gives it /// (``) or the file's. `None` when the bytes aren't such a page. fn read_html_table(name: &str, kind: Option, bytes: &[u8], csv_as_text: bool) -> Option)>> { - let text = String::from_utf8_lossy(bytes); + // Decoded as a CSV is (BOM, UTF-8, else Windows-1252/1256), so a legacy page keeps its letters. + let text = gridcraft_xlsx::decode_text(bytes); let head: String = text.trim_start_matches('\u{feff}').trim_start().chars().take(16).collect::().to_ascii_lowercase(); if ![", bytes: &[u8], csv_as_text let fields: Vec = row .iter() .map(|cell| { - let field = match cell.num { + // The number the page recorded (`x:num`) only when values are read; as text, the + // cell keeps the text it shows. + let field = match cell.num.filter(|_| !csv_as_text) { Some(n) => gridcraft_core::number_to_text(n), None => cell.text.split_whitespace().collect::>().join(" "), }; @@ -384,17 +387,24 @@ fn read_html_table(name: &str, kind: Option, bytes: &[u8], csv_as_text let read = gridcraft_xlsx::read_csv_with_report(tsv.as_bytes(), &opts).map_err(|e| EngineError::Other(e.to_string())); Some(read.map(|(mut wb, report)| { wb.csv_delimiter = None; - let header_style = wb.styles.derive(gridcraft_model::StyleId::DEFAULT, |st| { - st.font.bold = true; - st.align.h = gridcraft_model::HAlign::Center; - }); - if let Some(sh) = wb.sheet_mut(0) { - for (r, row) in rows.iter().enumerate() { - for (c, cell) in row.iter().enumerate() { - if cell.header && !cell.text.trim().is_empty() { - sh.set_style(CellRef::new(r as u32, c as u32), header_style); - } - } + // Header cells are bold and centred on top of the style the CSV reader gave them, so a + // date in a header keeps its date format. + let headers: Vec = rows + .iter() + .enumerate() + .flat_map(|(r, row)| row.iter().enumerate().filter(|(_, cell)| cell.header && !cell.text.trim().is_empty()).map(move |(c, _)| CellRef::new(r as u32, c as u32))) + .collect(); + let mut bold: std::collections::HashMap = std::collections::HashMap::new(); + for at in headers { + let Some(base) = wb.sheet(0).map(|sh| sh.style_id(at)) else { continue }; + let id = *bold.entry(base).or_insert_with(|| { + wb.styles.derive(base, |st| { + st.font.bold = true; + st.align.h = gridcraft_model::HAlign::Center; + }) + }); + if let Some(sh) = wb.sheet_mut(0) { + sh.set_style(at, id); } } let page_name = find_between(&text, "", "").map(|n| crate::clip_html::normalized(n.trim())); diff --git a/crates/engine/src/io/tests.rs b/crates/engine/src/io/tests.rs index 34b1a3c0a..fab60261f 100644 --- a/crates/engine/src/io/tests.rs +++ b/crates/engine/src/io/tests.rs @@ -636,3 +636,23 @@ fn a_web_page_saved_as_xls_opens_its_table() { let (wb, _) = open_bytes("notes.csv", b",1\nx,2\n").unwrap(); assert_eq!(wb.sheet(0).unwrap().value(CellRef::parse("B2").unwrap()), gridcraft_core::Value::number(2.0)); } + +#[test] +fn a_web_page_table_keeps_its_characters_text_and_header_formats() { + // A page saved in Windows-1252 keeps its accented letters (it read as replacement characters). + let mut page = b"
Caf".to_vec(); + page.push(0xE9); + page.extend_from_slice(b"2024-01-15
Plum4,50
"); + let (wb, _) = open_bytes("menu.xls", &page).unwrap(); + let sh = wb.sheet(0).unwrap(); + assert_eq!(sh.value(CellRef::parse("A1").unwrap()), gridcraft_core::Value::text("Café")); + // A date in a header cell stays a date, bold and centred (its format was replaced). + let b1 = wb.styles.get(sh.style_id(CellRef::parse("B1").unwrap())); + assert!(matches!(sh.value(CellRef::parse("B1").unwrap()), gridcraft_core::Value::Number(_))); + assert!(b1.font.bold && b1.align.h == gridcraft_model::HAlign::Center); + assert_ne!(b1.num_fmt.as_str(), "General", "the date keeps its date format"); + assert_eq!(sh.value(CellRef::parse("B2").unwrap()), gridcraft_core::Value::number(4.5)); + // Opened as text, a cell keeps the text it shows, not the number the page recorded for it. + let (wb, _) = open_bytes_with("menu.xls", &page, None, true).unwrap(); + assert_eq!(wb.sheet(0).unwrap().value(CellRef::parse("B2").unwrap()), gridcraft_core::Value::text("4,50")); +} diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 5b94531c4..a6ede6af5 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -47,7 +47,7 @@ use gridcraft_core::DateSystem; use gridcraft_model::Workbook; pub use crypto::{DecryptError, check_encryption, decrypt}; -pub use csv::{MAX_CSV_BYTES, read_csv, read_csv_with_report, write_csv, write_csv_limited}; +pub use csv::{MAX_CSV_BYTES, decode_text, read_csv, read_csv_with_report, write_csv, write_csv_limited}; pub use ods::read_ods; pub use package::MAX_PART; pub use passthrough::rebase_links;