Merge pull request #4483 from dotCooCoo/fix/saturate-requires-intensity

Edit Colors: Saturate without an intensity is an error instead of a pass at 0

crates/engine/src/cmd/colorcmds.rs

@@ -19,7 +19,8 @@ use vectorcraft_doc::{Document, ImageBlob, Node, NodeId, NodeKind};
 
 use super::edit::selected_roots;
 use super::{
-    CommandSpec, EngineError, Result, Session, always, bad, bool_or, cmd, color_value, f64_or, has_selection, ids_param, str_param, unique_name,
+    CommandSpec, EngineError, Result, Session, always, bad, bool_or, check_numbers, cmd, color_value, f64_or, f64_req, has_selection, ids_param,
+    str_param, unique_name,
 };
 
 pub fn specs() -> Vec<CommandSpec> {
@@ -70,7 +71,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Saturate…",
             ["Edit", "Edit Colors"],
             None,
-            "{intensity: -100..100 (%), fill?, stroke?, includeImages?, includePatterns?, ids?} scale saturation by (1 + intensity/100), reaching what edit.colors.invert does → {changed}",
+            "{intensity: -100..100 (%; a number outside the range counts as its nearest end), fill?, stroke?, includeImages?, includePatterns?, ids?} scale saturation by (1 + intensity/100), reaching what edit.colors.invert does → {changed}",
             has_selection,
             saturate
         ),
@@ -345,18 +346,21 @@ fn run_pass(s: &mut Session, p: &Value, label: &str, pass: Recolor) -> Result<Va
 }
 
 fn saturate(s: &mut Session, p: &Value) -> Result<Value> {
-    let i = (f64_or(p, "intensity", 0.0).clamp(-100.0, 100.0) / 100.0) as f32;
+    let i = (f64_req(p, "intensity", "edit.colors.saturate")?.clamp(-100.0, 100.0) / 100.0) as f32;
     recolor(s, p, "Saturate", &move |c| {
         let [h, sat, v] = c.to_hsb();
-        if sat <= 0.0 {
+        let to = (sat * (1.0 + i)).clamp(0.0, 1.0);
+        // A color whose saturation stays the same is returned as it is, so it keeps its swatch link.
+        if sat <= 0.0 || to == sat {
             return c;
         }
-        keep_model(c, Color::from_hsb(h, (sat * (1.0 + i)).clamp(0.0, 1.0), v))
+        keep_model(c, Color::from_hsb(h, to, v))
     })
 }
 
 fn adjust_balance(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "edit.colors.adjustBalance";
+    check_numbers(p, &["r", "g", "b", "c", "m", "y", "k", "gray", "tint"], C)?;
     let given = |keys: &[&str]| keys.iter().any(|k| p.get(*k).is_some());
     let mode = match str_param(p, "mode") {
         Some(m) => m.to_ascii_lowercase(),
@@ -513,6 +517,7 @@ fn harmony(s: &mut Session, p: &Value) -> Result<Value> {
         let ids: Vec<String> = Harmony::ALL.iter().map(|h| h.id()).collect();
         bad(C, format!("unknown rule `{rule}` ({})", ids.join("|")))
     })?;
+    check_numbers(p, &["steps", "amount"], C)?;
     let mut opts = GuideOptions::default();
     if let Some(n) = p.get("steps").and_then(Value::as_u64) {
         opts.steps = n.min(u32::MAX as u64) as u32;

crates/engine/src/tests/colorguide.rs

@@ -47,6 +47,19 @@ fn harmony_query_returns_the_group_and_its_grid() {
     assert!(s.execute("color.harmony", &json!({"color": "#ff0000", "rule": "triad", "variation": "loud"})).is_err());
 }
 
+/// The Color Guide refuses `steps` or `amount` that is not a number, which it would otherwise
+/// replace with its default.
+#[test]
+fn harmony_refuses_steps_or_amount_that_is_not_a_number() {
+    let mut s = Session::new();
+    for p in [json!({"color": "#ff0000", "rule": "triad", "steps": "2"}), json!({"color": "#ff0000", "rule": "triad", "amount": "100"})] {
+        let e = s.execute("color.harmony", &p).unwrap_err().to_string();
+        assert!(e.contains("must be a number"), "{p}: {e}");
+    }
+    let r = s.execute("color.harmony", &json!({"color": "#ff0000", "rule": "triad", "steps": null, "amount": null})).unwrap();
+    assert_eq!(r["grid"][0].as_array().unwrap().len(), 9, "a null is read as a key left out");
+}
+
 #[test]
 fn new_swatch_saves_several_colours_in_one_step() {
     let mut s = session();

crates/engine/src/tests/editcolors.rs

@@ -70,6 +70,60 @@ fn balance_previews_and_commits_one_step() {
     assert_eq!(fill_of(&s, a).to_hex(), "#808080");
 }
 
+/// Saturate needs its intensity: a call without one, or with one that is not a number, is an error
+/// that changes nothing and records no undo step. A number outside −100..100 counts as its nearest end.
+#[test]
+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})] {
+        let e = s.execute("edit.colors.saturate", &p).unwrap_err().to_string();
+        assert!(e.contains("missing number `intensity`"), "{p}: {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();
+    let full = fill_of(&s, a).to_hex();
+    assert_ne!(full, "#cc6666");
+    s.execute("edit.undo", &json!({})).unwrap();
+    s.execute("edit.colors.saturate", &json!({"intensity": 250})).unwrap();
+    assert_eq!(fill_of(&s, a).to_hex(), full, "250 counts as 100");
+}
+
+/// Saturate at 0 leaves every color as it was: a swatch-linked fill keeps its exact color and its link.
+#[test]
+fn saturate_keeps_a_color_it_does_not_change() {
+    let mut s = session();
+    s.execute("swatch.new", &json!({"name": "Sky", "color": "#3a7bd5", "global": true})).unwrap();
+    let a = rect(&mut s, json!("#3a7bd5"));
+    s.execute("paint.setFill", &json!({"ids": [a.0], "swatch": "Sky"})).unwrap();
+    let b = rect(&mut s, json!({"c": 20, "m": 40, "y": 0, "k": 10}));
+    let paint = |s: &Session, id: NodeId| s.doc().unwrap().doc.node(id).unwrap().appearance.fill_paint().clone();
+    let (sky, cmyk) = (paint(&s, a), paint(&s, b));
+    let v = s.execute("edit.colors.saturate", &json!({"ids": [a.0, b.0], "intensity": 0})).unwrap();
+    assert_eq!(v["changed"], 0);
+    assert_eq!(paint(&s, a), sky);
+    assert_eq!(paint(&s, b), cmyk);
+}
+
+/// Adjust Color Balance refuses a channel that is not a number, which it would otherwise read as 0.
+#[test]
+fn adjust_balance_refuses_a_channel_that_is_not_a_number() {
+    let mut s = session();
+    let a = rect(&mut s, json!("#808080"));
+    let undo = s.doc().unwrap().history.undo.len();
+    for p in [json!({"mode": "rgb", "r": "40"}), json!({"c": "10"}), json!({"gray": true}), json!({"tint": "5"})] {
+        let e = s.execute("edit.colors.adjustBalance", &p).unwrap_err().to_string();
+        assert!(e.contains("must be a number"), "{p}: {e}");
+    }
+    assert_eq!(fill_of(&s, a).to_hex(), "#808080");
+    assert_eq!(s.doc().unwrap().history.undo.len(), undo, "no undo step");
+    // A null is read as a channel left out.
+    s.execute("edit.colors.adjustBalance", &json!({"mode": "rgb", "r": 20, "g": null})).unwrap();
+    assert_eq!(fill_of(&s, a).to_hex(), "#b38080");
+}
+
 // ---------- meshes, images and patterns ----------
 
 fn mesh_colors(s: &Session, id: NodeId) -> Vec<Color> {

docs/mcp.md

@@ -1129,7 +1129,8 @@ and `radial` in `shape.polarGrid`; `size` and the `width`, `height` and `fitMinP
 `end` in `type.step`; `by` in `type.size.increase` and `type.size.decrease`; `size` in `type.ruby`; `width`, `height`
 and `fitMinPercent` in `text.areaOptions`; `x` and `y` in `symbol.place`; `radius`, `density` and `scale` in
 `symbol.spray`; `radius` and `intensity` in `symbol.adjust`; `to` in `swatch.move`; `weight` and `opacity` in
-`appearance.setItem`; `angle` in `paint.editGradient` and in the `gradient` of a command that takes a paint; `t` in
+`appearance.setItem`; `r`, `g`, `b`, `c`, `m`, `y`, `k`, `gray` and `tint` in `edit.colors.adjustBalance`; `steps`
+and `amount` in `color.harmony`; `angle` in `paint.editGradient` and in the `gradient` of a command that takes a paint; `t` in
 `paint.freeform.splitLine`; `tolerance` and `gap` in `shapeBuilder.merge`; `gap` in `shapeBuilder.regions`; and
 `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 ``.
@@ -1716,7 +1717,9 @@ the scope and what differs from the public documentation.
 fills, strokes, text, gradient stops, gradient-mesh points, embedded images (a recoloured copy of the image; linked
 images are left alone) and the tiles of pattern fills and strokes (a recoloured copy saved as a new pattern swatch,
 e.g. "Dots 2"; the original pattern is untouched). `includeImages: false` / `includePatterns: false` leave images or
-patterns out. The Blend commands also grade gradient meshes, which keep their shading.
+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.
 
 ```json
 {"name":"run_command","arguments":{"command":"recolor.colors","params":{}}}

docs/target-app-parity.md

@@ -845,6 +845,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
   - **Colour:** a Color Picker (colour field, H/S/B/R/G/B channel slider, HSB/RGB/Lab/CMYK fields, hex, web-only snapping, out-of-gamut correction); the Color panel shows a selected colour in its own model and, with nothing selected, the new-art defaults in the mode picked last, which it keeps between sessions; Alt-click paints the other proxy; proxies look through groups and show "?" for mixed paint; Invert, Complement, Apply Last Color (`,`) and Apply Last Gradient (`.`) keep the colour model; every paint command feeds the recent colours. The Color Guide has 21 harmony rules, Steps and Variation options and Limit to Library. Adjust Color Balance (its Global mode shifts tints) and Saturate preview live; Edit Colors and Recolor also reach meshes, images and patterns. Recolor Artwork has colour reduction, five recolour methods, presets, Limit to Library, a colour-wheel Edit tab and Edit Color Group; a local Color Themes panel saves five-colour themes. Fill/Stroke chips open swatch and mixer popovers (the Properties panel has the Control bar's Fill and Stroke chips, Stroke link and weight spinner, with nothing selected too), panels have Illustrator's Window menu keys (Align Shift+F7, Transform Shift+F8, Pathfinder Cmd+Shift+F9, Layers F7, Character Cmd+T…; every panel can be given one in Edit › Keyboard Shortcuts), and Eyedropper Options pick up and apply chosen attributes (Alt and Shift+Alt, image pixels). An ICC CMYK profile converts straight into the RGB working space (not through sRGB, so a wider space such as Wide Gamut RGB keeps colours sRGB can't hold) with the colour settings' black-point compensation, on the canvas and in RGB exports alike (the built-in Generic CMYK compensates both ways, so RGB black separated to a rich black displays as black again). Fills and strokes can overprint (Attributes panel, Overprint Black options), and CMYK documents blend in inks on screen, in PDF and when flattening. Document Color Mode converts the art, symbols, pattern tiles and swatches through the colour settings: Gray stays Gray, RGB greys separate into four-colour greys and a rich black as Illustrator does, or go on the black plate only with `grays: "black"` (also for `document.open {colorMode}`).
     View › Proof Setup › Color blindness – Tritanopia-type turns Proof Colors on and proofs the canvas with the tritanopia simulation of Brettel, Viénot and Mollon (1997). The Separations Preview panel's Proof Setup list offers the same target (`view.proofSetup {target: "tritanopia"}`). Grays keep their value.
   - **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.
+  - **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.
   - **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.
   - **Eyedropper sources (#1251):** ordinary and Shift clicks, Shift+Alt appearance append, raster pixels and gradient-stop sampling can read visible locked/template art and sources outside isolation. Sampling leaves the source and selection intact; Alt reverse still uses editable hits, and hidden art stays excluded. Engine/tool/hit regressions cover the editing boundary, color models and one-step Undo/Redo. The general object/image workflow is described in [public HelpX](https://helpx.adobe.com/illustrator/using/tool-techniques/eyedropper-tool.html); the lock/isolation expectations come from the user report and these regressions, without a new full-parity measurement.
   - **Eyedropper colors in a CMYK document (#2276):** a CMYK image (a four-component JPEG, or an 8- or 16-bit chunky CMYK TIFF) gives its ink amounts as they are, and an RGB image's pixel or an RGB color given to `paint.setFill`, `paint.setStroke`, `paint.sampleColor` or `swatch.new {color}` (hex input and gradient stops included) is separated through the color settings' CMYK working space and intent, as Document Color Mode converts it. With the default Generic CMYK, RGB 83/170/223 separates to C59 M12 Y3 K0, where the profile-free formula gave C63 M24 Y0 K13.