diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -145,7 +145,7 @@ pub fn specs() -> Vec { "Numbering", ["Home", "Paragraph"], None, - "{on?: bool, scheme?: arabicPeriod|arabicParenR|romanUcPeriod|romanLcPeriod|alphaUcPeriod|alphaLcParenR|alphaLcPeriod, start?: n, color?, size?: % of text}", + "{on?: bool, scheme?: arabicPeriod|arabicParenR|romanUcPeriod|romanLcPeriod|alphaUcPeriod|alphaLcParenR|alphaLcPeriod, start?: n (a paragraph whose start differs from the one before begins a new run of numbers at n, as PowerPoint's Start at does), color?, size?: % of text}", has_text_or_shapes, numbering ), @@ -883,6 +883,7 @@ fn line_spacing(s: &mut Session, p: &Value) -> Result { } fn paragraph(s: &mut Session, p: &Value) -> Result { + require_any(p, "format.paragraph", &["align", "indentLeft", "indentFirst", "spaceBefore", "spaceAfter", "lineSpacing", "lineSpacingPt"])?; let a = str_param(p, "align").map(|a| match a { "center" => Align::Center, "right" => Align::Right, @@ -1132,6 +1133,7 @@ fn wrap(s: &mut Session, p: &Value) -> Result { format_body(s, &move |b| b.wrap = Some(on)) } fn margins(s: &mut Session, p: &Value) -> Result { + require_any(p, "format.margins", &["left", "top", "right", "bottom"])?; let (l, t, r, b) = (f64_param(p, "left"), f64_param(p, "top"), f64_param(p, "right"), f64_param(p, "bottom")); format_body(s, &move |bp| { if l.is_some() { diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -297,6 +297,16 @@ pub fn parse_params(doc: &str) -> std::result::Result>, String> { /// growing would duplicate ids (#407). pub(crate) const NO_ID_SPACE: &str = "the presentation has used every object id and can hold no more"; +/// A style command whose keys are all optional is refused when it is given none of them, rather +/// than answering `changed` for a call that sets nothing. +pub(crate) fn require_any(p: &Value, cmd: &str, keys: &[&str]) -> Result<()> { + if keys.iter().any(|k| p.get(*k).is_some_and(|v| !v.is_null())) { + return Ok(()); + } + let (last, rest) = keys.split_last().map_or(("", &[][..]), |(l, r)| (*l, r)); + Err(bad(cmd, format!("`{cmd}` was given nothing to set: pass {} or {last}", rest.join(", ")))) +} + pub(crate) fn bad(cmd: &str, msg: impl Into) -> EngineError { EngineError::BadParams { cmd: cmd.into(), msg: msg.into() } } diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -683,6 +683,7 @@ fn line_end(v: &str) -> Option { } fn line(s: &mut Session, p: &Value) -> Result { + require_any(p, "shape.line", &["color", "none", "width", "dash", "cap", "join", "compound", "head", "tail", "headSize", "tailSize", "reset"])?; let reset = bool_or(p, "reset", false); let none = bool_or(p, "none", false); let color = color_param(p, "color"); @@ -802,6 +803,11 @@ fn shadow_preset(v: &Value) -> Option> { } fn effects(s: &mut Session, p: &Value) -> Result { + require_any( + p, + "shape.effects", + &["shadow", "glow", "softEdges", "reflection", "bevel", "threeD", "shadowProps", "glowProps", "reflectionProps", "reset"], + )?; let reset = bool_or(p, "reset", false); let shadow = p.get("shadow").and_then(shadow_preset); let glow: Option> = p.get("glow").map(|g| match g { @@ -1085,6 +1091,7 @@ fn crop(s: &mut Session, p: &Value) -> Result { } fn picture_adjust(s: &mut Session, p: &Value) -> Result { + require_any(p, "picture.adjust", &["brightness", "contrast", "saturation", "grayscale", "transparency", "reset"])?; edit_shapes(s, p, "picture.adjust", |sh| { if let deckcraft_model::ShapeKind::Picture { fill } = &mut sh.kind { let a = &mut fill.adjust; diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -87,7 +87,7 @@ pub fn specs() -> Vec { "Align Cell Text", ["Table Layout", "Alignment"], None, - "{anchor: top|middle|bottom, cells?, id?}", + "{anchor?: top|middle|bottom (vertical; top when neither anchor nor align is given), align?: left|center|right|justify (horizontal, applied to every paragraph of the cells), cells?, id?}", has_selection, cell_align ), @@ -499,18 +499,37 @@ fn cell_margins(s: &mut Session, p: &Value) -> Result { } fn cell_align(s: &mut Session, p: &Value) -> Result { - use deckcraft_model::text::Anchor; - let a = match str_param(p, "anchor").unwrap_or("top") { - "middle" | "center" | "ctr" => Anchor::Middle, - "bottom" | "b" => Anchor::Bottom, - "top" | "t" => Anchor::Top, - o => return Err(bad("table.cellAlign", format!("unknown anchor {o:?}; one of top, middle, bottom"))), + use deckcraft_model::text::{Align, Anchor}; + // Without `align`, the vertical anchor is the whole command (top when not given). + let align = match str_param(p, "align") { + None => None, + Some("left" | "l") => Some(Align::Left), + Some("center" | "centre" | "ctr") => Some(Align::Center), + Some("right" | "r") => Some(Align::Right), + Some("justify" | "just") => Some(Align::Justify), + Some(o) => return Err(bad("table.cellAlign", format!("unknown align {o:?}; one of left, center, right, justify"))), + }; + let anchor = match (str_param(p, "anchor"), align) { + (None, Some(_)) => None, + (a, _) => Some(match a.unwrap_or("top") { + "middle" | "center" | "ctr" => Anchor::Middle, + "bottom" | "b" => Anchor::Bottom, + "top" | "t" => Anchor::Top, + o => return Err(bad("table.cellAlign", format!("unknown anchor {o:?}; one of top, middle, bottom"))), + }), }; let targets = target_cells(s, p)?; with_table(s, p, "table.cellAlign", move |t| { for (r, c) in targets(t) { if let Some(cell) = t.cell_mut(r, c) { - cell.anchor = Some(a); + if anchor.is_some() { + cell.anchor = anchor; + } + if let Some(a) = align { + for para in &mut cell.text.paragraphs { + para.props.align = Some(a); + } + } } } Ok(()) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4724,3 +4724,38 @@ fn slide_hide_and_layout_reject_invalid_index() { } assert!(s.doc().unwrap().doc.slides.iter().all(|x| !x.hidden)); } + +#[test] +fn table_cell_align_sets_the_horizontal_alignment() { + use deckcraft_model::text::{Align, Anchor}; + let mut s = session(); + let id = table_at(&mut s, &[100.0, 100.0], json!([["a", "b"], ["c", "d"]])); + s.execute("table.cellAlign", &json!({"id": id.0, "align": "center", "cells": [[0, 0], [1, 0]]})).unwrap(); + s.execute("table.cellAlign", &json!({"id": id.0, "align": "right", "anchor": "bottom", "cells": [[0, 1]]})).unwrap(); + let t = table_of(&s, id); + let al = |r, c| t.cell(r, c).unwrap().text.paragraphs.iter().map(|p| p.props.align).collect::>(); + assert_eq!(al(0, 0), vec![Some(Align::Center)]); + assert_eq!(al(1, 0), vec![Some(Align::Center)]); + assert_eq!(al(0, 1), vec![Some(Align::Right)]); + assert_eq!(al(1, 1), vec![None], "other cells are untouched"); + assert_eq!(t.cell(0, 1).unwrap().anchor, Some(Anchor::Bottom)); + assert_eq!(t.cell(0, 0).unwrap().anchor, None, "align alone leaves the vertical anchor"); + assert!(s.execute("table.cellAlign", &json!({"id": id.0, "align": "diagonal"})).is_err()); +} + +#[test] +fn style_commands_given_nothing_to_set_are_refused() { + let mut s = session(); + s.execute("insert.textBox", &json!({"text": "Hi", "rect": [50.0, 50.0, 200.0, 100.0]})).unwrap(); + for cmd in ["shape.line", "shape.effects", "picture.adjust", "format.paragraph", "format.margins"] { + let rev = s.doc().unwrap().revision; + let e = s.execute(cmd, &json!({})).unwrap_err().to_string(); + assert!(e.contains("given nothing to set") && e.contains(cmd), "{cmd}: {e}"); + assert_eq!(s.doc().unwrap().revision, rev, "{cmd} changed the deck"); + } + let e = s.execute("shape.line", &json!({})).unwrap_err().to_string(); + assert!(e.contains("pass color, none, width, dash"), "{e}"); + // A key, even one that changes nothing visible, is something to set. + s.execute("shape.line", &json!({"width": 2})).unwrap(); + s.execute("format.margins", &json!({"left": 5})).unwrap(); +} diff --git a/crates/text/src/lib.rs b/crates/text/src/lib.rs --- a/crates/text/src/lib.rs +++ b/crates/text/src/lib.rs @@ -1871,6 +1871,9 @@ fn layout_scaled(ctx: &Ctx, shape: &Shape, body: &TextBody, bp: &BodyProps, opts let mut out = TextLayout { inner, font_scale: scale, line_reduction: reduce, rotation, ..Default::default() }; let mut y = 0.0; let mut numbering: Vec = vec![0; 9]; + // The `start_at` each level's current run of numbers began with: a paragraph that changes it + // starts a new run (PowerPoint's "Start at"). + let mut run_start: Vec = vec![0; 9]; let mut all_lines: Vec<(LineInfo, LinePlace, f64, Option<(String, f64, f64)>)> = vec![]; let mut prev_after = 0.0; // How far the last line's full pitch reaches past the text (exact spacing; see below). @@ -1881,6 +1884,12 @@ fn layout_scaled(ctx: &Ctx, shape: &Shape, body: &TextBody, bp: &BodyProps, opts let lvl = para.level.min(8) as usize; let pp = resolve::para(ctx, shape, para); let number = if matches!(pp.bullet, Some(Bullet::AutoNum { .. })) && !para.is_empty() { + if let (Some(Bullet::AutoNum { start_at, .. }), Some(rs), Some(n)) = (&pp.bullet, run_start.get_mut(lvl), numbering.get_mut(lvl)) + && *rs != *start_at + { + *rs = *start_at; + *n = 0; + } if let Some(n) = numbering.get_mut(lvl) { *n += 1; } @@ -1888,10 +1897,11 @@ fn layout_scaled(ctx: &Ctx, shape: &Shape, body: &TextBody, bp: &BodyProps, opts *deeper = 0; } numbering.get(lvl).copied() - } else if matches!(pp.bullet, Some(Bullet::AutoNum { .. })) { + } else if let Some(Bullet::AutoNum { start_at, .. }) = &pp.bullet { // Preview the next number without consuming it: empty paragraphs do not advance - // numbering in the saved document or affect later populated paragraphs. - numbering.get(lvl).map(|n| n.saturating_add(1)) + // numbering in the saved document or affect later populated paragraphs. A paragraph + // with its own start would begin a new run, so it previews that run's first number. + if run_start.get(lvl).is_some_and(|rs| *rs != *start_at) { Some(1) } else { numbering.get(lvl).map(|n| n.saturating_add(1)) } } else { if !para.is_empty() { for n in numbering.iter_mut().skip(lvl) { diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -1840,3 +1840,28 @@ fn an_empty_paragraph_keeps_its_paragraph_mark_height() { let h = l.lines[1].bottom - l.lines[1].top; assert!((h - h10).abs() < 0.5, "a 10pt empty paragraph should give {h10}, got {h}"); } + +#[test] +fn a_paragraph_with_its_own_start_at_restarts_the_count_there() { + // PowerPoint's "Start at" on a paragraph begins a new run of numbers at that value, so setting + // start k on paragraph k reads 1, 2, 3, not 1, 3, 5. + // The bullets' glyph ids next to those of the expected labels, as pairs (drawn, expected). + let check = |starts: &[u32], expect: &[u32]| { + let (p, mut sh) = setup("a\nb\nc\nd", 600.0, 300.0); + for (para, st) in sh.text.as_mut().unwrap().paragraphs.iter_mut().zip(starts) { + para.props.bullet = Some(Bullet::AutoNum { scheme: "arabicPeriod".into(), start_at: *st }); + } + let l = lay(&p, &sh, 600.0, 300.0); + let bullets: Vec<_> = l.runs.iter().filter(|r| r.cells.is_empty()).collect(); + assert_eq!(bullets.len(), expect.len()); + for (b, n) in bullets.iter().zip(expect) { + let want = measure_str(&b.face, b.size, &format!("{n}.")).1.iter().map(|g| g.0).collect::>(); + assert_eq!(b.glyphs.iter().map(|g| g.0).collect::>(), want, "expected {n}. for starts {starts:?}"); + } + }; + check(&[1, 2, 3, 4], &[1, 2, 3, 4]); + // The same start on every paragraph still counts on from it (a whole box numbered at once). + check(&[3, 3, 3, 3], &[3, 4, 5, 6]); + // A later paragraph with a new start begins a fresh run. + check(&[1, 1, 7, 7], &[1, 2, 7, 8]); +}