Merge PR #1085: Format: Row Height and Column Width say pixels, and take points like the reference app

Conflicts:
- crates/engine/src/cmd/format.rs: spec strings keep #885's sheet?/Sheet! forms with #1085's pixels/points wording; row_height and col_width keep #1085's unit logic with #885's (si, lines) = line_targets

crates/engine/src/cmd/format.rs

@@ -156,7 +156,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Row Height…",
             ["Home", "Cells", "Format"],
             None,
-            "{sheet?, rows?: \"2:5\" | \"2:2,4:5\" | \"Sheet2!2:5\", height: 20 (points on screen)}",
+            "{sheet?, rows?: \"2:5\" | \"2:2,4:5\" | \"Sheet2!2:5\", height: 20 (pixels) | points: 15}",
             has_doc,
             row_height
         ),
@@ -165,13 +165,13 @@ pub fn specs() -> Vec<CommandSpec> {
             "Column Width…",
             ["Home", "Cells", "Format"],
             None,
-            "{sheet?, cols?: \"B:D\" | \"B:B,D:F\" | \"Sheet2!B:D\", width: 64 (points) | chars: 8.43}",
+            "{sheet?, cols?: \"B:D\" | \"B:B,D:F\" | \"Sheet2!B:D\", width: 64 (pixels) | chars: 8.43}",
             has_doc,
             col_width
         ),
         cmd!("home.autofitRowHeight", "AutoFit Row Height", ["Home", "Cells", "Format"], None, "{sheet?, rows?}", has_doc, autofit_rows),
         cmd!("home.autofitColumnWidth", "AutoFit Column Width", ["Home", "Cells", "Format"], None, "{sheet?, cols?}", has_doc, autofit_cols),
-        cmd!("home.defaultWidth", "Default Width…", ["Home", "Cells", "Format"], None, "{sheet?, width: 64}", has_doc, default_width),
+        cmd!("home.defaultWidth", "Default Width…", ["Home", "Cells", "Format"], None, "{sheet?, width: 64 (pixels)}", has_doc, default_width),
         cmd!("home.hideRows", "Hide Rows", ["Home", "Cells", "Format", "Hide & Unhide"], Some("Cmd+9"), "{sheet?, rows?}", has_doc, |s, p| hide(
             s, p, true, true
         )),
@@ -972,11 +972,25 @@ fn line_targets(s: &Session, p: &Json, rows: bool) -> Result<(usize, Vec<(u32, u
     Ok((sheet, d.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<Json> {
-    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 (si, lines) = line_targets(s, p, true)?;
     edit(s, |cx| {
         let sh = cx.sheet_mut(si)?;
@@ -995,13 +1009,20 @@ fn row_height(s: &mut Session, p: &Json) -> Result<Json> {
 
 fn col_width(s: &mut Session, p: &Json) -> Result<Json> {
     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 (si, lines) = line_targets(s, p, false)?;
     edit(s, |cx| {
         let sh = cx.sheet_mut(si)?;
@@ -1016,7 +1037,7 @@ fn col_width(s: &mut Session, p: &Json) -> Result<Json> {
     })
 }
 
-/// 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

crates/engine/src/tests.rs

@@ -3141,6 +3141,48 @@ fn line_sizes_and_freeze_panes_take_a_sheet() {
     assert_eq!(sheet(&s, 0), before);
 }
 
+#[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#"<row r="1" ht="30" customHeight="1""#, r#"<row r="2" ht="22.5" customHeight="1""#, r#"<row r="3" ht="409.5" customHeight="1""#] {
+        assert!(xml.contains(row), "{row} missing from {xml}");
+    }
+
+    // Out of range, the message gives the limit in the unit that was passed.
+    for (id, params, message) in [
+        ("home.rowHeight", json!({"height": 547}), "Row height must be between 0 and 546 pixels (409.5 points)."),
+        ("home.rowHeight", json!({"points": 410}), "Row height must be between 0 and 409.5 points."),
+        ("home.columnWidth", json!({"width": 1801}), "Column width must be between 0 and 1800 pixels."),
+        ("home.columnWidth", json!({"chars": 256}), "Column width must be between 0 and 255 characters."),
+    ] {
+        assert_eq!(s.execute(id, params).unwrap_err().to_string(), message);
+    }
+
+    // The parameter docs agents read (`list_commands`) name the unit.
+    for (id, unit) in [
+        ("home.rowHeight", "height: 20 (pixels) | points: 15"),
+        ("home.columnWidth", "width: 64 (pixels) | chars: 8.43"),
+        ("home.defaultWidth", "width: 64 (pixels)"),
+    ] {
+        let params = s.commands().into_iter().find(|c| c.id == id).map(|c| c.params).unwrap_or_default();
+        assert!(params.contains(unit), "{id}: {params}");
+    }
+}
+
 #[test]
 fn copying_areas_that_share_rows_or_columns_pastes_them_as_one_block() {
     // Issue #348: Excel slides lined-up areas together; formulas travel as their values.

crates/model/src/features.rs

@@ -867,7 +867,7 @@ pub fn password_hash(pw: &str) -> 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<f32>,
     pub hidden: bool,
     pub outline: u8,

crates/model/src/sheet.rs

@@ -64,7 +64,7 @@ pub struct Sheet {
     pub spill_ranges: BTreeMap<CellRef, RangeRef>,
     pub cols: BTreeMap<u32, LineInfo>,
     pub rows: BTreeMap<u32, LineInfo>,
-    /// 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<RangeRef>,
@@ -113,7 +113,7 @@ pub struct Sheet {
     pub preserved: Option<Arc<crate::preserved::Preserved>>,
 }
 
-/// 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;