mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-10-28 11:47:21 -06:00
Review #985: one parser for exact number lists; null at is no position
- `finite_numbers::<N>` (cmd/mod.rs) reads an array of exactly N finite numbers, nothing dropped or padded; `rect` uses it, and the rect cut, Free Distort and anchor handles reuse it in the PRs stacked on this one. - `at` is parsed once (`at_param`) instead of point_param plus a second shape check that indexed the params; `at: null` means no position, as null does for the other optional params, and gets a test. - rustfmt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
004d015cfe
commit
ac6d72b44a
@@ -324,6 +324,17 @@ pub(crate) fn checked_id(s: &Session, value: &Value, command: &str) -> Result<No
|
||||
Ok(id)
|
||||
}
|
||||
|
||||
/// `v` as exactly `N` finite numbers (`[x, y]`, `[x, y, width, height]`…): `None` when it isn't an
|
||||
/// array of that length or an entry isn't a number, so nothing malformed is left out or padded.
|
||||
pub(crate) fn finite_numbers<const N: usize>(v: &Value) -> Option<[f64; N]> {
|
||||
let a = v.as_array().filter(|a| a.len() == N)?;
|
||||
let mut out = [0.0; N];
|
||||
for (o, n) in out.iter_mut().zip(a) {
|
||||
*o = n.as_f64().filter(|n| n.is_finite())?;
|
||||
}
|
||||
Some(out)
|
||||
}
|
||||
|
||||
pub(crate) fn point_param(p: &Value, key: &str) -> Option<Point> {
|
||||
let a = p.get(key)?.as_array()?;
|
||||
Some(Point::new(a.first()?.as_f64()?, a.get(1)?.as_f64()?))
|
||||
|
||||
@@ -309,15 +309,22 @@ fn transformed(mut node: Node, m: Affine) -> Node {
|
||||
/// `[x, y, width, height]` with a positive size.
|
||||
fn rect_param(p: &Value) -> Result<Option<Rect>> {
|
||||
let Some(v) = p.get("rect") else { return Ok(None) };
|
||||
// Reject invalid components instead of dropping them: filtering would turn a malformed
|
||||
// five-element box such as [0, "bad", 10, 40, 50] into a different valid rectangle.
|
||||
let numbers = v.as_array().filter(|a| a.len() == 4).and_then(|a| a.iter().map(Value::as_f64).collect::<Option<Vec<_>>>());
|
||||
match numbers.as_deref() {
|
||||
Some([x, y, w, h]) if *w > 0.0 && *h > 0.0 => Ok(Some(Rect::new(*x, *y, x + w, y + h))),
|
||||
// All four or nothing: a malformed entry dropped would shift the others ([0, "bad", 10, 40, 50]).
|
||||
match finite_numbers(v) {
|
||||
Some([x, y, w, h]) if w > 0.0 && h > 0.0 => Ok(Some(Rect::new(x, y, x + w, y + h))),
|
||||
_ => Err(bad(PLACE, "rect must be [x, y, width, height] with a positive width and height")),
|
||||
}
|
||||
}
|
||||
|
||||
/// `at`, the `[x, y]` the art is centred on (absent or null: none); anything else fails rather than
|
||||
/// placing the art as if no point were given.
|
||||
fn at_param(p: &Value) -> Result<Option<Point>> {
|
||||
p.get("at")
|
||||
.filter(|v| !v.is_null())
|
||||
.map(|v| finite_numbers(v).map(|[x, y]| Point::new(x, y)).ok_or_else(|| bad(PLACE, "at must be [x, y]")))
|
||||
.transpose()
|
||||
}
|
||||
|
||||
/// The transform that puts new art with the natural box `natural` where `old` is: an image's scale
|
||||
/// and rotation relative to its 100% size (its resolution's) and its centre are kept (with
|
||||
/// `image_xf`); other objects keep their centre.
|
||||
@@ -344,10 +351,7 @@ fn fit(natural: Rect, r: Rect) -> Affine {
|
||||
fn place(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let replace = bool_or(p, "replace", false);
|
||||
let template = bool_or(p, "template", false);
|
||||
let at = point_param(p, "at");
|
||||
if p.get("at").is_some() && (at.is_none() || p["at"].as_array().is_none_or(|a| a.len() != 2)) {
|
||||
return Err(bad(PLACE, "at must be [x, y] with two numeric coordinates"));
|
||||
}
|
||||
let at = at_param(p)?;
|
||||
let rect = rect_param(p)?;
|
||||
if replace && (template || at.is_some() || rect.is_some()) {
|
||||
return Err(bad(PLACE, "replace keeps the replaced object's place and transform: drop template, at and rect"));
|
||||
|
||||
@@ -57,7 +57,6 @@ fn serialize(s: &mut Session, format: &str) -> Vec<u8> {
|
||||
vectorcraft_format::base64_decode(v["dataBase64"].as_str().unwrap()).unwrap()
|
||||
}
|
||||
|
||||
|
||||
#[test]
|
||||
fn malformed_place_coordinates_are_rejected_instead_of_silently_changed() {
|
||||
let mut s = session();
|
||||
@@ -65,11 +64,7 @@ fn malformed_place_coordinates_are_rejected_instead_of_silently_changed() {
|
||||
let original = s.doc().unwrap().doc.layers.clone();
|
||||
// Previously filter_map dropped the string from a five-element rectangle and
|
||||
// treated the remaining four values as a different, valid placement.
|
||||
for rect in [
|
||||
json!([0, "ignored", 10, 40, 50]),
|
||||
json!([10, 20, 40, null, 50]),
|
||||
json!([10, 20, 40, 50, 60]),
|
||||
] {
|
||||
for rect in [json!([0, "ignored", 10, 40, 50]), json!([10, 20, 40, null, 50]), json!([10, 20, 40, 50, 60])] {
|
||||
let mut params = file("tiny.png", &png);
|
||||
params["rect"] = rect;
|
||||
assert!(s.execute("file.place", ¶ms).is_err(), "{params}");
|
||||
@@ -77,12 +72,7 @@ fn malformed_place_coordinates_are_rejected_instead_of_silently_changed() {
|
||||
}
|
||||
// An explicitly supplied invalid position formerly behaved as though no
|
||||
// position were supplied and centred the image on the artboard.
|
||||
for at in [
|
||||
json!(["not-a-number", 20]),
|
||||
json!([10, null]),
|
||||
json!([10, 20, 30]),
|
||||
json!("invalid"),
|
||||
] {
|
||||
for at in [json!(["not-a-number", 20]), json!([10, null]), json!([10, 20, 30]), json!("invalid")] {
|
||||
let mut params = file("tiny.png", &png);
|
||||
params["at"] = at;
|
||||
assert!(s.execute("file.place", ¶ms).is_err(), "{params}");
|
||||
@@ -92,6 +82,10 @@ fn malformed_place_coordinates_are_rejected_instead_of_silently_changed() {
|
||||
params["rect"] = json!([10, 20, 80, 60]);
|
||||
let placed = s.execute("file.place", ¶ms).unwrap();
|
||||
assert_eq!(placed["ids"].as_array().unwrap().len(), 1);
|
||||
// A null position is no position, as for the other optional params.
|
||||
let mut params = file("tiny.png", &png);
|
||||
params["at"] = Value::Null;
|
||||
assert_eq!(s.execute("file.place", ¶ms).unwrap()["ids"].as_array().unwrap().len(), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user