Insert Table: the style in the same step, and the engine's limits in the dialog

- table.insert takes tableStyle and applies it in its own edit, so one Insert
  Table is one undo, and a style that is gone (or not a name) changes nothing.
  The dialog passes it instead of running style.table.apply afterwards.
- The dialog's fields stop at table.insert's limits (500 body rows, 200
  columns, 100 header or footer rows; now public constants), so the table made
  is the one shown.

Found by CodeRabbit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
moppeeer
2026-10-23 22:04:42 +02:00
co-authored by Claude Opus 5.5
parent bbe29dfecc
commit dd2d7e6949
3 changed files with 76 additions and 22 deletions
+31 -5
View File
@@ -90,7 +90,7 @@ fn table_specs() -> Vec<CommandSpec> {
[],
// InDesign's Cmd+Alt+Shift+T opens the Insert Table dialog (`app.insertTableDialog`).
None,
"{rows?: body rows (4), cols? | columns? (4), headerRows? (0), footerRows? (0), width?} — at the text insertion point",
"{rows?: body rows (4, at most 500), cols? | columns? (4, at most 200), headerRows? (0), footerRows? (0, each at most 100), tableStyle?: name, width?} — at the text insertion point, styled in the same step",
has_text,
insert
),
@@ -564,15 +564,38 @@ fn para_direction(d: &Document, story: StoryId, offset: usize) -> designcraft_do
.map_or(designcraft_doc::TextDirection::LeftToRight, |pf| d.styles.resolve_para(pf).0.direction)
}
/// The most body rows, columns, and header or footer rows `table.insert` makes (the Insert Table
/// dialog's limits too).
pub const MAX_BODY_ROWS: u64 = 500;
pub const MAX_COLUMNS: u64 = 200;
pub const MAX_HEADER_FOOTER_ROWS: u64 = 100;
/// A command's `tableStyle`: none for an absent one or [Basic Table] (a new table's look), else the
/// resolved style and the cell styles it names. Anything but an existing style's name is an error.
fn table_style_param(s: &Session, p: &Value, cmd: &str) -> Result<Option<(designcraft_doc::TableStyle, Vec<designcraft_doc::CellStyle>)>> {
let name = match p.get("tableStyle") {
None | Some(Value::Null) => return Ok(None),
Some(Value::String(n)) if n == designcraft_doc::BASIC_TABLE => return Ok(None),
Some(Value::String(n)) => n,
Some(_) => return Err(bad(cmd, "`tableStyle` must be a table style's name")),
};
let styles = &s.doc()?.doc.styles;
if !styles.table.iter().any(|x| x.name == *name) {
return Err(bad(cmd, format!("no table style `{name}`")));
}
Ok(Some((styles.resolve_table_style(name), styles.cell.clone())))
}
fn insert(s: &mut Session, p: &Value) -> Result<Value> {
let t = s.doc()?.selection.text.ok_or_else(|| bad("table.insert", "no insertion point"))?;
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;
let rows = p.get("rows").and_then(Value::as_u64).unwrap_or(4).clamp(1, MAX_BODY_ROWS) as usize;
let cols = p.get("cols").or_else(|| p.get("columns")).and_then(Value::as_u64).unwrap_or(4).clamp(1, MAX_COLUMNS) as usize;
let header = p.get("headerRows").and_then(Value::as_u64).unwrap_or(0).min(MAX_HEADER_FOOTER_ROWS) as usize;
let footer = p.get("footerRows").and_then(Value::as_u64).unwrap_or(0).min(MAX_HEADER_FOOTER_ROWS) as usize;
let style = table_style_param(s, p, "table.insert")?;
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();
@@ -586,6 +609,9 @@ fn insert(s: &mut Session, p: &Value) -> Result<Value> {
c.text.paras[0] = designcraft_doc::ParaFormat { table: None, ..pf.clone() };
c.text.chars[0].format = cf.clone();
}
if let Some((ts, cells)) = &style {
ts.apply_to(&mut table, cells);
}
let r = t.range();
st.delete(r.clone());
st.insert_table(r.start, table);
+20
View File
@@ -426,6 +426,26 @@ fn rows_columns_merge_and_options() {
assert_eq!(st.doc.story(sid).unwrap().text, "Intro");
}
/// `table.insert` styles the table in the same step; a style that isn't there, or isn't a name,
/// leaves the story alone. Its dimensions stop at the documented limits.
#[test]
fn insert_table_with_a_style_is_one_step() {
let (mut s, sid, _) = session_with_frame();
s.execute("style.table.create", &json!({"name": "Grid"})).unwrap();
for bad in [json!("Missing"), json!(7)] {
assert!(s.execute("table.insert", &json!({"tableStyle": bad})).is_err(), "{bad}");
}
assert!(s.doc().unwrap().doc.story(designcraft_doc::StoryId(sid)).unwrap().tables.is_empty());
let r = s.execute("table.insert", &json!({"rows": 900, "cols": 300, "headerRows": 150, "tableStyle": "Grid"})).unwrap();
let t = s.execute("table.get", &json!({"table": r["table"]})).unwrap();
assert_eq!(t["style"], "Grid");
let n = |k: &str| t[k].as_array().unwrap().len() as u64;
use crate::cmd::table::{MAX_BODY_ROWS, MAX_COLUMNS, MAX_HEADER_FOOTER_ROWS};
assert_eq!((n("rows"), n("columns")), (MAX_BODY_ROWS + MAX_HEADER_FOOTER_ROWS, MAX_COLUMNS));
s.execute("edit.undo", &json!({})).unwrap();
assert!(s.doc().unwrap().doc.story(designcraft_doc::StoryId(sid)).unwrap().tables.is_empty(), "one undo");
}
#[test]
fn convert_text_to_table_and_back() {
let (mut s, sid, _) = session_with_frame();
+25 -17
View File
@@ -143,11 +143,13 @@ fn text_field(ui: &mut egui::Ui, d: &mut Dialog, key: &str, w: f32) {
fn insert_table_dialog(app: &DesignApp, ui: &mut egui::Ui, d: &mut Dialog) {
let lang = &app.ui.language;
crate::rtl::label(ui, egui::RichText::new(crate::i18n::tr(lang, "Table Dimensions")).font(semibold(12.0)));
use designcraft_engine::cmd::table::{MAX_BODY_ROWS, MAX_COLUMNS, MAX_HEADER_FOOTER_ROWS};
// `table.insert`'s limits, so the table made is the one the fields show.
let rows = [
("bodyRows", crate::i18n::tr(lang, "Body Rows"), 1.0),
("columns", crate::i18n::tr_context(lang, "Columns", "table"), 1.0),
("headerRows", crate::i18n::tr(lang, "Header Rows"), 0.0),
("footerRows", crate::i18n::tr(lang, "Footer Rows"), 0.0),
("bodyRows", crate::i18n::tr(lang, "Body Rows"), 1.0, MAX_BODY_ROWS),
("columns", crate::i18n::tr_context(lang, "Columns", "table"), 1.0, MAX_COLUMNS),
("headerRows", crate::i18n::tr(lang, "Header Rows"), 0.0, MAX_HEADER_FOOTER_ROWS),
("footerRows", crate::i18n::tr(lang, "Footer Rows"), 0.0, MAX_HEADER_FOOTER_ROWS),
];
// Body rows and columns, then (after a gap) header and footer rows; one label width for both.
for (g, pair) in rows.chunks(2).enumerate() {
@@ -155,9 +157,9 @@ fn insert_table_dialog(app: &DesignApp, ui: &mut egui::Ui, d: &mut Dialog) {
ui.add_space(8.0);
}
egui::Grid::new(("ins_table", g)).num_columns(2).min_col_width(100.0).spacing([8.0, 6.0]).show(ui, |ui| {
for (key, label, min) in pair {
for (key, label, min, max) in pair {
ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| crate::rtl::label(ui, format!("{label}:")));
let field = crate::widgets::NumField::number(key, d.n(key), "", 0).width(70.0).range(*min, 10_000.0).spinner();
let field = crate::widgets::NumField::number(key, d.n(key), "", 0).width(70.0).range(*min, *max as f64).spinner();
if let Some(v) = field.show(ui) {
d.fields.insert((*key).into(), json!(v.round()));
}
@@ -3128,20 +3130,16 @@ pub fn confirm(app: &mut DesignApp) -> Result<Value, String> {
None => Err(format!("no page \"{typed}\"")),
}
}
"insertTable" => app
.run(
"insertTable" => {
// The table and its style in one step; a style that is gone leaves the document alone.
let style = Some(d.s("tableStyle")).filter(|s| !s.is_empty());
app.run(
"table.insert",
json!({"rows": d.n("bodyRows").unwrap_or(4.0).max(1.0) as u64, "cols": d.n("columns").unwrap_or(4.0).max(1.0) as u64,
"headerRows": d.n("headerRows").unwrap_or(0.0).max(0.0) as u64, "footerRows": d.n("footerRows").unwrap_or(0.0).max(0.0) as u64}),
"headerRows": d.n("headerRows").unwrap_or(0.0).max(0.0) as u64, "footerRows": d.n("footerRows").unwrap_or(0.0).max(0.0) as u64,
"tableStyle": style}),
)
.and_then(|r| {
// A table style other than [Basic Table] (which a new table has) is applied after.
let style = d.s("tableStyle");
if style.is_empty() || style == designcraft_doc::BASIC_TABLE {
return Ok(r);
}
app.run("style.table.apply", json!({"name": style, "story": r["story"], "table": r["table"]})).map(|_| r)
}),
}
"numberingSection" => {
// On an error the dialog stays open to be corrected.
let r = crate::section_options::confirm(app, &d);
@@ -9664,7 +9662,17 @@ mod tests {
let len = |k: &str| t[k].as_array().map_or(0, Vec::len);
assert_eq!((len("rows"), len("columns"), t["headerRows"].as_u64()), (6, 3, Some(1)), "{t}");
assert_eq!(t["style"].as_str().unwrap_or(""), expected, "{t}");
// One Insert Table, one undo.
app.run("edit.undo", json!({})).unwrap();
assert!(app.run("table.get", json!({})).is_err(), "the table is gone");
}
// A style that no longer exists changes nothing.
let tables = |app: &DesignApp| app.session.active().unwrap().doc.stories.values().map(|s| s.tables.len()).sum::<usize>();
app.run("frame.create", json!({"rect": [72, 72, 372, 372], "content": "text"})).unwrap();
app.run("app.insertTableDialog", json!({})).unwrap();
ctl(&mut app, "ui.dialog.set", json!({"field": "tableStyle", "value": "Gone"}));
assert_eq!(ctl(&mut app, "ui.dialog.confirm", json!({}))["ok"], false);
assert_eq!(tables(&app), 0);
}
#[test]