Merge pull request #1189

# Conflicts:
#	crates/engine/src/tests.rs
This commit is contained in:
Brandon Thomas
2026-10-25 13:42:45 -04:00
2 changed files with 103 additions and 23 deletions
+54 -23
View File
@@ -13,22 +13,40 @@ use crate::{DocState, Result, Session};
pub fn specs() -> Vec<CommandSpec> {
vec![
cmd!("table.insertRowAbove", "Insert Above", ["Table Layout", "Rows & Columns"], None, "{row?, id?}", has_selection, |s, p| rows_cols(
s, p, "rowAbove"
s,
p,
"rowAbove",
"table.insertRowAbove"
)),
cmd!("table.insertRowBelow", "Insert Below", ["Table Layout", "Rows & Columns"], None, "{row?, id?}", has_selection, |s, p| rows_cols(
s, p, "rowBelow"
s,
p,
"rowBelow",
"table.insertRowBelow"
)),
cmd!("table.insertColumnLeft", "Insert Left", ["Table Layout", "Rows & Columns"], None, "{col?, id?}", has_selection, |s, p| rows_cols(
s, p, "colLeft"
s,
p,
"colLeft",
"table.insertColumnLeft"
)),
cmd!("table.insertColumnRight", "Insert Right", ["Table Layout", "Rows & Columns"], None, "{col?, id?}", has_selection, |s, p| rows_cols(
s, p, "colRight"
s,
p,
"colRight",
"table.insertColumnRight"
)),
cmd!("table.deleteRow", "Delete Rows", ["Table Layout", "Rows & Columns"], None, "{row?, id?}", has_selection, |s, p| rows_cols(
s, p, "delRow"
s,
p,
"delRow",
"table.deleteRow"
)),
cmd!("table.deleteColumn", "Delete Columns", ["Table Layout", "Rows & Columns"], None, "{col?, id?}", has_selection, |s, p| rows_cols(
s, p, "delCol"
s,
p,
"delCol",
"table.deleteColumn"
)),
cmd!("table.merge", "Merge Cells", ["Table Layout", "Merge"], None, "{from: [r, c], to: [r, c], id?}", has_selection, merge),
cmd!(
@@ -228,16 +246,26 @@ impl TableFrame {
}
}
fn cell_param(p: &Value, key: &str) -> Option<(usize, usize)> {
let a = p.get(key)?.as_array()?;
Some((a.first()?.as_u64()? as usize, a.get(1)?.as_u64()? as usize))
/// A `[r, c]` cell param: `Ok(None)` when absent (callers may fall back to the selection), `Err` when
/// present but not a valid cell, so a bad cell never acts on the selection.
fn cell_param(p: &Value, key: &str, cmd: &str) -> Result<Option<(usize, usize)>> {
let v = match p.get(key) {
None | Some(Value::Null) => return Ok(None),
Some(v) => v,
};
let idx = |i: usize| v.as_array().and_then(|a| a.get(i)).and_then(Value::as_u64).and_then(|x| usize::try_from(x).ok());
match (idx(0), idx(1)) {
(Some(r), Some(c)) => Ok(Some((r, c))),
_ => Err(bad(cmd, format!("`{key}` is not a valid cell: {v}"))),
}
}
/// The table's row/col to act on: params, the edited cell, or the selected cells.
fn at(s: &Session, p: &Value) -> (usize, usize) {
/// A present but invalid `row`/`col` is an error; only an absent one falls back.
fn at(s: &Session, p: &Value, cmd: &str) -> Result<(usize, usize)> {
let st = s.active();
let cell = st.and_then(|d| d.selection.text.as_ref().and_then(|t| t.cell)).or_else(|| st.and_then(|d| d.selection.cells.map(|c| c.1)));
(usize_param(p, "row").or(cell.map(|c| c.0)).unwrap_or(0), usize_param(p, "col").or(cell.map(|c| c.1)).unwrap_or(0))
Ok((opt_usize_param(p, "row", cmd)?.or(cell.map(|c| c.0)).unwrap_or(0), opt_usize_param(p, "col", cmd)?.or(cell.map(|c| c.1)).unwrap_or(0)))
}
fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> Result<()>) -> Result<Value> {
@@ -285,10 +313,10 @@ fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> R
Ok(r)
}
fn rows_cols(s: &mut Session, p: &Value, op: &str) -> Result<Value> {
let (r, c) = at(s, p);
fn rows_cols(s: &mut Session, p: &Value, op: &str, name: &'static str) -> Result<Value> {
let (r, c) = at(s, p, name)?;
let op = op.to_string();
with_table(s, p, "table", move |t| {
with_table(s, p, name, move |t| {
match op.as_str() {
"rowAbove" => t.insert_row(r),
"rowBelow" => t.insert_row(r.saturating_add(1)),
@@ -296,12 +324,12 @@ fn rows_cols(s: &mut Session, p: &Value, op: &str) -> Result<Value> {
"colRight" => t.insert_col(c.saturating_add(1)),
"delRow" => {
if !t.delete_row(r) {
return Err(bad("table.deleteRow", "can't delete the last row"));
return Err(bad(name, "can't delete the last row"));
}
}
_ => {
if !t.delete_col(c) {
return Err(bad("table.deleteColumn", "can't delete the last column"));
return Err(bad(name, "can't delete the last column"));
}
}
}
@@ -311,8 +339,8 @@ fn rows_cols(s: &mut Session, p: &Value, op: &str) -> Result<Value> {
fn merge(s: &mut Session, p: &Value) -> Result<Value> {
let sel = s.doc()?.selection.cells;
let from = cell_param(p, "from").or(sel.map(|c| c.1)).ok_or_else(|| bad("table.merge", "missing `from`"))?;
let to = cell_param(p, "to").or(sel.map(|c| c.2)).ok_or_else(|| bad("table.merge", "missing `to`"))?;
let from = cell_param(p, "from", "table.merge")?.or(sel.map(|c| c.1)).ok_or_else(|| bad("table.merge", "missing `from`"))?;
let to = cell_param(p, "to", "table.merge")?.or(sel.map(|c| c.2)).ok_or_else(|| bad("table.merge", "missing `to`"))?;
with_table(
s,
p,
@@ -322,7 +350,10 @@ fn merge(s: &mut Session, p: &Value) -> Result<Value> {
}
fn split(s: &mut Session, p: &Value) -> Result<Value> {
let cell = cell_param(p, "cell").unwrap_or_else(|| at(s, p));
let cell = match cell_param(p, "cell", "table.split")? {
Some(c) => c,
None => at(s, p, "table.split")?,
};
with_table(s, p, "table.split", move |t| if t.split(cell.0, cell.1) { Ok(()) } else { Err(bad("table.split", "that cell isn't merged")) })
}
@@ -536,7 +567,7 @@ fn distribute(s: &mut Session, p: &Value, rows: bool) -> Result<Value> {
}
fn column_width(s: &mut Session, p: &Value) -> Result<Value> {
let (_, c) = at(s, p);
let (_, c) = at(s, p, "table.columnWidth")?;
let w = f64_param(p, "width").ok_or_else(|| bad("table.columnWidth", "missing `width`"))?.clamp(4.0, 4000.0);
let keep_total = bool_or(p, "keepTotal", false);
with_table(s, p, "table.columnWidth", move |t| {
@@ -565,7 +596,7 @@ fn column_width(s: &mut Session, p: &Value) -> Result<Value> {
}
fn row_height(s: &mut Session, p: &Value) -> Result<Value> {
let (r, _) = at(s, p);
let (r, _) = at(s, p, "table.rowHeight")?;
let h = f64_param(p, "height").ok_or_else(|| bad("table.rowHeight", "missing `height`"))?.clamp(4.0, 4000.0);
with_table(s, p, "table.rowHeight", move |t| {
if let Some(x) = t.rows.get_mut(r) {
@@ -580,8 +611,8 @@ fn select_cells(s: &mut Session, p: &Value) -> Result<Value> {
Some(i) => i,
None => s.doc()?.selection.shapes.first().copied().ok_or_else(|| bad("table.selectCells", "select a table"))?,
};
let from = cell_param(p, "from").unwrap_or((0, 0));
let to = cell_param(p, "to").unwrap_or(from);
let from = cell_param(p, "from", "table.selectCells")?.unwrap_or((0, 0));
let to = cell_param(p, "to", "table.selectCells")?.unwrap_or(from);
s.select(|_, sel| {
sel.cells = Some((id, from, to));
sel.shapes = vec![id];
+49
View File
@@ -4941,6 +4941,55 @@ fn paragraph_commands_clamp_a_direct_hostile_text_selection() {
assert_eq!(body.paragraphs[0].props.align, Some(deckcraft_model::text::Align::Center));
}
#[test]
fn table_row_col_commands_reject_an_invalid_index() {
let mut s = session();
let id = table_at(&mut s, &[100.0, 100.0], json!([["a", "b"], ["c", "d"], ["e", "f"]]));
for (cmd, key) in [
("table.deleteRow", "row"),
("table.deleteColumn", "col"),
("table.insertRowAbove", "row"),
("table.insertColumnLeft", "col"),
("table.split", "row"),
] {
for bad in [json!(-1), json!(1.5), json!("1")] {
let r = s.execute(cmd, &json!({"id": id.0, key: bad}));
assert!(r.is_err(), "{cmd} {key}={bad} should be rejected, got {r:?}");
let t = table_of(&s, id);
assert_eq!((t.n_rows(), t.n_cols()), (3, 2), "{cmd} {key}={bad} must leave the table unchanged");
}
}
}
#[test]
fn table_cell_commands_reject_an_invalid_cell_and_name_the_registered_command() {
let mut s = session();
let id = table_at(&mut s, &[100.0, 100.0], json!([["a", "b"], ["c", "d"], ["e", "f"]]));
s.execute("table.selectCells", &json!({"id": id.0, "from": [0, 0], "to": [1, 1]})).unwrap();
for (cmd, params) in [
("table.split", json!({"id": id.0, "cell": [-1, 0]})),
("table.merge", json!({"id": id.0, "from": [-1, 0], "to": [1, 1]})),
("table.merge", json!({"id": id.0, "from": [0, 0], "to": "x"})),
("table.rowHeight", json!({"id": id.0, "row": -1, "height": 50.0})),
("table.rowHeight", json!({"id": id.0, "row": 1.5, "height": 50.0})),
("table.columnWidth", json!({"id": id.0, "col": -1, "width": 50.0})),
("table.columnWidth", json!({"id": id.0, "col": "1", "width": 50.0, "keepTotal": true})),
] {
let before = table_of(&s, id);
let (rows, cols) = (before.rows.iter().map(|r| r.height).collect::<Vec<_>>(), before.cols.clone());
let r = s.execute(cmd, &params);
assert!(r.is_err(), "{cmd} {params} should be rejected, got {r:?}");
let t = table_of(&s, id);
assert_eq!((t.n_rows(), t.n_cols()), (3, 2), "{cmd} {params} must leave the table unchanged");
assert_eq!(t.rows.iter().map(|r| r.height).collect::<Vec<_>>(), rows, "{cmd} {params} must not change row heights");
assert_eq!(t.cols, cols, "{cmd} {params} must not change column widths");
}
let e = s.execute("table.deleteRow", &json!({"id": id.0, "row": -1})).unwrap_err().to_string();
assert!(e.contains("table.deleteRow"), "error should name the registered command: {e}");
let e = s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": -1})).unwrap_err().to_string();
assert!(e.contains("table.insertColumnLeft"), "error should name the registered command: {e}");
}
#[test]
fn table_column_width_of_a_huge_column_is_ignored() {
let mut s = session();