Review #998: one strict anchor parser; build fix for path.appendAnchor
The PR added `checked_anchor_from_json` next to the lenient `anchor_from_json` and dropped path.rs's import of the latter, which `path.appendAnchor` still called, so the engine didn't compile. `anchor_from_json` is now the strict parser itself (handles through `finite_numbers`), used by path.create, path.setAnchors and path.appendAnchor alike: the Pen's appended anchor with a malformed handle fails too instead of losing the handle. Tests: a non-boolean `closed` and a malformed appended handle change nothing. rustfmt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
743fe2521e
commit
57050b0987
|
|
@ -385,48 +385,22 @@ fn polar_grid(s: &mut Session, p: &Value) -> Result<Value> {
|
|||
grid_group(s, "Polar Grid", shapes::polar_grid(r, c, rad))
|
||||
}
|
||||
|
||||
pub(crate) fn anchor_from_json(v: &Value) -> Option<Anchor> {
|
||||
let p = Point::new(v.get("x")?.as_f64()?, v.get("y")?.as_f64()?);
|
||||
let h_in = point_param(v, "in").unwrap_or(p);
|
||||
let h_out = point_param(v, "out").unwrap_or(p);
|
||||
let mut a = Anchor::with_handles(p, h_in, h_out);
|
||||
if v.get("smooth").and_then(Value::as_bool) == Some(true) {
|
||||
a.kind = AnchorKind::Smooth;
|
||||
}
|
||||
Some(a)
|
||||
}
|
||||
|
||||
/// Strictly parse command-provided anchors: dropping invalid entries would change geometry.
|
||||
pub(crate) fn checked_anchor_from_json(v: &Value, cmd: &str) -> Result<Anchor> {
|
||||
let coord = |key: &str| {
|
||||
v.get(key)
|
||||
.and_then(Value::as_f64)
|
||||
.filter(|n| n.is_finite())
|
||||
.ok_or_else(|| bad(cmd, format!("anchor `{key}` must be a finite number")))
|
||||
};
|
||||
/// An anchor `{x, y, in?, out?, smooth?}` given to `cmd` (handles `[x, y]`, at the anchor when
|
||||
/// absent or null): anything malformed fails rather than being left out or replaced by a default,
|
||||
/// which would change the path's shape.
|
||||
pub(crate) fn anchor_from_json(v: &Value, cmd: &str) -> Result<Anchor> {
|
||||
let coord =
|
||||
|key: &str| v.get(key).and_then(Value::as_f64).filter(|n| n.is_finite()).ok_or_else(|| bad(cmd, format!("anchor `{key}` must be a number")));
|
||||
let p = Point::new(coord("x")?, coord("y")?);
|
||||
let handle = |key: &str| -> Result<Point> {
|
||||
match v.get(key) {
|
||||
None | Some(Value::Null) => Ok(p),
|
||||
Some(value) => {
|
||||
let Some([x, y]) = value.as_array().map(Vec::as_slice) else {
|
||||
return Err(bad(cmd, format!("anchor `{key}` must be [x, y]")));
|
||||
};
|
||||
let (Some(x), Some(y)) = (x.as_f64(), y.as_f64()) else {
|
||||
return Err(bad(cmd, format!("anchor `{key}` must contain two numbers")));
|
||||
};
|
||||
if !x.is_finite() || !y.is_finite() {
|
||||
return Err(bad(cmd, format!("anchor `{key}` must contain finite numbers")));
|
||||
}
|
||||
Ok(Point::new(x, y))
|
||||
}
|
||||
}
|
||||
let handle = |key: &str| match v.get(key).filter(|h| !h.is_null()) {
|
||||
None => Ok(p),
|
||||
Some(h) => finite_numbers(h).map(|[x, y]| Point::new(x, y)).ok_or_else(|| bad(cmd, format!("anchor `{key}` must be [x, y]"))),
|
||||
};
|
||||
let mut anchor = Anchor::with_handles(p, handle("in")?, handle("out")?);
|
||||
match v.get("smooth") {
|
||||
None | Some(Value::Null) | Some(Value::Bool(false)) => {}
|
||||
match v.get("smooth").filter(|b| !b.is_null()) {
|
||||
None | Some(Value::Bool(false)) => {}
|
||||
Some(Value::Bool(true)) => anchor.kind = AnchorKind::Smooth,
|
||||
Some(_) => return Err(bad(cmd, "anchor `smooth` must be a boolean")),
|
||||
Some(_) => return Err(bad(cmd, "anchor `smooth` must be true or false")),
|
||||
}
|
||||
Ok(anchor)
|
||||
}
|
||||
|
|
@ -441,7 +415,7 @@ fn path_create(s: &mut Session, p: &Value) -> Result<Value> {
|
|||
.and_then(Value::as_array)
|
||||
.ok_or_else(|| bad("path.create", "missing anchors"))?
|
||||
.iter()
|
||||
.map(|a| checked_anchor_from_json(a, "path.create"))
|
||||
.map(|a| anchor_from_json(a, "path.create"))
|
||||
.collect::<Result<_>>()?;
|
||||
if anchors.is_empty() {
|
||||
return Err(bad("path.create", "need at least one anchor"));
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use serde_json::{Value, json};
|
|||
use vectorcraft_doc::{NodeId, NodeKind};
|
||||
use vectorcraft_geom::{AnchorKind, PathData, Point, SubPath, Vec2};
|
||||
|
||||
use super::create::checked_anchor_from_json;
|
||||
use super::create::anchor_from_json;
|
||||
use super::*;
|
||||
use crate::EngineError;
|
||||
|
||||
|
|
@ -129,7 +129,7 @@ fn path_mut(d: &mut vectorcraft_doc::Document, id: NodeId) -> Result<&mut PathDa
|
|||
|
||||
fn append_anchor(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let id = id_param(p, "id").ok_or_else(|| bad("path.appendAnchor", "missing id"))?;
|
||||
let a = anchor_from_json(p).ok_or_else(|| bad("path.appendAnchor", "missing x/y"))?;
|
||||
let a = anchor_from_json(p, "path.appendAnchor")?;
|
||||
s.edit("Pen", |d, sel| {
|
||||
let path = path_mut(d, id)?;
|
||||
match path.subpaths.last_mut() {
|
||||
|
|
@ -222,7 +222,7 @@ fn set_anchors(s: &mut Session, p: &Value) -> Result<Value> {
|
|||
.and_then(Value::as_array)
|
||||
.ok_or_else(|| bad("path.setAnchors", "each subpath needs an anchors array"))?
|
||||
.iter()
|
||||
.map(|a| checked_anchor_from_json(a, "path.setAnchors"))
|
||||
.map(|a| anchor_from_json(a, "path.setAnchors"))
|
||||
.collect::<Result<Vec<_>>>()?;
|
||||
let closed = match sp.get("closed") {
|
||||
None | Some(Value::Null) => false,
|
||||
|
|
|
|||
|
|
@ -1016,6 +1016,9 @@ fn path_commands_reject_bad_anchors_before_modifying_geometry() {
|
|||
assert_eq!(s.doc().unwrap().history.undo.len(), history);
|
||||
}
|
||||
assert!(s.execute("path.setAnchors", &json!({"id": a.0, "subpaths": [{"anchors": []}, {"closed": true}]})).is_err());
|
||||
assert!(s.execute("path.setAnchors", &json!({"id": a.0, "subpaths": [{"anchors": [valid.clone()], "closed": "yes"}]})).is_err());
|
||||
// The Pen's append reads its anchor the same way.
|
||||
assert!(s.execute("path.appendAnchor", &json!({"id": a.0, "x": 5, "y": 5, "out": [1]})).is_err());
|
||||
assert_eq!(s.doc().unwrap().history.undo.len(), history);
|
||||
s.execute("path.setAnchors", &json!({"id": a.0, "subpaths": [{"anchors": [{"x": 0, "y": 0}, {"x": 30, "y": 40}], "closed": false}]})).unwrap();
|
||||
assert_eq!(s.doc().unwrap().doc.node(a).unwrap().path_data().unwrap().anchor_count(), 2);
|
||||
|
|
|
|||
Loading…
Reference in New Issue