diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 7b6cdcc58..72e33b899 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -519,9 +519,13 @@ pub(crate) fn checked_ids_param(s: &Session, p: &Value, key: &str, command: &str } /// The objects a command acts on: `ids` when given (every one checked, and those inside another -/// given one dropped), else the selected objects. +/// given one dropped), else (`ids` left out or null) the selected objects. pub(crate) fn checked_ids_or_selection(s: &Session, p: &Value, command: &str) -> Result> { - if p.get("ids").is_some() { Ok(edit::outermost(&s.doc()?.doc, checked_ids_param(s, p, "ids", command)?)) } else { edit::selected_roots(s) } + if p.get("ids").is_some_and(|v| !v.is_null()) { + Ok(edit::outermost(&s.doc()?.doc, checked_ids_param(s, p, "ids", command)?)) + } else { + edit::selected_roots(s) + } } pub(crate) fn checked_id(s: &Session, value: &Value, command: &str) -> Result { diff --git a/crates/engine/src/cmd/addons/cleanup/text.rs b/crates/engine/src/cmd/addons/cleanup/text.rs index 6e6abfaa4..10fd432df 100644 --- a/crates/engine/src/cmd/addons/cleanup/text.rs +++ b/crates/engine/src/cmd/addons/cleanup/text.rs @@ -806,7 +806,7 @@ fn remove_breaks(s: &mut Session, p: &Value) -> Result { }; // The characters a Type tool has selected (after Select All in threaded text, all the text of // every frame of the story), else all the text of the selected type. - let editing = if p.get("ids").is_some() { None } else { editing_range(s)?.filter(|(_, a, b)| a != b) }; + let editing = if p.get("ids").is_some_and(|v| !v.is_null()) { None } else { editing_range(s)?.filter(|(_, a, b)| a != b) }; let story = if editing.is_some() { editing_story(s) } else { vec![] }; let d = &s.doc()?.doc; // Each type object with the breaks that go from it. diff --git a/crates/engine/src/cmd/addons/cleanup/text/tests.rs b/crates/engine/src/cmd/addons/cleanup/text/tests.rs index 1916d9dd2..0af24be36 100644 --- a/crates/engine/src/cmd/addons/cleanup/text/tests.rs +++ b/crates/engine/src/cmd/addons/cleanup/text/tests.rs @@ -288,6 +288,18 @@ fn remove_breaks_in_the_characters_a_type_tool_selected() { assert_eq!(obj(&s, id).plain_text(), "a b\nc"); } +/// `ids: null` is `ids` left out: the characters the Type tool selected lose their breaks, and +/// the rest of the text keeps them. +#[test] +fn remove_breaks_with_ids_null_in_the_characters_a_type_tool_selected() { + let mut s = session(); + let id = text(&mut s, json!({"x": 40, "y": 60, "text": "a\nb\nc", "size": 12})); + select(&mut s, &[id]); + edit_text(&mut s, id, 0, 2); + s.execute("addon.cleanup.removeBreaks", &json!({"kind": "paragraph", "ids": null})).unwrap(); + assert_eq!(obj(&s, id).plain_text(), "a b\nc"); +} + #[test] fn split_at_the_cursor_and_around_a_selection() { for (anchor, caret, want) in [(6, 6, &["Split", "here please"][..]), (6, 10, &["Split", "here", "please"][..])] { diff --git a/crates/engine/src/tests/ids_without_selection.rs b/crates/engine/src/tests/ids_without_selection.rs index 8e26577f9..1e13c468c 100644 --- a/crates/engine/src/tests/ids_without_selection.rs +++ b/crates/engine/src/tests/ids_without_selection.rs @@ -436,9 +436,7 @@ fn an_ids_entry_that_is_not_an_id_is_refused_and_changes_nothing() { /// With the rectangle apart (`d`) selected, the same commands with `ids` (or `slices`) left out and /// with it null do the same: what the selection's list does for the commands that take the /// selection, and the command's own default for the others (`object.slice.select` deselects the -/// slices). `addon.color.filter` and `addon.color.applyNearestSwatch` read `ids` through -/// [`cmd::checked_ids_or_selection`], which refuses null as a value that is not a list and changes -/// nothing. +/// slices). #[test] fn ids_left_out_or_null_take_the_selection_or_the_default() { let (_, [.., d], _) = doc_with_type(); @@ -450,15 +448,7 @@ fn ids_left_out_or_null_take_the_selection_or_the_default() { let mut null = params.clone(); null[key] = serde_json::Value::Null; let with_null = outcome(cmd, &null, &[d], slice); - if matches!(cmd, "addon.color.filter" | "addon.color.applyNearestSwatch") { - let (mut s, ..) = doc_with_type(); - s.execute("select.set", &json!({ "ids": [d] })).unwrap(); - let st = s.doc().unwrap(); - let refused = with_null.0.as_ref().is_err_and(|e| e.ends_with("ids must be an array of non-negative integer object ids")); - if !refused || (&with_null.1, &with_null.2, with_null.3) != (&*st.doc, &st.selection, st.history.undo.len()) { - failures.push(format!("{cmd} {key}: null is not refused with nothing changed ({:.160?})", with_null.0)); - } - } else if with_null != left_out { + if with_null != left_out { failures.push(format!("{cmd} {key}: null does not do what leaving it out does ({:.160?})", left_out.0)); } let mut listed = params.clone(); @@ -477,6 +467,65 @@ fn ids_left_out_or_null_take_the_selection_or_the_default() { assert!(failures.is_empty(), "{} fail:\n{}", failures.len(), failures.join("\n")); } +/// The commands that read `ids` through [`cmd::checked_ids_or_selection`] take `ids: null` as left +/// out: with the rectangle apart (`d`) or the type object (`t`) selected, null gives the same +/// result or error, document, selection and undo history as leaving `ids` out. Print Ink Fixes +/// runs in a CMYK document. +#[test] +fn ids_null_takes_the_selection_in_every_command_that_falls_back_to_it() { + let (_, [.., d], t) = doc_with_type(); + let mut failures = vec![]; + let (rect, both, text) = (&[d][..], &[d, t][..], &[t][..]); + let cases = [ + ("addon.arrange.randomizeOpacity", json!({"seed": 1}), rect), + ("addon.arrange.sort", json!({"by": "area", "apply": "x"}), both), + ("addon.cleanup.simplify", json!({}), rect), + ("addon.cleanup.deepUngroup", json!({}), rect), + ("addon.cleanup.rejoin", json!({}), rect), + ("addon.color.nearestSwatch", json!({}), rect), + ("addon.pathtools.cornerSet", json!({"radius": 2}), rect), + ("addon.pathtools.cornerRemove", json!({}), rect), + ("addon.pathtools.shape.convert", json!({}), rect), + ("addon.pathtools.erase", json!({"points": [[0, 20], [200, 20]]}), rect), + ("addon.pathtools.crossings", json!({}), rect), + ("addon.pathtools.expandDashes", json!({}), rect), + ("object.path.smooth", json!({}), rect), + ("object.textWrap.make", json!({}), rect), + ("object.textWrap.release", json!({}), rect), + ("object.textWrap.options", json!({}), rect), + ("variable.bind", json!({"variable": "v"}), rect), + ("variable.unbind", json!({}), rect), + ("addon.cleanup.breakText", json!({}), text), + ("addon.cleanup.combineText", json!({}), text), + ("addon.cleanup.joinText", json!({}), text), + ("addon.cleanup.removeBreaks", json!({"kind": "line"}), text), + ("addon.cleanup.resetTextTransform", json!({}), text), + ("addon.cleanup.alignInPlace", json!({"align": "left"}), text), + ]; + for (cmd, params, selected) in cases { + let left_out = outcome(cmd, ¶ms, selected, false); + let mut null = params.clone(); + null["ids"] = serde_json::Value::Null; + let with_null = outcome(cmd, &null, selected, false); + if with_null != left_out { + failures.push(format!("{cmd}: null gives {:.160?}; left out gives {:.160?}", with_null.0, left_out.0)); + } + } + let print_fixes = |p: &serde_json::Value| { + let mut s = Session::new(); + s.execute("file.new", &json!({"width": 200, "height": 200, "colorMode": "cmyk", "created": null})).unwrap(); + s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap(); + let r = s.execute("addon.color.printFixes", p).map_err(|e| e.to_string()); + let st = s.doc().unwrap(); + (r, (*st.doc).clone(), st.selection.clone(), st.history.undo.len()) + }; + let (left_out, with_null) = (print_fixes(&json!({"inkLimit": 200})), print_fixes(&json!({"inkLimit": 200, "ids": null}))); + if with_null != left_out { + failures.push(format!("addon.color.printFixes: null gives {:.160?}; left out gives {:.160?}", with_null.0, left_out.0)); + } + assert!(failures.is_empty(), "{} fail:\n{}", failures.len(), failures.join("\n")); +} + /// With no document open, every command that takes `ids` refuses a list with an entry that is not /// a non-negative integer, and a value that is not a list, with the same error as with a document /// open (`effect.list` and `links.findFiles` run without a document).