diff --git a/crates/engine/src/cmd/page.rs b/crates/engine/src/cmd/page.rs index 8fccd607..03dbef7a 100644 --- a/crates/engine/src/cmd/page.rs +++ b/crates/engine/src/cmd/page.rs @@ -23,6 +23,15 @@ fn settle(s: &mut Session, cmd: &str) -> crate::Result<()> { Ok(()) } +/// The page a command targets: the explicit `id` (which must be a page) or the open page. +fn page_target(s: &Session, cmd: &str, p: &Value) -> crate::Result { + let Some(target) = id(p, "id") else { return s.page() }; + if !s.doc()?.pages.contains(&target) { + return Err(bad(cmd, format!("{target} is not a page"))); + } + Ok(target) +} + pub fn specs() -> Vec { vec![ spec!("page.add", "Add page", "{name?} (a name starting with a hyphen adds a divider)", true, |s, p| { @@ -71,13 +80,13 @@ pub fn specs() -> Vec { }), spec!("page.rename", "Rename page", "{id?, name} (an empty page named with a leading hyphen becomes a divider)", true, |s, p| { let name = req_string("page.rename", p, "name")?; - let id = id(p, "id").unwrap_or(s.page()?); + let id = page_target(s, "page.rename", p)?; s.doc_mut()?.get_mut(id)?.name = name; settle(s, "page.rename")?; ok() }), spec!("page.delete", "Delete page", "{id?}", true, |s, p| { - let id = id(p, "id").unwrap_or(s.page()?); + let id = page_target(s, "page.delete", p)?; s.doc_mut()?.remove(id)?; settle(s, "page.delete")?; ok() @@ -114,7 +123,7 @@ pub fn specs() -> Vec { }), spec!("page.setBackground", "Page background", "{id?, color}", true, |s, p| { let c = color(p, "color").ok_or_else(|| bad("page.setBackground", "`color` required"))?; - let id = id(p, "id").unwrap_or(s.page()?); + let id = page_target(s, "page.setBackground", p)?; if let NodeKind::Page { background, .. } = &mut s.doc_mut()?.get_mut(id)?.kind { *background = c; } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 42423dbc..f39cd28d 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1528,6 +1528,8 @@ mod open_issues_tests; #[cfg(test)] mod outline_options_tests; #[cfg(test)] +mod page_target_tests; +#[cfg(test)] mod paint_reorder_tests; #[cfg(test)] mod paint_tests; diff --git a/crates/engine/src/page_target_tests.rs b/crates/engine/src/page_target_tests.rs new file mode 100644 index 00000000..fd88685d --- /dev/null +++ b/crates/engine/src/page_target_tests.rs @@ -0,0 +1,63 @@ +//! Page commands act on pages: a layer id (a rectangle, a frame) is refused before anything about +//! the layer changes, as `page.select` already does. + +use figcraft_doc::{NodeId, NodeKind}; +use serde_json::{Value, json}; + +use crate::Session; + +fn ex(s: &mut Session, cmd: &str, p: Value) -> Value { + s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}")) +} + +fn id_of(v: &Value) -> NodeId { + NodeId(v["id"].as_u64().unwrap()) +} + +fn name(s: &Session, id: NodeId) -> String { + s.doc().unwrap().get(id).unwrap().name.clone() +} + +#[test] +fn page_commands_reject_a_rectangle() { + let mut s = Session::new(); + let page = s.page().unwrap(); + let r = id_of(&ex(&mut s, "shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10}))); + let before = name(&s, r); + assert!(s.execute("page.select", &json!({"id": r})).is_err()); + assert!(s.execute("page.rename", &json!({"id": r, "name": "Not a page"})).is_err(), "rename refuses a layer"); + assert_eq!(name(&s, r), before, "the layer keeps its name"); + assert!(s.execute("page.setBackground", &json!({"id": r, "color": "#123456"})).is_err(), "background refuses a layer"); + assert!(s.execute("page.delete", &json!({"id": r})).is_err(), "delete refuses a layer"); + assert!(s.doc().unwrap().contains(r), "the layer is still there"); + assert!(s.doc().unwrap().contains(page)); +} + +#[test] +fn page_delete_keeps_a_frame_and_its_child() { + let mut s = Session::new(); + let f = id_of(&ex(&mut s, "frame.create", json!({"x": 0, "y": 0, "width": 100, "height": 100}))); + let c = id_of(&ex(&mut s, "shape.rectangle", json!({"x": 5, "y": 5, "width": 10, "height": 10, "parent": f}))); + assert_eq!(s.doc().unwrap().get(c).unwrap().parent, Some(f)); + assert!(s.execute("page.delete", &json!({"id": f})).is_err()); + assert!(s.doc().unwrap().contains(f) && s.doc().unwrap().contains(c), "the frame and its child remain"); + assert!(s.execute("page.rename", &json!({"id": f, "name": "Not a page"})).is_err()); + assert_ne!(name(&s, f), "Not a page"); +} + +#[test] +fn page_commands_still_work_on_pages() { + let mut s = Session::new(); + let first = s.page().unwrap(); + let second = id_of(&ex(&mut s, "page.add", json!({}))); + ex(&mut s, "page.rename", json!({"id": second, "name": "Flows"})); + assert_eq!(name(&s, second), "Flows"); + ex(&mut s, "page.setBackground", json!({"id": second, "color": "#123456"})); + match s.doc().unwrap().get(second).unwrap().kind { + NodeKind::Page { background, .. } => assert_eq!(background.to_css_hex().to_uppercase(), "#123456"), + _ => panic!("not a page"), + } + ex(&mut s, "page.delete", json!({"id": second})); + assert!(!s.doc().unwrap().contains(second)); + assert!(s.execute("page.delete", &json!({"id": first})).is_err(), "the last ordinary page stays"); +}