Merge pull request #6715 from dotCooCoo/fix/halftone-param-types

Vector Halftone: a parameter of the wrong type is refused

crates/engine/src/cmd/halftone.rs

@@ -11,7 +11,7 @@ use vectorcraft_doc::Node;
 use vectorcraft_render::effects::addons::halftone::{DotShape, Halftone, art_outline, dot_nodes};
 
 use super::edit::selected_roots;
-use super::{CommandSpec, EngineError, Result, Session, bad, bool_or, cmd, color_value, f64_or, has_selection, str_param};
+use super::{CommandSpec, EngineError, Result, Session, bad, cmd, color_value, has_selection, number_or, switch_or, text_param};
 
 pub fn specs() -> Vec<CommandSpec> {
     vec![cmd!(
@@ -27,28 +27,28 @@ pub fn specs() -> Vec<CommandSpec> {
 
 fn halftone(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "object.vectorHalftone";
-    let shape = match str_param(p, "shape") {
+    let shape = match text_param(p, "shape", C)? {
         Some(v) => DotShape::parse(v).ok_or_else(|| bad(C, format!("unknown shape `{v}` (circle|ellipse|square|diamond|line)")))?,
         None => DotShape::Circle,
     };
-    let frequency = f64_or(p, "frequency", 20.0);
+    let frequency = number_or(p, "frequency", 20.0, C)?;
     if !(1.0..=300.0).contains(&frequency) {
         return Err(bad(C, "frequency must be between 1 and 300 lines per inch"));
     }
-    let angle = f64_or(p, "angle", 45.0);
+    let angle = number_or(p, "angle", 45.0, C)?;
     if !angle.is_finite() {
         return Err(bad(C, "invalid angle"));
     }
-    let mono = match str_param(p, "mode").unwrap_or("mono") {
+    let mono = match text_param(p, "mode", C)?.unwrap_or("mono") {
         "mono" => true,
         "cmyk" => false,
         other => return Err(bad(C, format!("unknown mode `{other}` (mono|cmyk)"))),
     };
     let color = match p.get("color") {
+        None | Some(Value::Null) => Color::BLACK,
         Some(v) => color_value(v).ok_or_else(|| bad(C, "invalid color"))?,
-        None => Color::BLACK,
     };
-    let (invert, clip, keep) = (bool_or(p, "invert", false), bool_or(p, "clip", true), bool_or(p, "keepOriginal", false));
+    let (invert, clip, keep) = (switch_or(p, "invert", false, C)?, switch_or(p, "clip", true, C)?, switch_or(p, "keepOriginal", false, C)?);
     let ht = Halftone { shape, frequency, angle, mono, color, from_art: false, invert, clip };
 
     let roots = selected_roots(s)?;

crates/engine/src/tests/halftone.rs

@@ -65,3 +65,29 @@ fn halftone_options() {
         assert!(s.execute("object.vectorHalftone", &bad).is_err(), "{bad}");
     }
 }
+
+#[test]
+fn halftone_refuses_a_param_of_the_wrong_type() {
+    for bad in [
+        json!({"frequency": "60"}),
+        json!({"angle": "30"}),
+        json!({"shape": 1}),
+        json!({"mode": true}),
+        json!({"invert": "true"}),
+        json!({"clip": "false"}),
+        json!({"keepOriginal": 1}),
+        json!({"color": 0}),
+    ] {
+        let mut s = session();
+        let id = red_rect(&mut s);
+        let before = s.doc().unwrap().doc.clone();
+        assert!(s.execute("object.vectorHalftone", &bad).is_err(), "{bad}");
+        assert_eq!(s.doc().unwrap().doc, before, "{bad} changed the document");
+        assert!(s.doc().unwrap().doc.node(id).is_some());
+    }
+    // Null stands for an omitted value.
+    let mut s = session();
+    red_rect(&mut s);
+    let p = json!({"frequency": null, "angle": null, "shape": null, "mode": null, "color": null, "invert": null, "clip": null, "keepOriginal": null});
+    assert!(s.execute("object.vectorHalftone", &p).is_ok());
+}