fix(cmd): reject unknown spread values instead of falling back to page 1 (part of #651)

This commit is contained in:
Marco Arnulfo committed 2026-10-13 00:38:54 +02:00
1 parent 64f0ca8fa6
commit 3d59dd5d97
10 files changed
+65 -43

No files matched your search

+4 -4
View File
@@ -335,7 +335,7 @@ fn file_place(s: &mut Session, p: &Value) -> Result<Value> {
let st = s.active()?;
st.selection.items.iter().copied().find(|i| st.doc.item(*i).is_some_and(|it| matches!(it.content, Content::Unassigned | Content::Graphic(_))))
});
let spread = super::spread_param(p, "spread");
let spread = super::spread_param(p, "file.place")?;
let lid = s.doc()?.active_layer;
let px = p.get("x").and_then(Value::as_f64);
let py = p.get("y").and_then(Value::as_f64);
@@ -458,7 +458,7 @@ fn place_load(s: &mut Session, p: &Value) -> Result<Value> {
fn place_drop(s: &mut Session, p: &Value) -> Result<Value> {
let (aid, (nw, nh)) = s.loaded.ok_or_else(|| bad("place.drop", "nothing loaded in the place cursor"))?;
let sr = super::spread_param(p, "spread");
let sr = super::spread_param(p, "place.drop")?;
let lid = s.doc()?.active_layer;
let frame = super::id_param(p, "frame");
let rect = super::rect_param(p, "rect");
@@ -550,7 +550,7 @@ fn place_layout_page(s: &mut Session, p: &Value, name: &str, bytes: &[u8]) -> Re
if ids.is_empty() {
return Err(bad(ID, format!("page {page} has no objects")));
}
let to = super::spread_param(p, "spread");
let to = super::spread_param(p, ID)?;
let off = match (p.get("x").and_then(Value::as_f64), p.get("y").and_then(Value::as_f64)) {
(Some(x), Some(y)) => designcraft_geom::Vec2::new(x - pr.x0, y - pr.y0),
_ => designcraft_geom::Vec2::new(-pr.x0, -pr.y0),
@@ -586,7 +586,7 @@ pub(crate) fn snippet_place(s: &mut Session, p: &Value) -> Result<Value> {
if ids.is_empty() {
return Err(bad("snippet.place", "the snippet is empty"));
}
let sr = super::spread_param(p, "spread");
let sr = super::spread_param(p, "snippet.place")?;
let bounds = ids.iter().filter_map(|i| src.item(*i).map(|it| it.bounds())).reduce(|a, b| a.union(b)).unwrap_or(Rect::ZERO);
let off = match (p.get("x").and_then(Value::as_f64), p.get("y").and_then(Value::as_f64)) {
(Some(x), Some(y)) => designcraft_geom::Vec2::new(x - bounds.x0, y - bounds.y0),
+5 -22
View File
@@ -28,7 +28,7 @@ pub fn specs() -> Vec<CommandSpec> {
cmd!("guide.move", "Move Guide", [], None, "{spread?, page, index, position}", has_doc, move_guide),
cmd!("guide.delete", "Delete Guide", [], None, "{spread?, page, index}", has_doc, delete),
cmd!("guide.deleteAll", "Delete All Guides on Spread", ["View", "Grids & Guides"], None, "{spread?}", has_doc, |s, p| {
let r = guide_spread(p, "guide.deleteAll")?;
let r = super::spread_param(p, "guide.deleteAll")?;
s.edit(|d, _| {
let sp = d.spread_mut(r).ok_or_else(|| bad("guide.deleteAll", "no such spread"))?;
let mut n = 0;
@@ -41,7 +41,7 @@ pub fn specs() -> Vec<CommandSpec> {
}),
cmd!(query "guide.list", "Guides", [], None, "{spread?} → [{page, index, orientation, position, spread}]", has_doc, |s, p| {
let d = &s.doc()?.doc;
let sp = d.spread(guide_spread(p, "guide.list")?).ok_or_else(|| bad("guide.list", "no such spread"))?;
let sp = d.spread(super::spread_param(p, "guide.list")?).ok_or_else(|| bad("guide.list", "no such spread"))?;
let mut out = Vec::new();
for (pi, pg) in sp.pages.iter().enumerate() {
for (i, g) in pg.guides.iter().enumerate() {
@@ -71,23 +71,6 @@ fn optional_usize(p: &Value, key: &str, command: &str) -> Result<Option<usize>>
p.get(key).map(|value| checked_usize(value, key, command)).transpose()
}
/// Numeric spread shorthand or the serialized `SpreadRef` form, without lossy/defaulting casts.
fn guide_spread(p: &Value, command: &str) -> Result<SpreadRef> {
let Some(value) = p.get("spread") else { return Ok(SpreadRef::Doc(0)) };
if value.is_number() {
return Ok(SpreadRef::Doc(checked_usize(value, "spread", command)?));
}
let object = value.as_object().ok_or_else(|| bad(command, "`spread` must be a non-negative integer or a spread reference"))?;
let kind = object.get("kind").and_then(Value::as_str).ok_or_else(|| bad(command, "`spread.kind` must be `doc` or `parent`"))?;
let index = object.get("index").ok_or_else(|| bad(command, "missing `spread.index`"))?;
let index = checked_usize(index, "spread.index", command)?;
match kind {
"doc" => Ok(SpreadRef::Doc(index)),
"parent" => Ok(SpreadRef::Parent(index)),
_ => Err(bad(command, "`spread.kind` must be `doc` or `parent`")),
}
}
fn finite_number(p: &Value, key: &str, command: &str) -> Result<Option<f64>> {
let Some(value) = p.get(key) else { return Ok(None) };
let number = value.as_f64().ok_or_else(|| bad(command, format!("`{key}` must be a finite number")))?;
@@ -113,7 +96,7 @@ fn page_at(d: &Document, r: SpreadRef, x: f64) -> usize {
fn add(s: &mut Session, p: &Value) -> Result<Value> {
let o = orientation(p)?;
let pos = finite_number(p, "position", "guide.add")?.ok_or_else(|| bad("guide.add", "missing position"))?;
let r = guide_spread(p, "guide.add")?;
let r = super::spread_param(p, "guide.add")?;
let spread_guide = p.get("spreadGuide").and_then(Value::as_bool).unwrap_or(false);
let d = &s.doc()?.doc;
let pi = match optional_usize(p, "page", "guide.add")? {
@@ -139,7 +122,7 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
fn guide_ref(p: &Value, cmd: &str) -> Result<(SpreadRef, usize, usize)> {
let pi = optional_usize(p, "page", cmd)?.ok_or_else(|| bad(cmd, "missing page"))?;
let i = optional_usize(p, "index", cmd)?.ok_or_else(|| bad(cmd, "missing index"))?;
Ok((guide_spread(p, cmd)?, pi, i))
Ok((super::spread_param(p, cmd)?, pi, i))
}
/// Commands must use the same guide/layer protection as pointer editing.
@@ -237,7 +220,7 @@ fn create_guides(s: &mut Session, p: &Value) -> Result<Value> {
let cg = guide_gutter(p, "columnGutter")?;
let to_page = str_param(p, "fitTo") == Some("page");
let remove = p.get("removeExisting").and_then(Value::as_bool).unwrap_or(false);
let r = guide_spread(p, "layout.createGuides")?;
let r = super::spread_param(p, "layout.createGuides")?;
let only = optional_usize(p, "page", "layout.createGuides")?;
let page_count = {
let sp = s.doc()?.doc.spread(r).ok_or_else(|| bad("layout.createGuides", "no such spread"))?;
+1 -1
View File
@@ -28,7 +28,7 @@ pub fn specs() -> Vec<CommandSpec> {
|s, p| {
let parent = StoryId(p.get("story").and_then(Value::as_u64).ok_or_else(|| bad("story.placeAndLink", "missing story"))?);
let rect = rect_param(p, "rect").ok_or_else(|| bad("story.placeAndLink", "missing rect"))?;
let sr = spread_param(p, "spread");
let sr = spread_param(p, "story.placeAndLink")?;
let layer = s.doc()?.active_layer;
s.edit(|d, sel| {
if d.story(parent).is_none() {
+1 -1
View File
@@ -168,7 +168,7 @@ pub fn specs() -> Vec<CommandSpec> {
}
),
cmd!("guide.liquid", "Convert to Liquid Guide", [], None, "{spread?, page, index, on?: bool (default true)}", has_doc, |s, p| {
let sr = super::spread_param(p, "spread");
let sr = super::spread_param(p, "guide.liquid")?;
let pi = p.get("page").and_then(Value::as_u64).unwrap_or(0) as usize;
let gi = p.get("index").and_then(Value::as_u64).ok_or_else(|| bad("guide.liquid", "`index` required"))? as usize;
let on = p.get("on").and_then(Value::as_bool).unwrap_or(true);
+1 -1
View File
@@ -70,7 +70,7 @@ fn insert(s: &mut Session, p: &Value) -> Result<Value> {
});
}
let text = s.doc()?.selection.text.filter(|t| t.cell.is_none());
let spread = super::spread_param(p, "spread");
let spread = super::spread_param(p, "math.insert")?;
let (px, py) = (p.get("x").and_then(Value::as_f64), p.get("y").and_then(Value::as_f64));
s.edit(|d, sel| {
if let Some(t) = text {
+1 -1
View File
@@ -20,7 +20,7 @@ pub(crate) fn place_media(s: &mut Session, p: &Value, name: String, mime: &str,
let st = s.active()?;
st.selection.items.iter().copied().find(|i| st.doc.item(*i).is_some_and(|it| matches!(it.content, Content::Unassigned | Content::Graphic(_))))
});
let spread = super::spread_param(p, "spread");
let spread = super::spread_param(p, "file.place")?;
let lid = s.doc()?.active_layer;
let (px, py) = (p.get("x").and_then(Value::as_f64), p.get("y").and_then(Value::as_f64));
let mime = mime.to_string();
+17 -6
View File
@@ -254,12 +254,23 @@ pub(crate) fn rect_param(p: &Value, key: &str) -> Option<Rect> {
let f = |i: usize| a.get(i).and_then(Value::as_f64);
Some(Rect::new(f(0)?, f(1)?, f(2)?, f(3)?))
}
/// `{"kind":"doc","index":0}`, `0` or absent (= spread 0).
pub(crate) fn spread_param(p: &Value, key: &str) -> SpreadRef {
match p.get(key) {
Some(Value::Number(n)) => SpreadRef::Doc(n.as_u64().unwrap_or(0) as usize),
Some(v) => serde_json::from_value(v.clone()).unwrap_or(SpreadRef::Doc(0)),
None => SpreadRef::Doc(0),
/// The `spread` param: a body spread index (`0`), a spread reference (`{"kind":"doc"|"parent","index":0}`)
/// or absent (= spread 0). Anything else is an error: read as spread 0, a script aiming at a parent
/// spread got its items on the first page without a word.
pub(crate) fn spread_param(p: &Value, command: &str) -> Result<SpreadRef> {
let index = |v: &Value, key: &str| {
v.as_u64().and_then(|n| usize::try_from(n).ok()).ok_or_else(|| bad(command, format!("`{key}` must be a non-negative integer")))
};
let Some(value) = p.get("spread") else { return Ok(SpreadRef::Doc(0)) };
if value.is_number() {
return Ok(SpreadRef::Doc(index(value, "spread")?));
}
let object = value.as_object().ok_or_else(|| bad(command, "`spread` must be a non-negative integer or {kind: doc|parent, index}"))?;
let i = index(object.get("index").ok_or_else(|| bad(command, "missing `spread.index`"))?, "spread.index")?;
match object.get("kind").and_then(Value::as_str) {
Some("doc") => Ok(SpreadRef::Doc(i)),
Some("parent") => Ok(SpreadRef::Parent(i)),
_ => Err(bad(command, "`spread.kind` must be `doc` or `parent`")),
}
}
+31 -5
View File
@@ -32,7 +32,7 @@ pub fn specs() -> Vec<CommandSpec> {
"Create Frame",
[],
None,
"{spread?, rect: [x0,y0,x1,y1] (spread coords), shape?: rectangle|ellipse|polygon, content?: graphic|text|unassigned, sides?: 6, text?: string, caret?: bool, vertical?: bool (text: a new vertical story)}",
"{spread?: index | {kind: doc|parent, index} (a parent spread: index into `parents` of document.inspect), rect: [x0,y0,x1,y1] (spread coords), shape?: rectangle|ellipse|polygon, content?: graphic|text|unassigned, sides?: 6, text?: string, caret?: bool, vertical?: bool (text: a new vertical story)}",
has_doc,
frame_create
),
@@ -828,7 +828,7 @@ fn frame_create(s: &mut Session, p: &Value) -> Result<Value> {
let rect = rect_param(p, "rect").ok_or_else(|| bad("frame.create", "missing rect"))?;
let shape = str_param(p, "shape").unwrap_or("rectangle");
let content = str_param(p, "content").unwrap_or("graphic");
let sr = spread_param(p, "spread");
let sr = spread_param(p, "frame.create")?;
let lid = s.doc()?.active_layer;
let text = super::text_param(p, "text");
let caret = bool_or(p, "caret", content == "text");
@@ -891,7 +891,7 @@ pub(crate) fn polygon_in(r: Rect, sides: u32, inset: f64) -> designcraft_geom::P
fn line_create(s: &mut Session, p: &Value) -> Result<Value> {
let a = point_param(p, "a").ok_or_else(|| bad("line.create", "missing a"))?;
let b = point_param(p, "b").ok_or_else(|| bad("line.create", "missing b"))?;
let sr = spread_param(p, "spread");
let sr = spread_param(p, "line.create")?;
let lid = s.doc()?.active_layer;
s.edit(|d, sel| {
let id = ItemId(d.alloc());
@@ -1592,7 +1592,7 @@ fn gap_move(s: &mut Session, p: &Value) -> Result<Value> {
.and_then(Value::as_array)
.and_then(|a| Some(Point::new(a.first()?.as_f64()?, a.get(1)?.as_f64()?)))
.ok_or_else(|| bad(ID, "missing at"))?;
let sr = spread_param(p, "spread");
let sr = spread_param(p, ID)?;
let (vertical, a, z, before, after) = find_gap(&s.doc()?.doc, sr, at).ok_or_else(|| bad(ID, "no gap there"))?;
// `delta` along the gap's axis, or a drag (dx, dy) of which the axis part counts.
let delta =
@@ -1919,7 +1919,7 @@ fn path_create(s: &mut Session, p: &Value) -> Result<Value> {
return Err(bad("path.create", "a path needs at least two anchors"));
}
let closed = bool_or(p, "closed", false);
let sr = spread_param(p, "spread");
let sr = spread_param(p, "path.create")?;
let lid = s.doc()?.active_layer;
s.edit(|d, sel| {
let id = ItemId(d.alloc());
@@ -2813,3 +2813,29 @@ mod swatch_name_tests {
}
}
}
#[cfg(test)]
mod spread_target_tests {
use super::*;
#[test]
fn creating_on_a_parent_spread_and_rejecting_unknown_spreads() {
let mut s = Session::new();
s.execute("file.new", &json!({"pages": 1, "facingPages": false})).unwrap();
let parent = json!({"kind": "parent", "index": 0});
let r = s.execute("frame.create", &json!({"spread": parent, "rect": [0, 0, 100, 100]})).unwrap();
let id = ItemId(r["id"].as_u64().unwrap());
assert_eq!(s.doc().unwrap().doc.find(id).unwrap().spread, SpreadRef::Parent(0));
s.execute("line.create", &json!({"spread": parent, "a": [0, 0], "b": [50, 50]})).unwrap();
assert_eq!(s.doc().unwrap().doc.parents[0].items.len(), 2);
// Each of these used to land on the first body page.
for bad in
[json!("A-Parent"), json!("0"), json!(-1), json!(1.5), json!({"kind": "master", "index": 0}), json!({"kind": "parent"}), json!([0])]
{
assert!(s.execute("frame.create", &json!({"spread": bad, "rect": [0, 0, 100, 100]})).is_err(), "frame.create spread {bad}");
assert!(s.execute("line.create", &json!({"spread": bad, "a": [0, 0], "b": [50, 50]})).is_err(), "line.create spread {bad}");
}
assert!(s.doc().unwrap().doc.spreads[0].items.is_empty());
}
}
+1 -1
View File
@@ -58,7 +58,7 @@ fn qr_code(s: &mut Session, p: &Value) -> Result<Value> {
let text = encode(p).map_err(|e| bad(ID, e))?;
let color = str_param(p, "color").unwrap_or(designcraft_color::swatch::BLACK).to_string();
let rect = rect_param(p, "rect");
let sr = spread_param(p, "spread");
let sr = spread_param(p, ID)?;
let st = s.doc()?;
let lid = st.active_layer;
let sel = st.selection.items.clone();
+3 -1
View File
@@ -58,7 +58,9 @@ so inspect the document before retrying. The next request re-establishes the con
Points (1/72 in), y down, in **spread** space: on a single-page spread the page's top-left is (0, 0); on a facing
spread the right page starts at x = page width. `inspect_document` lists page bounds per spread. Page indices in
`render_page` / `export_png` are 0-based. `pointer` events in connected mode may use `"space": "screen"` (egui points,
`render_page` / `export_png` are 0-based. Commands that create items take `spread`: a body spread index, or
`{"kind": "parent", "index": 0}` for a parent spread (its index in `parents` of `inspect_document`); any other
value is an error. `pointer` events in connected mode may use `"space": "screen"` (egui points,
see `ui_inspect` → `canvasRect`).
## Text offsets