diff --git a/ROADMAP.md b/ROADMAP.md index 7f2b698..3664b6f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365) > **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (editable ink survives XLSX saves) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX @@ -132,6 +133,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | | 2026-10-12 | Security: cell hyperlinks open only http, https and mailto, after a confirmation; other schemes (file:, smb:, UNC, custom handlers) are refused (#777, part 1) | @@ -192,6 +194,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | Rows auto-fit oversized fonts on open and whole-row/column formatting (#833); progress log row | | 2026-10-12 | minor | Progress log: multi-area row/column delete (#758) | | 2026-10-12 | trivial | Progress log: XLSX chart sheets kept on save (#727) | | 2026-10-12 | trivial | Progress log: CSV export size cap (#528) | diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 3ed8ac1..4712351 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -237,10 +237,12 @@ pub(crate) fn apply_style(s: &mut Session, p: &Json, f: impl Fn(&mut Style)) -> // Existing cells in those lines. let cells: Vec<(CellRef, StyleId)> = cx.wb.sheet(sheet).map(|sh| sh.cells.iter_range(*r).map(|(c, x)| (c, x.style)).collect()).unwrap_or_default(); - for (c, st) in cells { - let new = *cache.entry(st).or_insert_with(|| cx.wb.styles.derive(st, &f)); - cx.sheet_mut(sheet)?.set_style(c, new); + for (c, st) in &cells { + let new = *cache.entry(*st).or_insert_with(|| cx.wb.styles.derive(*st, &f)); + cx.sheet_mut(sheet)?.set_style(*c, new); } + // Formatting touched these cells: re-fit their rows (Excel auto-fits). + cx.fit_rows.extend(cells.iter().map(|(c, _)| (sheet, c.row))); continue; } if r.count() > 4_000_000 { diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 52aafae..3e84036 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -56,7 +56,9 @@ pub fn open_bytes(name: &str, bytes: &[u8]) -> Result<(Workbook, Vec)> { /// quote the password. pub fn open_bytes_with_password(name: &str, bytes: &[u8], password: Option<&str>) -> Result<(Workbook, Vec)> { if gridcraft_xlsx::sniff(bytes) != gridcraft_xlsx::Format::Encrypted { - return open_plain(name, bytes); + let (mut wb, warnings) = open_plain(name, bytes)?; + fit_imported_rows(&mut wb); + return Ok((wb, warnings)); } // A scheme no password opens is said straight away, rather than after asking for one. let decrypt_error = |e: gridcraft_xlsx::DecryptError| EngineError::Other(decrypt_message(name, e)); @@ -68,11 +70,36 @@ pub fn open_bytes_with_password(name: &str, bytes: &[u8], password: Option<&str> if gridcraft_xlsx::sniff(&plain) == gridcraft_xlsx::Format::Encrypted { return Err(EngineError::Other(format!("We can't open '{name}': the encrypted file is damaged (it holds another encrypted file)."))); } - let (wb, mut warnings) = open_plain(name, &plain)?; + let (mut wb, mut warnings) = open_plain(name, &plain)?; + fit_imported_rows(&mut wb); warnings.insert(0, DECRYPTED_NOTICE.into()); Ok((wb, warnings)) } +/// Excel auto-fits rows to their tallest font when opening a file: rows without a user-set +/// height grow to fit their content. Rows the file made taller keep their height — our +/// line-height estimate can differ from Excel's, and shrinking files that already look +/// right would be a visible regression. Custom-height rows are left alone entirely. +fn fit_imported_rows(wb: &mut Workbook) { + for si in 0..wb.sheets.len() { + let Some(sh) = wb.sheet(si) else { continue }; + let mut rows: Vec = sh.cells.iter().map(|(c, _)| c.row).collect(); + rows.sort_unstable(); + rows.dedup(); + for row in rows { + let before = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); + crate::cmd::auto_row_height(wb, si, row); + let Some(before) = before else { continue }; + let after = wb.sheet(si).and_then(|sh| sh.rows.get(&row)).and_then(|i| i.size); + if after.is_none_or(|a| before > a + 0.5) + && let Some(sh) = wb.sheet_mut(si) + { + sh.rows.entry(row).or_default().size = Some(before); + } + } + } +} + /// What to tell the user when an encrypted workbook can't be decrypted. Never quotes the password. fn decrypt_message(name: &str, e: gridcraft_xlsx::DecryptError) -> String { match e { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6f4b67a..ae499a1 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1069,6 +1069,87 @@ fn spill_references_roundtrip_through_xlsx() { assert_eq!((v(&s, "C1"), v(&s, "C2")), (Value::Number(10.0), Value::Number(4.0))); } +/// 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 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")); + 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); + e.custom = custom; + e.hidden = hidden; + } + gridcraft_xlsx::write_xlsx(&wb).unwrap() +} + +#[test] // A — the issue's repro +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, 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"); +} + +#[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)), 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 (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)), 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)), true))})) + .unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + let info = sh.rows.get(&4).unwrap(); + 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, 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] fn tables_and_structured_refs() { let mut s = s(); diff --git a/crates/engine/tests/row_autofit.rs b/crates/engine/tests/row_autofit.rs new file mode 100644 index 0000000..5bea5ab --- /dev/null +++ b/crates/engine/tests/row_autofit.rs @@ -0,0 +1,61 @@ +//! Whole-row and whole-column formatting must re-fit row heights (issue #833). + +use serde_json::json; + +fn sheet_mut(s: &mut gridcraft_engine::Session) -> &mut gridcraft_engine::model::Sheet { + std::sync::Arc::make_mut(&mut s.active_mut().unwrap().wb).sheet_mut(0).unwrap() +} + +#[test] +fn whole_row_font_format_grows_row_height_and_undoes() { + 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); + s.execute("home.fontSize", json!({"range": "5:5", "size": 20})).unwrap(); + let after = s.doc().unwrap().wb.active().unwrap().row_height(4); + assert!(after > before + 10.0, "20pt whole-row format should grow the row: before={before}, after={after}"); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(s.doc().unwrap().wb.active().unwrap().row_height(4), before, "undo must restore the height"); +} + +#[test] +fn whole_column_font_format_grows_only_rows_with_cells() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["a"]]})).unwrap(); + s.execute("range.setValues", json!({"range": "A50", "values": [["b"]]})).unwrap(); + s.execute("home.fontSize", json!({"range": "A:A", "size": 20})).unwrap(); + let sh = s.doc().unwrap().wb.active().unwrap(); + assert!(sh.row_height(0) > 30.0 && sh.row_height(49) > 30.0, "rows with cells grow"); + assert!(sh.rows.len() <= 3, "full-column format must fit only rows that have cells, rows map has {} entries", sh.rows.len()); +} + +#[test] +fn whole_row_format_does_not_resize_custom_height_rows() { + let mut s = gridcraft_engine::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A5", "values": [["plain big bold"]]})).unwrap(); + { + let sh = sheet_mut(&mut s); + let e = sh.rows.entry(4).or_default(); + e.custom = true; + e.size = Some(25.0); + } + 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"); +} diff --git a/docs/gaps.md b/docs/gaps.md index 540d17a..c1be6c7 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,5 +1,6 @@ # Gaps +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365) > **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (ink XLSX preservation; icons still pending) · **Target:** Microsoft Excel (Microsoft 365) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (§8: PivotTable values on rows) · **Target:** Microsoft Excel (Microsoft 365) @@ -169,6 +170,7 @@ No alpha blockers: every core workflow passes the [alpha gate](roadmap.md#alpha- | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | rows auto-fit oversized fonts on open and whole-row/column formatting (#833) | | 2026-10-12 | minor | Page Layout view: dragging margins on the rulers (#792) removed from the §14 remainder | | 2026-10-12 | minor | §13: Forecast Sheet Forecast Start (hindcast) landed (#785) | | 2026-10-12 | minor | §17: ink geometry survives XLSX saves; the icon portion of #551 remains | diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 5b4fa6d..1d6aef3 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,5 +1,6 @@ # Target-app parity: GridCraft vs Microsoft Excel +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and row formatting) · **Target:** Microsoft Excel (Microsoft 365) > **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (sheet-tab navigation reveals the selected tab without interrupting manual scrolling) · **Target:** Microsoft Excel (Microsoft 365) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (PivotTables: values on rows) · **Target:** Microsoft Excel (Microsoft 365) @@ -163,7 +164,7 @@ edge cases are left out, along with everything excluded from the mainstream numb | Typing data, simple formulas, AutoSum (SUM, AVERAGE, arithmetic) | 20 | 88 | Solid; 15-digit equality (#57) is rare at this level | | Basic formatting: fonts, bold, fills, borders, currency/percent/date formats | 15 | 85 | | | Copy, paste, fill handle, undo/redo | 15 | 78 | Selection bug (#28), monthly fill (#132) | -| Rows and columns: insert, delete, widths, AutoFit | 10 | 82 | AutoFit fixed (#42, #88) | +| Rows and columns: insert, delete, widths, AutoFit | 10 | 82 | AutoFit fixed (#42, #88); rows auto-fit oversized fonts on open and row formatting (#833) | | Sort and filter | 10 | 82 | | | Save, Save As, reopen XLSX | 10 | 85 | | | A basic chart | 8 | 55 | Created easily; can't be resized (#190) | @@ -333,6 +334,7 @@ The inventory carried over from the 2026-10-07 ROADMAP.md, updated for what land | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | Essentials AutoFit evidence: rows auto-fit oversized fonts on open and row formatting (#833) | | 2026-10-12 | minor | Sheet tabs: reveal the selected tab on navigation and resize while preserving manual scrolling (#681); no score change | | 2026-10-12 | minor | PivotTables: values on rows (Σ Values in Rows at any position, all three layouts; `pivot.moveField` and the field pane move it; `dataOnRows` round-trips) (no score change) | | 2026-10-11 | minor | Stability: command-sequence fuzz harness (`crates/engine/tests/command_sequence_fuzz.rs`) checks random command sequences against undo/redo, XLSX round-trip, full-recalc and structure invariants; the sequence bugs it found are fixed (no score change) |