Merge pull request #1568 from Hamza0101/fix/table-insert-size

A table size Create Table cannot mean is refused

crates/engine/src/cmd/table/sizing.rs

@@ -320,3 +320,49 @@ mod resize_border_tests {
         assert!(s.execute("table.resizeBorder", &json!({"frame": f, "table": t, "axis": "column", "index": 0, "delta": 5})).is_err());
     }
 }
+
+#[cfg(test)]
+mod insert_size_tests {
+    use serde_json::json;
+
+    use crate::Session;
+
+    /// A table size that is given but is not one a table can have used to be corrected in
+    /// silence: `{"rows": 0}` made a one-row table, `-1` and `1.5` made the default four, and
+    /// 10000 was capped at 500 — so a caller's mistake became a table nobody asked for.
+    #[test]
+    fn a_table_size_insert_cannot_mean_is_refused() {
+        let mut s = Session::new();
+        s.execute("file.new", &json!({"pages": 1})).unwrap();
+        let frame = s.execute("frame.create", &json!({"rect": [20, 20, 400, 300], "content": "text", "text": ""})).unwrap();
+        let at = |s: &mut Session| {
+            s.execute("text.select", &json!({"story": frame["story"].clone(), "anchor": 0, "focus": 0})).unwrap();
+        };
+        for (params, names) in [
+            (json!({"rows": 0}), "rows 0: 1 to 500"),
+            (json!({"rows": -1}), "rows -1: 1 to 500"),
+            (json!({"rows": 1.5}), "rows 1.5: 1 to 500"),
+            (json!({"rows": 10000}), "rows 10000: 1 to 500"),
+            (json!({"cols": 0}), "cols 0: 1 to 200"),
+            (json!({"columns": 201}), "cols 201: 1 to 200"),
+            (json!({"headerRows": 101}), "headerRows 101: 0 to 100"),
+            (json!({"footerRows": -2}), "footerRows -2: 0 to 100"),
+        ] {
+            at(&mut s);
+            let e = s.execute("table.insert", &params).expect_err("refused").to_string();
+            assert!(e.contains(names), "{params}: {e}");
+        }
+        // Nothing was inserted by any of those.
+        at(&mut s);
+        assert!(s.execute("table.get", &json!({})).is_err(), "no table yet");
+        // The sizes a table can have still work, and absent means the default four by four.
+        at(&mut s);
+        s.execute("table.insert", &json!({})).unwrap();
+        let t = s.execute("table.get", &json!({})).unwrap();
+        assert_eq!(t["cells"].as_array().unwrap().len(), 16, "four by four");
+        at(&mut s);
+        s.execute("table.insert", &json!({"rows": 1, "cols": 2, "headerRows": 1})).unwrap();
+        let t = s.execute("table.get", &json!({})).unwrap();
+        assert_eq!(t["cells"].as_array().unwrap().len(), 4, "one body row and one header row, two columns");
+    }
+}

crates/engine/src/cmd/table/structure.rs

@@ -6,10 +6,24 @@ pub(super) fn insert(s: &mut Session, p: &Value) -> Result<Value> {
     if t.cell.is_some() {
         return Err(bad("table.insert", "tables can't be nested in cells"));
     }
-    let rows = p.get("rows").and_then(Value::as_u64).unwrap_or(4).clamp(1, 500) as usize;
-    let cols = p.get("cols").or_else(|| p.get("columns")).and_then(Value::as_u64).unwrap_or(4).clamp(1, 200) as usize;
-    let header = p.get("headerRows").and_then(Value::as_u64).unwrap_or(0).min(100) as usize;
-    let footer = p.get("footerRows").and_then(Value::as_u64).unwrap_or(0).min(100) as usize;
+    // A size that is given but is not one a table can have used to be corrected in silence:
+    // `{"rows": 0}` made a table with one row, `-1` and `1.5` made the default four, and 10000
+    // was capped at 500. Named `given` rather than `count`, which this module already imports
+    // for the repeat count of a row or column insert.
+    let given = |key: &str, alt: Option<&str>, default: u64, range: std::ops::RangeInclusive<u64>| -> Result<usize> {
+        let v = p.get(key).or_else(|| alt.and_then(|a| p.get(a))).filter(|v| !v.is_null());
+        match v {
+            None => Ok(default as usize),
+            Some(v) => match v.as_u64().filter(|n| range.contains(n)) {
+                Some(n) => Ok(n as usize),
+                None => Err(bad("table.insert", format!("{key} {v}: {} to {}", range.start(), range.end()))),
+            },
+        }
+    };
+    let rows = given("rows", None, 4, 1..=500)?;
+    let cols = given("cols", Some("columns"), 4, 1..=200)?;
+    let header = given("headerRows", None, 0, 0..=100)?;
+    let footer = given("footerRows", None, 0, 0..=100)?;
     let width = p.get("width").and_then(Value::as_f64).unwrap_or_else(|| column_width(s, &t));
     s.edit(|d, sel| {
         let id = d.alloc();