From 37d40a8782b39435366fd6ef19e8b97fce8f2fef Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Sun, 18 Oct 2026 17:30:17 -0500 Subject: [PATCH 1/5] table.insertColumn* keeps the table's width and copies the neighbour column's cell formatting Inserting a column grew the table by one column width, so a full-width table ran off the slide, and the new cells came with none of the fill, bold or anchor the neighbouring cells had. The new column now takes the width of the column it is inserted beside and the other columns shrink in proportion, so the table keeps its total width (`keepWidth: false` restores the growth); each new cell takes the neighbour cell's fill, borders, margins, anchor, text direction and text properties (kept on the empty paragraph's end mark). Rows are left alone: a table growing by a row is normal. Found by an AI agent building a deck through the MCP server. Co-Authored-By: Claude Fable 5.1 --- crates/engine/src/cmd/table.rs | 90 ++++++++++++++++++++++++++++------ crates/engine/src/tests.rs | 42 ++++++++++++++++ 2 files changed, 117 insertions(+), 15 deletions(-) diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 91ac7ac70..23f591719 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -1,7 +1,7 @@ //! Table Design and Table Layout tabs. use deckcraft_geom::{Affine, Point, Rect}; -use deckcraft_model::{Shape, ShapeKind, Table}; +use deckcraft_model::{Cell, Shape, ShapeKind, Table}; use serde_json::{Value, json}; use super::{ @@ -24,18 +24,24 @@ pub fn specs() -> Vec { "rowBelow", "table.insertRowBelow" )), - cmd!("table.insertColumnLeft", "Insert Left", ["Table Layout", "Rows & Columns"], None, "{col?, id?}", has_selection, |s, p| rows_cols( - 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", - "table.insertColumnRight" - )), + cmd!( + "table.insertColumnLeft", + "Insert Left", + ["Table Layout", "Rows & Columns"], + None, + "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside", + has_selection, + |s, p| rows_cols(s, p, "colLeft", "table.insertColumnLeft") + ), + cmd!( + "table.insertColumnRight", + "Insert Right", + ["Table Layout", "Rows & Columns"], + None, + "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside", + has_selection, + |s, p| rows_cols(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, @@ -315,13 +321,14 @@ fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> R fn rows_cols(s: &mut Session, p: &Value, op: &str, name: &'static str) -> Result { let (r, c) = at(s, p, name)?; + let keep_width = bool_or(p, "keepWidth", true); let op = op.to_string(); with_table(s, p, name, move |t| { match op.as_str() { "rowAbove" => t.insert_row(r), "rowBelow" => t.insert_row(r.saturating_add(1)), - "colLeft" => t.insert_col(c), - "colRight" => t.insert_col(c.saturating_add(1)), + "colLeft" => insert_col_like(t, c, c, keep_width), + "colRight" => insert_col_like(t, c.saturating_add(1), c, keep_width), "delRow" => { if !t.delete_row(r) { return Err(bad(name, "can't delete the last row")); @@ -337,6 +344,59 @@ fn rows_cols(s: &mut Session, p: &Value, op: &str, name: &'static str) -> Result }) } +/// Insert a column before `at` that looks like column `neighbour` (the one it is inserted beside): +/// its width, and in every row the neighbour cell's explicit formatting with no text. With +/// `keep_width` the columns then shrink in proportion so the table keeps its total width; +/// otherwise the table grows by the new column. +fn insert_col_like(t: &mut Table, at: usize, neighbour: usize, keep_width: bool) { + let total = t.width(); + let neighbour = neighbour.min(t.cols.len().saturating_sub(1)); + let w = t.cols.get(neighbour).copied(); + let templates: Vec> = t.rows.iter().map(|r| r.cells.get(neighbour).cloned()).collect(); + t.insert_col(at); + let at = at.min(t.cols.len().saturating_sub(1)); + if let (Some(w), Some(slot)) = (w, t.cols.get_mut(at)) { + *slot = w; + } + for (row, tpl) in t.rows.iter_mut().zip(templates) { + if let (Some(cell), Some(tpl)) = (row.cells.get_mut(at), tpl) { + copy_cell_format(cell, &tpl); + } + } + let grown = t.width(); + if keep_width && total > 1e-9 && grown > 1e-9 { + let k = total / grown; + for cw in &mut t.cols { + *cw *= k; + } + } +} + +/// `dst` takes `src`'s formatting (fill, borders, margins, anchor, text direction and the text +/// properties of its first paragraph, kept on the end mark of an empty paragraph) and keeps its +/// own spans and merge cover. +fn copy_cell_format(dst: &mut Cell, src: &Cell) { + dst.fill.clone_from(&src.fill); + dst.borders.clone_from(&src.borders); + dst.diag_down.clone_from(&src.diag_down); + dst.diag_up.clone_from(&src.diag_up); + dst.margins = src.margins; + dst.anchor = src.anchor; + dst.vertical = src.vertical; + let para = src.text.paragraphs.first().map(|p0| deckcraft_model::text::Paragraph { + level: p0.level, + props: p0.props.clone(), + runs: vec![], + def_props: p0.def_props.clone(), + end_props: p0.runs.first().map_or_else(|| p0.end_props.clone(), |r| r.props.clone()), + }); + dst.text = deckcraft_model::text::TextBody { + body: src.text.body.clone(), + list_style: src.text.list_style.clone(), + paragraphs: vec![para.unwrap_or_default()], + }; +} + fn merge(s: &mut Session, p: &Value) -> Result { let sel = s.doc()?.selection.cells; let from = cell_param(p, "from", "table.merge")?.or(sel.map(|c| c.1)).ok_or_else(|| bad("table.merge", "missing `from`"))?; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 85cbe9d8c..0fcdceb92 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5956,3 +5956,45 @@ fn invalid_slide_index_never_falls_back_to_the_selection_in_export_commands() { } let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn table_insert_column_keeps_the_table_width_and_copies_the_neighbour_format() { + let mut s = session(); + let id = table_at(&mut s, &[280.0, 280.0, 280.0], json!([["a", "b", "c"], ["d", "e", "f"]])); + s.execute("table.cellFill", &json!({"id": id.0, "color": "#FF0000", "cells": [[0, 2], [1, 2]]})).unwrap(); + s.execute("table.cellMargins", &json!({"id": id.0, "left": 20, "right": 20, "top": 5, "bottom": 5, "cells": [[0, 2]]})).unwrap(); + s.execute("table.cellAlign", &json!({"id": id.0, "anchor": "bottom", "cells": [[0, 2]]})).unwrap(); + s.execute("edit.select", &json!({"ids": [id.0]})).unwrap(); + s.execute("table.selectCells", &json!({"from": [0, 2], "to": [1, 2]})).unwrap(); + s.execute("format.bold", &json!({"bold": true})).unwrap(); + // Right of the last column: four columns in the same 840 pt, the new one as wide as its + // neighbour, the others shrunk to make room. + s.execute("table.insertColumnRight", &json!({"id": id.0, "col": 2})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 4); + assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); + assert!(t.cols.iter().all(|w| (w - 210.0).abs() < 1e-6), "{:?}", t.cols); + let x = s.doc().unwrap().shape(id).unwrap().xfrm.unwrap(); + assert!((x.w - 840.0).abs() < 1e-6, "frame {}", x.w); + // The new cells take the neighbour's explicit formatting. + let new = t.cell(0, 3).unwrap(); + assert_eq!(new.fill, Some(deckcraft_model::Fill::solid(deckcraft_model::ColorRef::rgb(deckcraft_color::Rgba::rgb(255, 0, 0))))); + assert_eq!(new.margins, Some([20.0, 20.0, 5.0, 5.0])); + assert_eq!(new.anchor, Some(deckcraft_model::text::Anchor::Bottom)); + assert_eq!(new.text.text(), "", "the new cell is empty"); + let bold = new.text.paragraphs.first().map(|p| p.end_props.bold.or(p.def_props.bold)); + assert_eq!(bold, Some(Some(true)), "the neighbour's bold carries over: {:?}", new.text); + assert_eq!(t.cell(1, 3).unwrap().fill, t.cell(1, 2).unwrap().fill); + // Left of the first column: the neighbour is the old first column. + s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 0})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 5); + assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); + assert_eq!(t.cell(0, 1).unwrap().text.text(), "a"); + assert_eq!(t.cell(0, 0).unwrap().fill, None); + // keepWidth: false grows the table by the neighbour's width, as before. + s.execute("table.insertColumnRight", &json!({"id": id.0, "col": 4, "keepWidth": false})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 6); + assert!((t.width() - 840.0 - 168.0).abs() < 1e-6, "width {}", t.width()); +} From 386faa67e5e0aa2e27bef2f765d470bf036d451c Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Sun, 18 Oct 2026 18:23:05 -0500 Subject: [PATCH 2/5] table.insertColumn* refuses a `keepWidth` that is not a bool, so a mistyped value is not taken as the default A `"keepWidth": "false"` was read as absent and the columns shrunk as if `true` had been asked; now the command answers an error naming the field and leaves the table alone, as the other bool params that are validated do. Co-Authored-By: Claude Fable 5.1 --- crates/engine/src/cmd/table.rs | 10 +++++++++- crates/engine/src/tests.rs | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 23f591719..930849ef8 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -321,7 +321,15 @@ fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> R fn rows_cols(s: &mut Session, p: &Value, op: &str, name: &'static str) -> Result { let (r, c) = at(s, p, name)?; - let keep_width = bool_or(p, "keepWidth", true); + // Absent: keep the table's width; present, it must be a bool, so a mistyped value is refused + // rather than silently taken as the default. + let keep_width = match (op, p.get("keepWidth")) { + (_, None) | (_, Some(Value::Null)) => true, + (_, Some(Value::Bool(b))) => *b, + ("colLeft", Some(_)) => return Err(bad("table.insertColumnLeft", "`keepWidth` must be true or false")), + ("colRight", Some(_)) => return Err(bad("table.insertColumnRight", "`keepWidth` must be true or false")), + _ => true, + }; let op = op.to_string(); with_table(s, p, name, move |t| { match op.as_str() { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0fcdceb92..11be0181b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5998,3 +5998,18 @@ fn table_insert_column_keeps_the_table_width_and_copies_the_neighbour_format() { assert_eq!(t.n_cols(), 6); assert!((t.width() - 840.0 - 168.0).abs() < 1e-6, "width {}", t.width()); } + +/// A `keepWidth` that is not a bool is refused, naming it, and the table is left alone; it used +/// to be read as absent and the columns shrunk as if `true` had been asked. +#[test] +fn table_insert_column_refuses_a_keep_width_that_is_not_a_bool() { + let mut s = session(); + let id = table_at(&mut s, &[280.0, 280.0, 280.0], json!([["a", "b", "c"]])); + for (cmd, bad) in [("table.insertColumnRight", json!("false")), ("table.insertColumnLeft", json!(0))] { + let err = s.execute(cmd, &json!({"id": id.0, "col": 1, "keepWidth": bad})).unwrap_err().to_string(); + assert!(err.contains("keepWidth") && err.contains(cmd), "{err}"); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 3, "{cmd} changed the table: {:?}", t.cols); + assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); + } +} From 2de50400bc54ae8985abc8576cd08a7eeea65283 Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Sun, 18 Oct 2026 18:30:01 -0500 Subject: [PATCH 3/5] The column-insertion test also checks the journal entry and that each insertion is one undo step The test went through Session::execute but only read the table afterwards; it now checks the last journal entry is the call as made and that three undos bring back the three 280 pt columns, their text and the frame width. Co-Authored-By: Claude Fable 5.1 --- crates/engine/src/tests.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 11be0181b..88a74236c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5997,6 +5997,23 @@ fn table_insert_column_keeps_the_table_width_and_copies_the_neighbour_format() { let t = table_of(&s, id); assert_eq!(t.n_cols(), 6); assert!((t.width() - 840.0 - 168.0).abs() < 1e-6, "width {}", t.width()); + // The insertion is journaled as called, and each one is a single undo step back to the table + // as it was: the columns, their widths and the frame. + let (jid, jp) = s.journal.last().cloned().unwrap(); + assert_eq!(jid, "table.insertColumnRight"); + assert_eq!(jp["keepWidth"], json!(false), "{jp}"); + s.execute("edit.undo", &json!({})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 5); + assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); + s.execute("edit.undo", &json!({})).unwrap(); + s.execute("edit.undo", &json!({})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 3); + assert!(t.cols.iter().all(|w| (w - 280.0).abs() < 1e-6), "{:?}", t.cols); + assert_eq!(t.cell(0, 2).unwrap().text.text(), "c"); + let x = s.doc().unwrap().shape(id).unwrap().xfrm.unwrap(); + assert!((x.w - 840.0).abs() < 1e-6, "frame {}", x.w); } /// A `keepWidth` that is not a bool is refused, naming it, and the table is left alone; it used From 22b342b68be57c8af212aa9b2c6c5c4068d64306 Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:18:25 -0500 Subject: [PATCH 4/5] [EE1] table.insertColumn* extends a merge that spans the neighbour column A horizontal merge covering the column the new one is inserted beside now grows over it, also at its edge (a merged title row keeps spanning the table). Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/table.rs | 14 +++++++++---- crates/engine/src/tests.rs | 38 ++++++++++++++++++++++++++++++++++ crates/model/src/table.rs | 34 +++++++++++++++++++++++++++++- 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 930849ef8..76be6bfb0 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -29,7 +29,7 @@ pub fn specs() -> Vec { "Insert Left", ["Table Layout", "Rows & Columns"], None, - "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside", + "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside; a merged row that spans that column grows to span the new one", has_selection, |s, p| rows_cols(s, p, "colLeft", "table.insertColumnLeft") ), @@ -38,7 +38,7 @@ pub fn specs() -> Vec { "Insert Right", ["Table Layout", "Rows & Columns"], None, - "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside", + "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside; a merged row that spans that column grows to span the new one", has_selection, |s, p| rows_cols(s, p, "colRight", "table.insertColumnRight") ), @@ -361,13 +361,19 @@ fn insert_col_like(t: &mut Table, at: usize, neighbour: usize, keep_width: bool) let neighbour = neighbour.min(t.cols.len().saturating_sub(1)); let w = t.cols.get(neighbour).copied(); let templates: Vec> = t.rows.iter().map(|r| r.cells.get(neighbour).cloned()).collect(); - t.insert_col(at); + t.insert_col_beside(at, Some(neighbour)); let at = at.min(t.cols.len().saturating_sub(1)); if let (Some(w), Some(slot)) = (w, t.cols.get_mut(at)) { *slot = w; } for (row, tpl) in t.rows.iter_mut().zip(templates) { - if let (Some(cell), Some(tpl)) = (row.cells.get_mut(at), tpl) { + let Some(tpl) = tpl else { continue }; + // Inserted left of a merge's anchor, the merge now starts at the new cell, which took the + // anchor's cell whole, text and formatting (Table::insert_col_beside); the old anchor, + // covered now, is the empty cell that gets the neighbour's formatting. Read off the table + // as it is now: a span the file lets run past the last column is no merge to the model. + let anchor_moved = row.cells.get(at).is_some_and(|c| c.grid_span > 1 && !c.is_covered()); + if let Some(cell) = row.cells.get_mut(if anchor_moved { at + 1 } else { at }) { copy_cell_format(cell, &tpl); } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 88a74236c..52710a393 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6030,3 +6030,41 @@ fn table_insert_column_refuses_a_keep_width_that_is_not_a_bool() { assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); } } + +#[test] +fn table_insert_column_extends_a_merge_that_spans_the_neighbour() { + let mut s = session(); + let id = table_at(&mut s, &[100.0, 100.0, 100.0], json!([["title", "", ""], ["a", "b", "c"]])); + s.execute("table.merge", &json!({"id": id.0, "from": [0, 0], "to": [0, 2]})).unwrap(); + s.execute("table.insertColumnRight", &json!({"id": id.0, "col": 2})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.cell(0, 0).unwrap().grid_span, 4, "the title now spans the new column"); + assert!(t.cell(0, 3).unwrap().h_merge); + assert_eq!(t.cell(1, 0).unwrap().grid_span, 1); + s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 0})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.cell(0, 0).unwrap().grid_span, 5, "left of the first column too"); + assert_eq!(t.cell(0, 0).unwrap().text.text(), "title"); +} + +/// A span a file lets run past the last column is no merge to the model, so inserting left of +/// that cell adds a plain column: the cell keeps its text and the new one gets its formatting. +#[test] +fn table_insert_column_left_of_an_overflowing_span_keeps_the_text() { + let mut s = session(); + let id = table_at(&mut s, &[100.0, 100.0, 100.0], json!([["a", "b", "wide"], ["d", "e", "f"]])); + { + let doc = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let slide = std::sync::Arc::make_mut(&mut doc.slides[0]); + let sh = deckcraft_model::find_shape_mut(&mut slide.shapes, id).unwrap(); + let deckcraft_model::ShapeKind::Table(t) = &mut sh.kind else { panic!("a table") }; + t.cell_mut(0, 2).unwrap().grid_span = 4; + } + s.execute("table.cellFill", &json!({"id": id.0, "color": "#FF0000", "cells": [[0, 2]]})).unwrap(); + s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 2})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 4); + assert_eq!(t.cell(0, 3).unwrap().text.text(), "wide", "the cell keeps its text"); + assert_eq!(t.cell(0, 2).unwrap().text.text(), ""); + assert_eq!(t.cell(0, 2).unwrap().fill, t.cell(0, 3).unwrap().fill, "the new cell takes the neighbour's fill"); +} diff --git a/crates/model/src/table.rs b/crates/model/src/table.rs index 0705d1a08..ce0f0434a 100644 --- a/crates/model/src/table.rs +++ b/crates/model/src/table.rs @@ -116,6 +116,12 @@ impl Table { } /// Insert an empty column before `at`. A merge the new column falls inside grows to cover it. pub fn insert_col(&mut self, at: usize) { + self.insert_col_beside(at, None); + } + /// Like [`Table::insert_col`], for a column inserted beside column `neighbour` (as before the + /// insertion): a horizontal merge that spans the neighbour also grows over the new column, even when the + /// insertion point is at its edge. + pub fn insert_col_beside(&mut self, at: usize, neighbour: Option) { let at = at.min(self.cols.len()); let merges = self.merges(); let w = self.cols.get(at.saturating_sub(1)).copied().unwrap_or(80.0); @@ -124,7 +130,13 @@ impl Table { let i = at.min(r.cells.len()); r.cells.insert(i, Cell::new("")); } - self.set_merges(merges.into_iter().map(|m| m.insert_col(at))); + let grows = |m: &Merge| matches!(neighbour, Some(n) if m.gs > 1 && m.c <= n && n < m.c + m.gs && (at == m.c || at == m.c + m.gs)); + // A merge that grows to the left starts at the new cell, which takes the anchor's text and + // formatting; the old anchor becomes a covered, empty cell. + for m in merges.iter().filter(|m| grows(m) && at == m.c) { + self.move_anchor((m.r, at + 1), (m.r, at)); + } + self.set_merges(merges.into_iter().map(|m| if grows(&m) { Merge { gs: m.gs + 1, ..m } } else { m.insert_col(at) })); } /// Delete row `at`. Merges through it shrink; a merge anchored on it moves its anchor (and /// text) to the next row it covers. @@ -638,4 +650,24 @@ mod tests { assert!(t.split(0, 0)); assert_eq!(t.cell(0, 0).map(|c| (c.grid_span, c.row_span)), Some((1, 1))); } + + /// A column inserted left of a merge's anchor grows the merge, which now starts at the new + /// cell: that cell holds the text, and the old anchor is a covered, empty cell. + #[test] + fn a_column_inserted_left_of_a_merge_keeps_the_text_in_its_anchor() { + let mut t = merged(); + t.insert_col_beside(1, Some(1)); + assert_topology(&t); + let a = t.cell(1, 1).unwrap(); + assert_eq!((a.grid_span, a.row_span), (3, 2)); + assert_eq!(anchor_text(&t, 1, 1), "M"); + assert!(t.cell(1, 2).unwrap().is_covered()); + assert_eq!(anchor_text(&t, 1, 2), ""); + // Right of the merge, the anchor stays where it is. + let mut t = merged(); + t.insert_col_beside(3, Some(2)); + assert_topology(&t); + assert_eq!(t.cell(1, 1).map(|c| c.grid_span), Some(3)); + assert_eq!(anchor_text(&t, 1, 1), "M"); + } } From f7c1c21c5bdcf156328ad5913dd34cfe7589cc29 Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:07:03 -0500 Subject: [PATCH 5/5] [CCC1] table.insertColumn leaves the other column widths alone PowerPoint inserts the new column as wide as the column it is inserted beside and widens the table by that amount; the existing widths stay. DeckCraft rescaled every column to keep the table width by default. keepWidth now defaults to false (true stays as an opt-in). Deleting a column already narrows the table by its width and leaves the rest alone; a test pins both. Co-Authored-By: Claude Fable 5.1 --- crates/engine/src/cmd/table.rs | 12 +-- crates/engine/src/tests.rs | 129 ++++++++++++++++++++------------- 2 files changed, 85 insertions(+), 56 deletions(-) diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 76be6bfb0..7ea66acfd 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -29,7 +29,7 @@ pub fn specs() -> Vec { "Insert Left", ["Table Layout", "Rows & Columns"], None, - "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside; a merged row that spans that column grows to span the new one", + "{col?, keepWidth?: bool (default false, as in PowerPoint: the table grows by the new column and the other widths stay; true: the other columns shrink in proportion so the table keeps its width), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside; a merged row that spans that column grows to span the new one", has_selection, |s, p| rows_cols(s, p, "colLeft", "table.insertColumnLeft") ), @@ -38,7 +38,7 @@ pub fn specs() -> Vec { "Insert Right", ["Table Layout", "Rows & Columns"], None, - "{col?, keepWidth?: bool (default true: the other columns shrink in proportion so the table keeps its width; false: the table grows by the new column), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside; a merged row that spans that column grows to span the new one", + "{col?, keepWidth?: bool (default false, as in PowerPoint: the table grows by the new column and the other widths stay; true: the other columns shrink in proportion so the table keeps its width), id?} the new column takes the width and cell formatting (fill, margins, anchor, text properties) of the column it is inserted beside; a merged row that spans that column grows to span the new one", has_selection, |s, p| rows_cols(s, p, "colRight", "table.insertColumnRight") ), @@ -321,14 +321,14 @@ fn with_table(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut Table) -> R fn rows_cols(s: &mut Session, p: &Value, op: &str, name: &'static str) -> Result { let (r, c) = at(s, p, name)?; - // Absent: keep the table's width; present, it must be a bool, so a mistyped value is refused + // Absent: the table grows, as in PowerPoint; present, it must be a bool, so a mistyped value is refused // rather than silently taken as the default. let keep_width = match (op, p.get("keepWidth")) { - (_, None) | (_, Some(Value::Null)) => true, + (_, None) | (_, Some(Value::Null)) => false, (_, Some(Value::Bool(b))) => *b, ("colLeft", Some(_)) => return Err(bad("table.insertColumnLeft", "`keepWidth` must be true or false")), ("colRight", Some(_)) => return Err(bad("table.insertColumnRight", "`keepWidth` must be true or false")), - _ => true, + _ => false, }; let op = op.to_string(); with_table(s, p, name, move |t| { @@ -355,7 +355,7 @@ fn rows_cols(s: &mut Session, p: &Value, op: &str, name: &'static str) -> Result /// Insert a column before `at` that looks like column `neighbour` (the one it is inserted beside): /// its width, and in every row the neighbour cell's explicit formatting with no text. With /// `keep_width` the columns then shrink in proportion so the table keeps its total width; -/// otherwise the table grows by the new column. +/// otherwise (PowerPoint's behaviour) the table grows by the new column and the widths stay. fn insert_col_like(t: &mut Table, at: usize, neighbour: usize, keep_width: bool) { let total = t.width(); let neighbour = neighbour.min(t.cols.len().saturating_sub(1)); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 52710a393..2fd3c23ec 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5957,8 +5957,61 @@ fn invalid_slide_index_never_falls_back_to_the_selection_in_export_commands() { let _ = std::fs::remove_dir_all(&dir); } +/// A `keepWidth` that is not a bool is refused, naming it, and the table is left alone; it used +/// to be read as absent and the columns shrunk as if `true` had been asked. +#[test] +fn table_insert_column_refuses_a_keep_width_that_is_not_a_bool() { + let mut s = session(); + let id = table_at(&mut s, &[280.0, 280.0, 280.0], json!([["a", "b", "c"]])); + for (cmd, bad) in [("table.insertColumnRight", json!("false")), ("table.insertColumnLeft", json!(0))] { + let err = s.execute(cmd, &json!({"id": id.0, "col": 1, "keepWidth": bad})).unwrap_err().to_string(); + assert!(err.contains("keepWidth") && err.contains(cmd), "{err}"); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 3, "{cmd} changed the table: {:?}", t.cols); + assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); + } +} + +#[test] +fn table_insert_column_extends_a_merge_that_spans_the_neighbour() { + let mut s = session(); + let id = table_at(&mut s, &[100.0, 100.0, 100.0], json!([["title", "", ""], ["a", "b", "c"]])); + s.execute("table.merge", &json!({"id": id.0, "from": [0, 0], "to": [0, 2]})).unwrap(); + s.execute("table.insertColumnRight", &json!({"id": id.0, "col": 2})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.cell(0, 0).unwrap().grid_span, 4, "the title now spans the new column"); + assert!(t.cell(0, 3).unwrap().h_merge); + assert_eq!(t.cell(1, 0).unwrap().grid_span, 1); + s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 0})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.cell(0, 0).unwrap().grid_span, 5, "left of the first column too"); + assert_eq!(t.cell(0, 0).unwrap().text.text(), "title"); +} + +/// A span a file lets run past the last column is no merge to the model, so inserting left of +/// that cell adds a plain column: the cell keeps its text and the new one gets its formatting. +#[test] +fn table_insert_column_left_of_an_overflowing_span_keeps_the_text() { + let mut s = session(); + let id = table_at(&mut s, &[100.0, 100.0, 100.0], json!([["a", "b", "wide"], ["d", "e", "f"]])); + { + let doc = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); + let slide = std::sync::Arc::make_mut(&mut doc.slides[0]); + let sh = deckcraft_model::find_shape_mut(&mut slide.shapes, id).unwrap(); + let deckcraft_model::ShapeKind::Table(t) = &mut sh.kind else { panic!("a table") }; + t.cell_mut(0, 2).unwrap().grid_span = 4; + } + s.execute("table.cellFill", &json!({"id": id.0, "color": "#FF0000", "cells": [[0, 2]]})).unwrap(); + s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 2})).unwrap(); + let t = table_of(&s, id); + assert_eq!(t.n_cols(), 4); + assert_eq!(t.cell(0, 3).unwrap().text.text(), "wide", "the cell keeps its text"); + assert_eq!(t.cell(0, 2).unwrap().text.text(), ""); + assert_eq!(t.cell(0, 2).unwrap().fill, t.cell(0, 3).unwrap().fill, "the new cell takes the neighbour's fill"); +} + #[test] -fn table_insert_column_keeps_the_table_width_and_copies_the_neighbour_format() { +fn table_insert_column_with_keep_width_keeps_the_table_width_and_copies_the_neighbour_format() { let mut s = session(); let id = table_at(&mut s, &[280.0, 280.0, 280.0], json!([["a", "b", "c"], ["d", "e", "f"]])); s.execute("table.cellFill", &json!({"id": id.0, "color": "#FF0000", "cells": [[0, 2], [1, 2]]})).unwrap(); @@ -5966,10 +6019,10 @@ fn table_insert_column_keeps_the_table_width_and_copies_the_neighbour_format() { s.execute("table.cellAlign", &json!({"id": id.0, "anchor": "bottom", "cells": [[0, 2]]})).unwrap(); s.execute("edit.select", &json!({"ids": [id.0]})).unwrap(); s.execute("table.selectCells", &json!({"from": [0, 2], "to": [1, 2]})).unwrap(); - s.execute("format.bold", &json!({"bold": true})).unwrap(); + s.execute("format.bold", &json!({"on": true})).unwrap(); // Right of the last column: four columns in the same 840 pt, the new one as wide as its // neighbour, the others shrunk to make room. - s.execute("table.insertColumnRight", &json!({"id": id.0, "col": 2})).unwrap(); + s.execute("table.insertColumnRight", &json!({"id": id.0, "col": 2, "keepWidth": true})).unwrap(); let t = table_of(&s, id); assert_eq!(t.n_cols(), 4); assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); @@ -5986,7 +6039,7 @@ fn table_insert_column_keeps_the_table_width_and_copies_the_neighbour_format() { assert_eq!(bold, Some(Some(true)), "the neighbour's bold carries over: {:?}", new.text); assert_eq!(t.cell(1, 3).unwrap().fill, t.cell(1, 2).unwrap().fill); // Left of the first column: the neighbour is the old first column. - s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 0})).unwrap(); + s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 0, "keepWidth": true})).unwrap(); let t = table_of(&s, id); assert_eq!(t.n_cols(), 5); assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); @@ -6016,55 +6069,31 @@ fn table_insert_column_keeps_the_table_width_and_copies_the_neighbour_format() { assert!((x.w - 840.0).abs() < 1e-6, "frame {}", x.w); } -/// A `keepWidth` that is not a bool is refused, naming it, and the table is left alone; it used -/// to be read as absent and the columns shrunk as if `true` had been asked. +/// PowerPoint inserts a column as wide as its neighbour and widens the table by that amount; the +/// other widths stay. Deleting a column narrows the table by its width, the others untouched. #[test] -fn table_insert_column_refuses_a_keep_width_that_is_not_a_bool() { +fn table_insert_and_delete_column_leave_other_widths_alone() { let mut s = session(); - let id = table_at(&mut s, &[280.0, 280.0, 280.0], json!([["a", "b", "c"]])); - for (cmd, bad) in [("table.insertColumnRight", json!("false")), ("table.insertColumnLeft", json!(0))] { - let err = s.execute(cmd, &json!({"id": id.0, "col": 1, "keepWidth": bad})).unwrap_err().to_string(); - assert!(err.contains("keepWidth") && err.contains(cmd), "{err}"); + let id = table_at(&mut s, &[160.0, 234.0, 235.0, 235.0], json!([["a", "b", "c", "d"]])); + for (cmd, col, at, wide) in [("table.insertColumnRight", 0, 1, 160.0), ("table.insertColumnLeft", 3, 3, 235.0)] { + let before = table_of(&s, id).cols.clone(); + let w0 = table_of(&s, id).width(); + s.execute(cmd, &json!({"id": id.0, "col": col})).unwrap(); let t = table_of(&s, id); - assert_eq!(t.n_cols(), 3, "{cmd} changed the table: {:?}", t.cols); - assert!((t.width() - 840.0).abs() < 1e-6, "width {}", t.width()); - } -} - -#[test] -fn table_insert_column_extends_a_merge_that_spans_the_neighbour() { - let mut s = session(); - let id = table_at(&mut s, &[100.0, 100.0, 100.0], json!([["title", "", ""], ["a", "b", "c"]])); - s.execute("table.merge", &json!({"id": id.0, "from": [0, 0], "to": [0, 2]})).unwrap(); - s.execute("table.insertColumnRight", &json!({"id": id.0, "col": 2})).unwrap(); - let t = table_of(&s, id); - assert_eq!(t.cell(0, 0).unwrap().grid_span, 4, "the title now spans the new column"); - assert!(t.cell(0, 3).unwrap().h_merge); - assert_eq!(t.cell(1, 0).unwrap().grid_span, 1); - s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 0})).unwrap(); - let t = table_of(&s, id); - assert_eq!(t.cell(0, 0).unwrap().grid_span, 5, "left of the first column too"); - assert_eq!(t.cell(0, 0).unwrap().text.text(), "title"); -} - -/// A span a file lets run past the last column is no merge to the model, so inserting left of -/// that cell adds a plain column: the cell keeps its text and the new one gets its formatting. -#[test] -fn table_insert_column_left_of_an_overflowing_span_keeps_the_text() { - let mut s = session(); - let id = table_at(&mut s, &[100.0, 100.0, 100.0], json!([["a", "b", "wide"], ["d", "e", "f"]])); - { - let doc = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); - let slide = std::sync::Arc::make_mut(&mut doc.slides[0]); - let sh = deckcraft_model::find_shape_mut(&mut slide.shapes, id).unwrap(); - let deckcraft_model::ShapeKind::Table(t) = &mut sh.kind else { panic!("a table") }; - t.cell_mut(0, 2).unwrap().grid_span = 4; + let mut want = before.clone(); + want.insert(at, wide); + assert_eq!(t.cols, want, "{cmd}"); + assert!((t.width() - w0 - wide).abs() < 1e-6, "{cmd} width {}", t.width()); + let x = s.doc().unwrap().shape(id).unwrap().xfrm.unwrap(); + assert!((x.w - t.width()).abs() < 1e-6, "{cmd} frame {}", x.w); } - s.execute("table.cellFill", &json!({"id": id.0, "color": "#FF0000", "cells": [[0, 2]]})).unwrap(); - s.execute("table.insertColumnLeft", &json!({"id": id.0, "col": 2})).unwrap(); + let before = table_of(&s, id).cols.clone(); + s.execute("table.deleteColumn", &json!({"id": id.0, "col": 1})).unwrap(); let t = table_of(&s, id); - assert_eq!(t.n_cols(), 4); - assert_eq!(t.cell(0, 3).unwrap().text.text(), "wide", "the cell keeps its text"); - assert_eq!(t.cell(0, 2).unwrap().text.text(), ""); - assert_eq!(t.cell(0, 2).unwrap().fill, t.cell(0, 3).unwrap().fill, "the new cell takes the neighbour's fill"); + let mut want = before.clone(); + let gone = want.remove(1); + assert_eq!(t.cols, want); + assert!((t.width() - (before.iter().sum::() - gone)).abs() < 1e-6); + let x = s.doc().unwrap().shape(id).unwrap().xfrm.unwrap(); + assert!((x.w - t.width()).abs() < 1e-6, "frame {}", x.w); }