From 9a816f80f7c635e05f8b5054bacae01385e3b09d Mon Sep 17 00:00:00 2001 From: CouchRiv <270190108+CouchRiv@users.noreply.github.com> Date: Mon, 12 Oct 2026 22:54:25 -0400 Subject: [PATCH] Format: Row Height and Column Width say pixels, and take points like Excel home.rowHeight and home.columnWidth take pixels at 96 dpi, the model's unit (header drags and the Row Height dialog send pixels), but their parameter docs said points. An agent asking for a 30 pt row got `ht="22.5"` in the file, and the range check accepted 500 while its message said "between 0 and 409 points". - The params docs say pixels: `height: 20 (pixels) | points: 15`, `width: 64 (pixels) | chars: 8.43`, and `{width: 64 (pixels)}` for home.defaultWidth. Pixel values behave as before. - home.rowHeight takes `points`, Excel's unit (Row Height dialog, `ht` in the file), as home.columnWidth already takes `chars`. - Range errors give the limit in the unit passed: 546 pixels (409.5 points), 409.5 points, 1800 pixels, 255 characters. `chars` is checked against 255, Excel's limit and the widest column a file holds (it went to 256.4 through the pixel limit). - The model's size comments say pixels. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/format.rs | 49 +++++++++++++++++++++++---------- crates/engine/src/tests.rs | 42 ++++++++++++++++++++++++++++ crates/model/src/features.rs | 2 +- crates/model/src/sheet.rs | 4 +-- 4 files changed, 80 insertions(+), 17 deletions(-) diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index dde5fa120..5b550d480 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -156,7 +156,7 @@ pub fn specs() -> Vec { "Row Height…", ["Home", "Cells", "Format"], None, - "{rows?: \"2:5\" | \"2:2,4:5\", height: 20 (points on screen)}", + "{rows?: \"2:5\" | \"2:2,4:5\", height: 20 (pixels) | points: 15}", has_doc, row_height ), @@ -165,13 +165,13 @@ pub fn specs() -> Vec { "Column Width…", ["Home", "Cells", "Format"], None, - "{cols?: \"B:D\" | \"B:B,D:F\", width: 64 (points) | chars: 8.43}", + "{cols?: \"B:D\" | \"B:B,D:F\", width: 64 (pixels) | chars: 8.43}", has_doc, col_width ), cmd!("home.autofitRowHeight", "AutoFit Row Height", ["Home", "Cells", "Format"], None, "{rows?}", has_doc, autofit_rows), cmd!("home.autofitColumnWidth", "AutoFit Column Width", ["Home", "Cells", "Format"], None, "{cols?}", has_doc, autofit_cols), - cmd!("home.defaultWidth", "Default Width…", ["Home", "Cells", "Format"], None, "{width: 64}", has_doc, default_width), + cmd!("home.defaultWidth", "Default Width…", ["Home", "Cells", "Format"], None, "{width: 64 (pixels)}", has_doc, default_width), cmd!("home.hideRows", "Hide Rows", ["Home", "Cells", "Format", "Hide & Unhide"], Some("Cmd+9"), "{rows?}", has_doc, |s, p| hide( s, p, true, true )), @@ -936,11 +936,25 @@ fn line_targets(s: &Session, p: &Json, rows: bool) -> Result> { Ok(s.doc()?.selection.ranges.iter().map(|r| if rows { (r.start.row, r.end.row) } else { (r.start.col, r.end.col) }).collect()) } +/// Sizes are pixels at 96 dpi, the model's unit (header drags send pixels). `points` (row height) +/// and `chars` (column width) take Excel's units, those of its Row Height and Column Width dialogs +/// and of the file (`ht` is in points). fn row_height(s: &mut Session, p: &Json) -> Result { - let h = f64_param(p, "height").ok_or_else(|| bad("home.rowHeight", "missing `height`"))?; - if !(0.0..=546.0).contains(&h) { - return Err(EngineError::Other("Row height must be between 0 and 409 points.".into())); - } + let h = match (f64_param(p, "height"), f64_param(p, "points")) { + (Some(h), _) => { + if !(0.0..=546.0).contains(&h) { + return Err(EngineError::Other("Row height must be between 0 and 546 pixels (409.5 points).".into())); + } + h + } + (None, Some(pt)) => { + if !(0.0..=409.5).contains(&pt) { + return Err(EngineError::Other("Row height must be between 0 and 409.5 points.".into())); + } + pt * 4.0 / 3.0 + } + _ => return Err(bad("home.rowHeight", "missing `height` (pixels) or `points`")), + }; let lines = line_targets(s, p, true)?; edit(s, |cx| { let si = cx.sheet_index(); @@ -959,13 +973,20 @@ fn row_height(s: &mut Session, p: &Json) -> Result { fn col_width(s: &mut Session, p: &Json) -> Result { let w = match (f64_param(p, "width"), f64_param(p, "chars")) { - (Some(w), _) => w, - (None, Some(c)) => chars_to_points(c), - _ => return Err(bad("home.columnWidth", "missing `width`")), + (Some(w), _) => { + if !(0.0..=1800.0).contains(&w) { + return Err(EngineError::Other("Column width must be between 0 and 1800 pixels.".into())); + } + w + } + (None, Some(c)) => { + if !(0.0..=255.0).contains(&c) { + return Err(EngineError::Other("Column width must be between 0 and 255 characters.".into())); + } + chars_to_points(c) + } + _ => return Err(bad("home.columnWidth", "missing `width` (pixels) or `chars`")), }; - if !(0.0..=1800.0).contains(&w) { - return Err(EngineError::Other("Column width must be between 0 and 255 characters.".into())); - } let lines = line_targets(s, p, false)?; edit(s, |cx| { let si = cx.sheet_index(); @@ -981,7 +1002,7 @@ fn col_width(s: &mut Session, p: &Json) -> Result { }) } -/// Excel column width in characters (of the default font's digit width, 7 px) → points. +/// Excel column width in characters (of the default font's digit width, 7 px) → pixels. pub fn chars_to_points(c: f64) -> f64 { if c <= 0.0 { 0.0 diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 33fdc113e..2ffb096fb 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2636,6 +2636,48 @@ fn row_height_and_column_width_take_a_list_of_areas() { assert_eq!(s.doc().unwrap().wb.active().unwrap().col_width(1), 120.0, "a bad list changes nothing"); } +#[test] +fn row_height_and_column_width_take_pixels_or_excel_units() { + // `height` and `width` are pixels at 96 dpi (the model's unit); `points` and `chars` are + // Excel's units, those of its Row Height and Column Width dialogs. A row is saved in points. + let mut s = s(); + s.execute("home.rowHeight", json!({"rows": "1:1", "points": 30})).unwrap(); + s.execute("home.rowHeight", json!({"rows": "2:2", "height": 30})).unwrap(); + s.execute("home.rowHeight", json!({"rows": "3:3", "points": 409.5})).unwrap(); + s.execute("home.columnWidth", json!({"cols": "A:A", "chars": 8.43})).unwrap(); + s.execute("home.columnWidth", json!({"cols": "B:B", "width": 100})).unwrap(); + let sheet = s.doc().unwrap().wb.active().unwrap(); + assert_eq!([sheet.row_height(0), sheet.row_height(1), sheet.row_height(2)], [40.0, 30.0, 546.0]); + assert_eq!([sheet.col_width(0), sheet.col_width(1)], [64.0, 100.0]); + let bytes = gridcraft_xlsx::write_xlsx(&s.doc().unwrap().wb).unwrap(); + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap(); + let mut xml = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("xl/worksheets/sheet1.xml").unwrap(), &mut xml).unwrap(); + for row in [r#" u64 { /// Outline grouping info for a row or column. #[derive(Clone, Copy, Debug, Default, PartialEq, Serialize, Deserialize)] pub struct LineInfo { - /// Size in points; `None` = default (rows auto-fit to content). + /// Size in pixels at 96 dpi; `None` = default (rows auto-fit to content). pub size: Option, pub hidden: bool, pub outline: u8, diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index e05d3bfb5..5b5d1e294 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -64,7 +64,7 @@ pub struct Sheet { pub spill_ranges: BTreeMap, pub cols: BTreeMap, pub rows: BTreeMap, - /// Default column width and row height in points. + /// Default column width and row height in pixels at 96 dpi (a file stores row heights in points). pub default_col_width: f32, pub default_row_height: f32, pub merges: Vec, @@ -113,7 +113,7 @@ pub struct Sheet { pub preserved: Option>, } -/// Default width of a column (points) and height of a row (points) for the default font. +/// Default width of a column and height of a row (pixels at 96 dpi) for the default font. pub const DEFAULT_COL_WIDTH: f32 = 64.0; pub const DEFAULT_ROW_HEIGHT: f32 = 20.0;