diff --git a/crates/engine/src/cmd/textedit.rs b/crates/engine/src/cmd/textedit.rs index b6d20999a..c315c2a1c 100644 --- a/crates/engine/src/cmd/textedit.rs +++ b/crates/engine/src/cmd/textedit.rs @@ -12,7 +12,7 @@ use super::stroke::StrokeChange; use super::typecmd::refresh_bounds; use super::{ CommandSpec, EngineError, Result, Session, bad, bool_or, check_numbers, check_switches, cmd, color_value, has_doc, has_selection, id_param, - index, number_or, point_param, str_param, switch_or, targets, + index, number_or, point_param, str_param, switch_or, targets, text_param, }; pub fn specs() -> Vec { @@ -568,7 +568,8 @@ pub(crate) fn change_font_axes(st: &mut CharStyle, change: &AxisChange) { /// `features: ["dlig", "-liga", …]` → the canonical tag list (differences from the defaults). pub(crate) fn features_param(p: &Value, cmd: &str) -> Result>> { let Some(v) = p.get("features").filter(|v| !v.is_null()) else { return Ok(None) }; - let tags: Vec<&str> = v.as_array().ok_or_else(|| bad(cmd, "features must be a list of tags"))?.iter().filter_map(Value::as_str).collect(); + let not_tags = || bad(cmd, "features must be a list of tags"); + let tags: Vec<&str> = v.as_array().ok_or_else(not_tags)?.iter().map(|t| t.as_str().ok_or_else(not_tags)).collect::>()?; if let Some(t) = tags.iter().find(|t| !vectorcraft_text::OtFeatures::known_tag(t)) { return Err(bad( cmd, @@ -708,8 +709,8 @@ impl CharChange { stroke_opts.weight = Some(w.clamp(0.0, 1000.0)); } let c = Self { - font: str_param(p, "font").map(str::to_string), - style: str_param(p, "style").map(str::to_string), + font: text_param(p, "font", cmd)?.map(str::to_string), + style: text_param(p, "style", cmd)?.map(str::to_string), size: num("size"), leading: auto_or_num(p, "leading", cmd)?, tracking: num("tracking"), diff --git a/crates/engine/src/cmd/typecmd.rs b/crates/engine/src/cmd/typecmd.rs index 2257b1a8a..42b35d33d 100644 --- a/crates/engine/src/cmd/typecmd.rs +++ b/crates/engine/src/cmd/typecmd.rs @@ -11,7 +11,7 @@ use super::edit::selected_roots; use super::pathops::{len_param, num_param, shape_node}; use super::{ CommandSpec, EngineError, Result, Session, bad, check_numbers, cmd, color_value, f64_req, has_doc, has_selection, id_param, index, number_or, ok, - str_param, switch_or, targets, + str_param, switch_or, targets, text_param, }; pub fn specs() -> Vec { @@ -249,16 +249,16 @@ fn ruby(s: &mut Session, p: &Value) -> Result { let ids = text_targets(s, p, C)?; let range = TextRange::parse(p, C)?; let remove = switch_or(p, "remove", false, C)?; - let reading = str_param(p, "text").map(str::trim).filter(|t| !t.is_empty()).map(str::to_string); + let reading = text_param(p, "text", C)?.map(str::trim).filter(|t| !t.is_empty()).map(str::to_string); if !remove && reading.is_none() { return Err(bad(C, "give `text` (the reading) or `remove`")); } - let position = match str_param(p, "position").map(str::to_ascii_lowercase).as_deref() { + let position = match text_param(p, "position", C)?.map(str::to_ascii_lowercase).as_deref() { None | Some("above") | Some("right") => RubyPosition::Above, Some("below") | Some("left") => RubyPosition::Below, Some(_) => return Err(bad(C, "`position` is above or below")), }; - let align = match str_param(p, "align").map(str::to_ascii_lowercase).as_deref() { + let align = match text_param(p, "align", C)?.map(str::to_ascii_lowercase).as_deref() { None | Some("center") | Some("centre") => RubyAlign::Center, Some("start") | Some("left") | Some("top") => RubyAlign::Start, Some("end") | Some("right") | Some("bottom") => RubyAlign::End, @@ -810,11 +810,30 @@ pub(crate) fn justify_param(v: &str) -> Option { }) } +/// The number `p` gives `cmd` for `key`, as [`num_param`] reads it (`"20"` included), or `None` when +/// `p` leaves it out or gives null. Any other value, such as `true` or `"wide"`, is an error: +/// "`{key}` must be a number, not {v}". +fn number_given(p: &Value, key: &str, cmd: &str) -> Result> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(v) => num_param(p, key).map(Some).ok_or_else(|| bad(cmd, format!("`{key}` must be a number, not {v}"))), + } +} + +/// The distance `p` gives `cmd` for `key`, as [`len_param`] reads it (`"5 mm"` included), or `None` +/// when `p` leaves it out or gives null. Any other value is an error, as [`number_given`] gives it. +fn length_given(p: &Value, key: &str, cmd: &str) -> Result> { + match p.get(key) { + None | Some(Value::Null) => Ok(None), + Some(v) => len_param(p, key).map(Some).ok_or_else(|| bad(cmd, format!("`{key}` must be a number, not {v}"))), + } +} + fn set_style(s: &mut Session, p: &Value) -> Result { const C: &str = "text.setStyle"; - let font = str_param(p, "font").map(str::to_string); - let style = str_param(p, "style").map(str::to_string); - let size = len_param(p, "size"); + let font = text_param(p, "font", C)?.map(str::to_string); + let style = text_param(p, "style", C)?.map(str::to_string); + let size = length_given(p, "size", C)?; if size.is_some_and(|v| v <= 0.0) { return Err(bad(C, "size must be positive")); } @@ -823,8 +842,8 @@ fn set_style(s: &mut Session, p: &Value) -> Result { Some(Value::String(a)) if a.eq_ignore_ascii_case("auto") => Some(None), Some(_) => Some(Some(len_param(p, "leading").ok_or_else(|| bad(C, "leading must be a number or \"auto\""))?.clamp(0.1, 5000.0))), }; - let tracking = num_param(p, "tracking"); - let justify = match str_param(p, "justify") { + let tracking = number_given(p, "tracking", C)?; + let justify = match text_param(p, "justify", C)? { Some(j) => Some(justify_param(j).ok_or_else(|| bad(C, "justify must be auto|left|center|right|justifyAll"))?), None => None, }; @@ -1282,7 +1301,7 @@ fn set_path_handle(s: &mut Session, p: &Value) -> Result { let id = id_param(p, "id").ok_or_else(|| bad(C, "missing id"))?; let si = super::draw2::usize_opt(p, "subpath", C)?.unwrap_or(0); let ai = p.get("anchor").and_then(Value::as_u64).map(index).ok_or_else(|| bad(C, "missing anchor"))?; - let out = str_param(p, "which") != Some("in"); + let out = super::object::choice(p, C, "which", &["in", "out"])? != Some("in"); let pos = vectorcraft_geom::Point::new(f64_req(p, "x", C)?, f64_req(p, "y", C)?); let independent = switch_or(p, "independent", false, C)?; let node = s.doc()?.doc.node(id); diff --git a/crates/engine/src/tests/textedit.rs b/crates/engine/src/tests/textedit.rs index 885ce09e1..425d545df 100644 --- a/crates/engine/src/tests/textedit.rs +++ b/crates/engine/src/tests/textedit.rs @@ -467,6 +467,82 @@ fn type_commands_refuse_a_number_or_boolean_given_as_text() { assert!(!t.ruby.is_empty() && t.ruby.iter().all(|r| !r.overhang && r.size_percent == 50.0), "{:?}", t.ruby); } +/// The Character commands, Ruby and Move Type Path Handle refuse a param of the wrong type with an +/// error naming the param, also when another param would change something, and leave the +/// document, its revision, the selection and the undo history as they were. Null counts as a param +/// left out, which keeps its default, and `size` and `leading` take a length with a unit. +#[test] +fn character_ruby_and_path_handle_refuse_a_param_of_the_wrong_type() { + use crate::tests::{history, not_a_number, select_with_a_step_to_redo}; + let mut s = session(); + let id = text(&mut s, "abc"); + let path = s.execute("path.create", &json!({"anchors": [{"x": 100, "y": 300, "in": [50, 300], "out": [150, 300]}, {"x": 400, "y": 300}]})); + let on_path = s.execute("text.createInPath", &json!({"path": path.unwrap()["id"], "mode": "onPath", "text": "Along"})).unwrap()["id"].clone(); + select_with_a_step_to_redo(&mut s, &[id]); + let string = |k: &str| format!("`{k}` must be a string"); + let tags = "features must be a list of tags".to_string(); + let which = "`which` must be one of in, out".to_string(); + let handle = |which: Value| json!({"id": on_path, "anchor": 0, "which": which, "x": 100, "y": 250}); + let mut calls = vec![ + ("text.setStyle", json!({"font": 5}), string("font")), + ("text.setStyle", json!({"style": true}), string("style")), + ("text.setStyle", json!({"size": 30, "style": ["Bold"]}), string("style")), + ("text.setStyle", json!({"justify": 1}), string("justify")), + ("text.setStyle", json!({"size": 30, "justify": {"value": "center"}}), string("justify")), + ("text.setStyle", json!({"features": ["dlig", 5]}), tags.clone()), + ("text.setStyle", json!({"leading": true}), "leading must be a number or \"auto\"".to_string()), + ("text.setRangeStyle", json!({"id": id.0, "font": 5}), string("font")), + ("text.setRangeStyle", json!({"id": id.0, "size": 30, "style": false}), string("style")), + ("text.setRangeStyle", json!({"id": id.0, "features": [null]}), tags), + ("type.ruby", json!({"text": 5}), string("text")), + ("type.ruby", json!({"remove": true, "text": ["a"]}), string("text")), + ("type.ruby", json!({"text": "a", "position": 1}), string("position")), + ("type.ruby", json!({"text": "a", "align": true}), string("align")), + ("text.setPathHandle", handle(json!(1)), which.clone()), + ("text.setPathHandle", handle(json!("up")), which), + ]; + calls.extend( + [ + ("text.setStyle", json!({"size": true}), "size"), + ("text.setStyle", json!({"size": "big"}), "size"), + ("text.setStyle", json!({"font": "Inter", "size": [12]}), "size"), + ("text.setStyle", json!({"tracking": "wide"}), "tracking"), + ("text.setStyle", json!({"size": 30, "tracking": false}), "tracking"), + ] + .map(|(cmd, p, key)| { + let (_, p, msg) = not_a_number(cmd, p, key); + (cmd, p, msg) + }), + ); + let state = |s: &Session| { + let st = s.doc().unwrap(); + (st.doc.clone(), st.revision, st.selection.objects.clone(), history(s)) + }; + let before = state(&s); + let wrong: Vec = calls + .iter() + .filter_map(|(cmd, p, want)| { + let r = s.execute(cmd, p); + let refused = matches!(&r, Err(EngineError::BadParams { msg, .. }) if msg == want); + let unchanged = state(&s) == before; + (!refused || !unchanged).then(|| format!("{cmd} {p}: {r:?}, unchanged: {unchanged}")) + }) + .collect(); + assert!(wrong.is_empty(), "not refused: {wrong:#?}"); + // Null keeps each default; a size and a leading with a unit are lengths. + let r = json!({"font": null, "style": null, "justify": null, "tracking": null, "features": null, "leading": null, "size": "5 mm"}); + s.execute("text.setStyle", &r).unwrap(); + let st = obj(&s, id).runs[0].style.clone(); + assert!((st.size - 5.0 * 72.0 / 25.4).abs() < 1e-6 && st.tracking == 0.0 && st.leading.is_none(), "{st:?}"); + s.execute("text.setStyle", &json!({"size": "12 pt", "leading": "18 pt", "tracking": 50})).unwrap(); + let st = obj(&s, id).runs[0].style.clone(); + assert_eq!((st.size, st.leading, st.tracking), (12.0, Some(18.0), 50.0)); + s.execute("type.ruby", &json!({"text": "a", "position": null, "align": null})).unwrap(); + let ruby = obj(&s, id).ruby; + assert!(ruby.iter().all(|r| r.position == vectorcraft_doc::RubyPosition::Above && r.align == vectorcraft_doc::RubyAlign::Center), "{ruby:?}"); + s.execute("text.setPathHandle", &handle(Value::Null)).unwrap(); +} + /// Where each glyph of type object `id` is drawn from (its origin in font units), document space. fn glyph_pens(s: &Session, id: NodeId) -> Vec<(usize, Point)> { let t = obj(s, id); diff --git a/docs/mcp.md b/docs/mcp.md index 972731ccc..a0a7f20b4 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1358,7 +1358,7 @@ naming the key, and change nothing: `radius` in `shape.rectangle`; `sides` and ` in `shape.flare`; `decay` and `segments` in `shape.spiral`; `rows` and `columns` in `shape.rectangularGrid`; `concentric` and `radial` in `shape.polarGrid`; `size` and the `width`, `height` and `fitMinPercent` of `area` in `text.create`; `size` and `fitMinPercent` in `text.createInPath`; `start`, `end`, `size`, `tracking`, -`baselineShift`, `hScale`, `vScale`, `rotation` and `strokeWidth` in `text.setRangeStyle`; `start` and `end` in +`baselineShift`, `hScale`, `vScale`, `rotation` and `strokeWidth` in `text.setRangeStyle`; `strokeWidth` in `text.setStyle`; `start` and `end` in `text.editRange` and `text.getRange`; `at` in `text.startList`; `start`, `hScale`, `vScale` and `rotation` in `text.touchType`; `start` in `text.glyphAlternates` and `text.setGlyphAlternate`; `by`, `start` and `end` in `type.step`; `by` in `type.size.increase` and `type.size.decrease`; `size` in `type.ruby`; `width`, `height` @@ -1370,6 +1370,15 @@ and `amount` in `color.harmony`; `angle` in `paint.editGradient` and in the `gra `width` and `tolerance` in `livePaint.strokeEdge`. A null is read as a key left out. A gradient stop that names a swatch refuses a `tint` that is not a number with `` stop 0: `tint` must be a number ``. +`text.setStyle` refuses a `size` that is neither a number nor a length with a unit (`"12 pt"`, `"5 mm"`) with +`` `size` must be a number, not true ``, and a `tracking` that is neither a number nor a number written as a string +(`"20"`) with `` `tracking` must be a number, not false ``. It and `text.setRangeStyle` refuse a `font` or `style` that is not a string +with `` `font` must be a string ``, and a `features` list with an entry that is not a string with +`features must be a list of tags`; `text.setStyle` refuses such a `justify` too. `type.ruby` refuses a `text`, +`position` or `align` that is not a string, and `text.setPathHandle` refuses a `which` other than `in` or `out` with +`` `which` must be one of in, out ``. Each leaves the document, the selection and the undo history as they were, also +when the call gives another param it could apply. A null is read as a key left out. + A byte offset must be a whole number of at least 0. `at` in `text.startList`, and `start` and `end` in `text.editRange`, `text.getRange` and `text.setRangeStyle`, and in `type.step` and `addon.type.textTweaker` given an `id`, refuse a negative or fractional number with