diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 66eb2104d1..eab57e2a91 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -426,8 +426,13 @@ pub(crate) fn bad(cmd: &str, msg: impl Into) -> EngineError { pub(crate) fn f64_or(p: &Value, key: &str, default: f64) -> f64 { p.get(key).and_then(Value::as_f64).unwrap_or(default) } +/// The number `p` gives for `key`. A missing key or null is "missing number `{key}`"; any other +/// value, such as `"40"`, is "`{key}` must be a number, not {v}", as [`number_or`] words it. pub(crate) fn f64_req(p: &Value, key: &str, cmd: &str) -> Result { - p.get(key).and_then(Value::as_f64).ok_or_else(|| bad(cmd, format!("missing number `{key}`"))) + match p.get(key) { + None | Some(Value::Null) => Err(bad(cmd, format!("missing number `{key}`"))), + Some(v) => v.as_f64().ok_or_else(|| bad(cmd, format!("`{key}` must be a number, not {v}"))), + } } /// The number `p` gives for `key`, or `default` when `p` leaves it out or gives null. Any other /// value, such as `"40"`, is an error: "`{key}` must be a number, not {v}". diff --git a/crates/engine/src/tests/editcolors.rs b/crates/engine/src/tests/editcolors.rs index 8f41ed3a73..a3dcea263a 100644 --- a/crates/engine/src/tests/editcolors.rs +++ b/crates/engine/src/tests/editcolors.rs @@ -77,10 +77,12 @@ fn saturate_needs_a_number_intensity() { let mut s = session(); let a = rect(&mut s, json!("#cc6666")); let undo = s.doc().unwrap().history.undo.len(); - for p in [json!({}), json!({"intensity": "40"}), json!({"intensity": null})] { + for p in [json!({}), json!({"intensity": null})] { let e = s.execute("edit.colors.saturate", &p).unwrap_err().to_string(); assert!(e.contains("missing number `intensity`"), "{p}: {e}"); } + let e = s.execute("edit.colors.saturate", &json!({"intensity": "40"})).unwrap_err().to_string(); + assert!(e.contains("`intensity` must be a number, not \"40\""), "{e}"); assert_eq!(fill_of(&s, a).to_hex(), "#cc6666"); assert_eq!(s.doc().unwrap().history.undo.len(), undo, "no undo step"); s.execute("edit.colors.saturate", &json!({"intensity": 100})).unwrap(); diff --git a/crates/engine/src/tests/params.rs b/crates/engine/src/tests/params.rs index 6850715b0e..3cbdb5df9f 100644 --- a/crates/engine/src/tests/params.rs +++ b/crates/engine/src/tests/params.rs @@ -484,6 +484,57 @@ fn a_malformed_object_id_is_not_reported_as_missing() { assert_eq!(undo(&s), before); } +/// A required number given as something other than a number (`"40"`, `true`, `[]`, `{}`) is refused +/// as the wrong type, naming its key and showing the value as given, not as a missing number; one left +/// out or given as null is still a missing number. Neither changes the document or records an undo step. +#[test] +fn a_malformed_required_number_is_not_reported_as_missing() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap(); + let doc = s.doc().unwrap().doc.clone(); + let undo = s.doc().unwrap().history.undo.len(); + let with = |base: &serde_json::Value, key: &str, v: Option| { + let mut p = base.clone(); + let o = p.as_object_mut().unwrap(); + match v { + Some(v) => o.insert(key.into(), v), + None => o.remove(key), + }; + p + }; + let cases = [ + ("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10}), "x"), + ("edit.colors.saturate", json!({"intensity": 40}), "intensity"), + ("object.envelope.setMeshPoint", json!({"id": 1, "index": 0, "x": 0, "y": 0}), "index"), + ("object.envelope.setMeshPoint", json!({"id": 1, "index": 0, "x": 0, "y": 0}), "x"), + ("object.mesh.movePoint", json!({"id": 1, "index": 0, "x": 0, "y": 0}), "y"), + ("guide.add", json!({"pos": 40}), "pos"), + ]; + for (cmd, base, key) in &cases { + for (v, shown) in [(json!("40"), "\"40\""), (json!(true), "true"), (json!([]), "[]"), (json!({}), "{}")] { + let p = with(base, key, Some(v)); + match s.execute(cmd, &p) { + Err(EngineError::BadParams { cmd: c, msg }) => { + assert_eq!((c.as_str(), msg), (*cmd, format!("`{key}` must be a number, not {shown}")), "{p}"); + } + other => panic!("{cmd} {p}: {other:?}"), + } + } + for p in [with(base, key, None), with(base, key, Some(json!(null)))] { + match s.execute(cmd, &p) { + Err(EngineError::BadParams { cmd: c, msg }) => assert_eq!((c.as_str(), msg), (*cmd, format!("missing number `{key}`")), "{p}"), + other => panic!("{cmd} {p}: {other:?}"), + } + } + } + assert_eq!(s.doc().unwrap().doc, doc); + assert_eq!(s.doc().unwrap().history.undo.len(), undo); + // A whole JSON number is a number for a fractional parameter. + s.execute("guide.add", &json!({"pos": 40})).unwrap(); + assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1); +} + /// A command that takes the keys of one that isn't checked yet isn't checked either. #[test] fn a_reference_to_an_unchecked_command_is_unchecked() { diff --git a/docs/mcp.md b/docs/mcp.md index 0a6d0cf4b9..6b665d16c7 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -547,7 +547,11 @@ change nothing; an omitted or null `id` keeps the default. Commands that need an `text.createInPath`) refuse a value that is not a non-negative integer with "`id` must be a whole number", naming the key, and report a key left out or null as missing. The optional `group` of `livePaint.fill` and `parent` of `node.move` are refused the same way when given; left out, they keep their defaults. `livePaint.info` reads `id` when -`group` is left out or null, and reports a missing group only when both are left out. A command that leaves out an object selected inside a +`group` is left out or null, and reports a missing group only when both are left out. A command that needs a number +(`x` of `shape.rectangle`, `pos` of `guide.add`, `index`, `x` and `y` of the `object.mesh.*` and +`object.envelope.setMeshPoint` point commands, and the others like them) refuses a value that is not a number with +`` `x` must be a number, not "40" ``, naming the key and showing the value as given, and reports a key left out or null +as `` missing number `x` ``; it changes nothing either way. A command that leaves out an object selected inside a selected group also leaves it out when `ids` list it together with a group that holds it. Every command that takes `ids` (or `slices`, for the slice commands) refuses a list with an entry that is not a non-negative integer, or a value that is not a list, and changes nothing; it does not act on the other entries. The error for an entry names the key, @@ -3564,7 +3568,8 @@ For `edit.colors.*`, `fill: false` leaves colorized image fills unchanged. `reco with one gray channel, so a PDF writes it in gray. `includePatterns: false` leaves patterns out. The Blend commands also grade gradient meshes, which keep their shading. `edit.colors.saturate` needs `intensity`, a number from -100 to 100 (a number outside the range counts as its nearest -end). Without one, or with a value that is not a number, it answers `` missing number `intensity` `` and changes nothing. +end). Without one, or with `null`, it answers `` missing number `intensity` ``; with a value that is not a number, such as +`"40"`, it answers `` `intensity` must be a number, not "40" ``. Either way it changes nothing. The `edit.colors.*` commands, `edit.colors.overprintBlack` and `addon.color.filter` answer a `fill`, `stroke`, `includeImages`, `includePatterns`, `convert`, `remove`, `includeCmyBlacks` or `includeSpotBlacks` that is not true or false with `` `fill` must be true or false ``, naming the key, a `percentage` that is not a number with diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 68b93ac4d8..c1b3cb6f62 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1365,7 +1365,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **Document Color Mode for effects and graphs:** File › Document Color Mode and `document.open {colorMode}` convert the colors of effects (Drop Shadow, Inner Glow, Outer Glow and the other effects with a color, on objects, their fills and strokes and in graphic styles) and the series paints and label styles that a graph keeps, so a graph drawn again from new data keeps the converted colors; File › Document Color Mode also converts the fill and stroke that new art takes. Undo restores the original new-art fill and stroke and Redo applies the conversion again. - **Recolor Artwork layout (#3416):** the Edit chip list uses the modal scroll helper and its actual remaining height; the tab's minimum-height space scrolls with the chips. `tests_recolor` covers 5/20/200/400 colors, window resizing, 100/125/150/200% UI scaling, scrolling to and editing the final chip, Preview, Cancel, Confirm and Undo, alongside the existing #1733 drag regressions. Scores and estimates unchanged. - **Recolor Artwork Edit displays:** buttons beside the Edit tab's wheel show the colors on a smooth color wheel, a segmented color wheel or as color bars ([Edit colors using Recolor Artwork](https://helpx.adobe.com/illustrator/desktop/manage-colors/modify-colors/edit-colors-using-edit-colors-dialog.html)). The segmented wheel draws 24 hues by 5 saturations as solid patches, and its markers move freely as on the smooth wheel. The bars show one solid bar per recolored color in row order: a click selects a bar for the color sliders, and a drag moves its row without changing a color ([Advanced recolor options overview](https://helpx.adobe.com/illustrator/desktop/manage-colors/learn-color-basics/edit-colors-dialog-overview.html)). The dialog opens on the smooth wheel; `display` (`smooth`, `segmented`, `bars`) is the dialog field. `tests_recolor` covers switching, the dialog height, segmented dragging and Brightness, and bar selection, editing, reordering, scrolling, Preview, Cancel and OK. Scores and estimates unchanged. - - **Edit Colors parameters:** `edit.colors.saturate` needs a number `intensity`, the value from -100% to 100% that Saturate takes ([Adjusting colors](https://web.archive.org/web/20230128232631/https://helpx.adobe.com/illustrator/using/adjusting-colors.html)), and answers "missing number `intensity`" without one; a number outside the range counts as its nearest end. `edit.colors.adjustBalance` refuses a channel and `color.harmony` refuses `steps` or `amount` that is not a number; both read `null` as a value left out. `tests::editcolors` and `tests::colorguide` cover them. + - **Edit Colors parameters:** `edit.colors.saturate` needs a number `intensity`, the value from -100% to 100% that Saturate takes ([Adjusting colors](https://web.archive.org/web/20230128232631/https://helpx.adobe.com/illustrator/using/adjusting-colors.html)), and answers "missing number `intensity`" without one; a number outside the range counts as its nearest end. A value that is not a number, such as `"40"`, is refused with `` `intensity` must be a number, not "40" ``, as in every command that needs a number (`tests::params::a_malformed_required_number_is_not_reported_as_missing`). `edit.colors.adjustBalance` refuses a channel and `color.harmony` refuses `steps` or `amount` that is not a number; both read `null` as a value left out. `tests::editcolors` and `tests::colorguide` cover them. - **Working RGB numeric controls:** the Color panel and Color Picker read RGB, HSB, hex and Web Safe RGB values in the active working RGB space. Editing one RGB channel preserves the other native channels, including colors outside display sRGB. Regressions cover sRGB, Display P3, Wide Gamut RGB and ProPhoto RGB, picker initialization, native numeric round trips, and a slider click with one Undo step. Scores and estimates unchanged. - **Pasting into hex color fields:** in the hex fields of the Color Picker, Swatch Options, Recolor Artwork, the Color panel, Document Setup, Duotone, Color Themes and Perspective Grid, a click or keyboard focus selects the digits. A pasted code of six digits, with or without `#`, replaces them wherever the caret is. A pasted code of three digits also replaces them, unless the paste holds only those digits and exactly three digits stay; those digits then edit the text in place. A press on another field commits the text as Tab and Enter do. As VectorCraft's own rule, a paste that is not a color leaves the field as it was. Tests: `a_hex_code_pasted_into_the_hex_field_replaces_its_digits` in `crates/ui-egui/src/dialogs/tests.rs`, `define_grid_takes_a_three_digit_color` in `crates/ui-egui/src/dialogs/tests_perspective.rs` and `a_paste_into_a_hex_field_replaces_or_edits_its_digits` in `crates/ui-egui/src/tests/numfields.rs`. - **Color Picker layout (#2008):** a centered draggable header, equal-width actions above aligned HSB/RGB/Hex and Lab/CMYK fields, compact field and slider, and warnings below the preview. No horizontal scrolling; narrow windows stack the sections with vertical scrolling. Tests in `dialogs/color_picker.rs` cover actual pointer/key input, typed values before OK/Enter, Cancel/Escape rollback, one Undo step, mode-size stability, warnings and layout at two UI scales. The screen sampler sits above the field and the upstream two-grid Tab order is retained. AccessKit regressions check the action names and each channel slider's displayed value, units and range. German and enlarged English UI renders were reviewed locally; no reference images are shipped. This is a layout improvement, with no change to scores or remaining-hour estimates.