diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 2c3f397..da7bf62 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -241,7 +241,7 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> let new = *cache.entry(*st).or_insert_with(|| cx.wb.styles.derive(*st, &f)); cx.sheet_mut(sheet)?.set_style(*c, new); } - // Formatting changed these cells' fonts: re-fit their rows (Excel auto-fits). + // Formatting touched these cells: re-fit their rows (Excel auto-fits). cx.fit_rows.extend(cells.iter().map(|(c, _)| (sheet, c.row))); continue; } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 49f67e8..7fcc8fe 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1008,17 +1008,20 @@ fn spill_references_roundtrip_through_xlsx() { } /// Fixture for the open-time row fit (issue #833): `row_info` = (size, custom, hidden) for -/// row 5 (index 4), `None` = no row entry at all. Cell A5 = "plain big bold", whole-cell -/// font 20pt. 11pt cell in A1 for contrast rows. Built with the model API, never through -/// commands — commands would auto-fit the rows — and exported with `write_xlsx`, which -/// omits `ht` for rows without a size: exactly the bug's input. -fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec { +/// row 5 (index 4), `None` = no row entry at all. Cell A5 = "plain big bold" — whole-cell +/// font 20pt with `big_font`, plain 11pt without (a tall plain row's fit removes its entry). +/// 11pt cell in A1 for contrast rows. Built with the model API, never through commands — +/// commands would auto-fit the rows — and exported with `write_xlsx`, which omits `ht` for +/// rows without a size: exactly the bug's input. +fn big_font_workbook(row_info: Option<(f32, bool, bool)>, big_font: bool) -> Vec { let mut wb = gridcraft_model::Workbook::new(); let big = wb.styles.derive(gridcraft_model::StyleId::DEFAULT, |s| s.font.size = 20.0); let sh = wb.sheet_mut(0).unwrap(); sh.set_value(CellRef::new(0, 0), Value::text("small")); sh.set_value(CellRef::new(4, 0), Value::text("plain big bold")); - sh.set_style(CellRef::new(4, 0), big); + if big_font { + sh.set_style(CellRef::new(4, 0), big); + } if let Some((size, custom, hidden)) = row_info { let e = sh.rows.entry(4).or_default(); e.size = Some(size); @@ -1032,7 +1035,7 @@ fn big_font_workbook(row_info: Option<(f32, bool, bool)>) -> Vec { fn open_grows_default_height_rows_to_their_font() { let default_h = gridcraft_model::Workbook::new().active().unwrap().row_height(0); let mut s = Session::new(); - s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); + s.execute("file.open", json!({"name": "bigfont.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None, true))})).unwrap(); let h = s.doc().unwrap().wb.active().unwrap().row_height(4); assert!(h >= 30.0, "20pt cell must grow its default-height row on open, got {h}"); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(0), default_h, "11pt row stays default"); @@ -1041,35 +1044,48 @@ fn open_grows_default_height_rows_to_their_font() { #[test] // B — Review Focus 1 fn open_keeps_custom_height_rows() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "custom.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((25.0, true, false))))})) + s.execute("file.open", json!({"name": "custom.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((25.0, true, false)), true))})) .unwrap(); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows are left alone on open"); } -#[test] // C — Review Focus 2 +#[test] // C — Review Focus 2 (Some→Some: the fit shrinks the row, the clamp keeps the taller height) fn open_never_shrinks_taller_auto_rows() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "tall.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false))))})).unwrap(); + s.execute("file.open", json!({"name": "tall.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false)), true))})) + .unwrap(); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 60.0, "open is grow-only: a 60px auto-height row keeps its height"); } +#[test] // C′ — Review Focus 2 (Some→None: a plain tall row's entry is removed by the fit, the clamp restores it) +fn open_restores_height_of_plain_rows_the_fit_removed() { + let mut s = Session::new(); + s.execute( + "file.open", + json!({"name": "tall-plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((60.0, false, false)), false))}), + ) + .unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 60.0, "a tall plain row keeps its height after the fit removes its entry"); +} + #[test] // D — Review Focus 5 fn open_fitting_keeps_hidden_rows_hidden() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "hidden.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((20.0, false, true))))})) + s.execute("file.open", json!({"name": "hidden.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(Some((20.0, false, true)), true))})) .unwrap(); let sh = s.doc().unwrap().wb.active().unwrap(); let info = sh.rows.get(&4).unwrap(); - assert!(info.size.is_none_or(|h| h >= 20.0), "hidden row grows or keeps its height, got {:?}", info.size); + assert!(info.size.is_some_and(|h| h >= 20.0), "hidden row grows or keeps its height, got {:?}", info.size); assert!(info.hidden, "fitting a hidden row must not unhide it"); } #[test] // E — Review Focus 4 fn open_leaves_plain_workbooks_untouched() { let mut s = Session::new(); - s.execute("file.open", json!({"name": "plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None))})).unwrap(); + s.execute("file.open", json!({"name": "plain.xlsx", "base64": crate::io::base64_encode(&big_font_workbook(None, false))})).unwrap(); let sh = s.doc().unwrap().wb.active().unwrap(); assert!(sh.rows.get(&0).is_none_or(|i| i.size.is_none()), "11pt default rows get no height entry"); + assert!(sh.rows.values().all(|i| i.size.is_none()), "no row gains a height entry"); } #[test] diff --git a/crates/engine/tests/row_autofit.rs b/crates/engine/tests/row_autofit.rs index 8b9af4d..5bea5ab 100644 --- a/crates/engine/tests/row_autofit.rs +++ b/crates/engine/tests/row_autofit.rs @@ -45,3 +45,17 @@ fn whole_row_format_does_not_resize_custom_height_rows() { s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), 25.0, "custom-height rows keep their height"); } + +#[test] +fn whole_row_format_keeps_hidden_rows_hidden() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); + let before = s.doc().unwrap().wb.active().unwrap().row_height(4); + sheet_mut(&mut s).rows.entry(4).or_default().hidden = true; + s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + let info = sh.rows.get(&4).unwrap(); + assert!(info.size.unwrap_or(before) > before + 10.0, "hidden row still grows to its font: before={before}, size={:?}", info.size); + assert!(info.hidden, "formatting a hidden row must not unhide it"); +}