mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-03 06:30:00 -06:00
Fix #3965: id works as ids of one with nothing selected wherever params take ids?|id?
This commit is contained in:
committed by
bflatastic
parent
9090afea1c
commit
8c5714a260
@@ -436,8 +436,11 @@ fn snap_xf(b: Rect, half: bool) -> Affine {
|
||||
}
|
||||
|
||||
fn pixel_perfect(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let ids: Vec<NodeId> =
|
||||
if p.get("ids").is_some() { super::edit::outermost(&s.doc()?.doc, targets(s, p)?) } else { super::edit::selected_roots(s)? };
|
||||
let ids: Vec<NodeId> = if p.get("ids").is_some() || p.get("id").is_some() {
|
||||
super::edit::outermost(&s.doc()?.doc, targets(s, p)?)
|
||||
} else {
|
||||
super::edit::selected_roots(s)?
|
||||
};
|
||||
let n = ids.len();
|
||||
s.edit("Make Pixel Perfect", |d, _| {
|
||||
for id in &ids {
|
||||
|
||||
@@ -586,7 +586,7 @@ fn set_gradient_geom(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
let stroke = edits_stroke(s, p, item, !s.fill_active, C)?;
|
||||
let item = item.of_kind(s, !stroke);
|
||||
let targets = match item {
|
||||
ItemTarget::Top => leaf_targets(s, &ids_param(p, "ids").map_or_else(|| selected_roots(s), |ids| Ok(outermost(&s.doc()?.doc, ids)))?)?,
|
||||
ItemTarget::Top => leaf_targets(s, &super::given_objects(p).map_or_else(|| selected_roots(s), |ids| Ok(outermost(&s.doc()?.doc, ids)))?)?,
|
||||
_ => item.targets(s, p)?,
|
||||
};
|
||||
if targets.is_empty() {
|
||||
|
||||
@@ -367,6 +367,14 @@ pub(crate) fn id_param(p: &Value, key: &str) -> Option<NodeId> {
|
||||
pub(crate) fn ids_param(p: &Value, key: &str) -> Option<Vec<NodeId>> {
|
||||
p.get(key).and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_u64).map(NodeId).collect())
|
||||
}
|
||||
/// The objects `p` gives: its `ids`, else its `id` as `ids` of one (as [`targets`] reads them);
|
||||
/// `None` when it gives neither.
|
||||
pub(crate) fn given_objects(p: &Value) -> Option<Vec<NodeId>> {
|
||||
match p.get("ids").filter(|v| !v.is_null()) {
|
||||
Some(_) => ids_param(p, "ids"),
|
||||
None => id_param(p, "id").map(|id| vec![id]),
|
||||
}
|
||||
}
|
||||
/// `v` as an index, a count or a byte offset: past `usize::MAX` reads as `usize::MAX`, past the end of any list.
|
||||
pub(crate) fn index(v: u64) -> usize {
|
||||
usize::try_from(v).unwrap_or(usize::MAX)
|
||||
|
||||
@@ -684,7 +684,7 @@ fn default_paint(s: &mut Session, p: &Value) -> Result<Value> {
|
||||
/// active item of the proxy's kind is recoloured in each selected object's own stack instead.
|
||||
fn proxy_recolor(s: &mut Session, p: &Value, cmd: &str, label: &str, f: &dyn Fn(Color) -> Color) -> Result<Value> {
|
||||
let stroke = stroke_param(s, p, cmd)?;
|
||||
let ids = match ids_param(p, "ids") {
|
||||
let ids = match super::given_objects(p) {
|
||||
Some(ids) => ids,
|
||||
None => super::edit::selected_roots(s)?,
|
||||
};
|
||||
|
||||
+23
-12
@@ -1762,11 +1762,12 @@ impl Session {
|
||||
self.check_params(id, params)?;
|
||||
// Clear's menu needs a selection; an agent supplying explicit targets only needs a
|
||||
// document. The command validates those targets before making any edit.
|
||||
let given = given_ids(spec.params, params);
|
||||
let enabled = if id == "edit.clear" && params.get("ids").is_some() {
|
||||
cmd::has_doc(self)
|
||||
} else if let Some(ids) = given_ids(spec.params, params) {
|
||||
// Objects given as `ids` stand in for the selection: the command is enabled as it is
|
||||
// with them selected (#1575).
|
||||
} else if let Some(ids) = given.clone() {
|
||||
// Objects given as `ids` (or `id`) stand in for the selection: the command is enabled
|
||||
// as it is with them selected (#1575).
|
||||
self.enabled_with_selection(spec, ids)
|
||||
} else {
|
||||
(spec.enabled)(self)
|
||||
@@ -1782,13 +1783,18 @@ impl Session {
|
||||
self.journal_query = false;
|
||||
self.batch_stash = None;
|
||||
}
|
||||
// A selection-only command given `ids` selects them first, in the same undo step (#2130);
|
||||
// when the command then fails, the selection (and the current layer selecting them made
|
||||
// current) goes back to how it was, as when the ids are refused.
|
||||
let select_first = SELECTS_GIVEN_IDS.contains(&id).then(|| params.get("ids")).flatten();
|
||||
let selected_before = select_first.and(self.active()).map(|d| (d.uid, d.selection.clone(), d.active_layer));
|
||||
// A selection-only command given `ids` (or `id`) selects them first, in the same undo step
|
||||
// (#2130); when the command then fails, the selection (and the current layer selecting
|
||||
// them made current) goes back to how it was, as when the ids are refused.
|
||||
let select_first = SELECTS_GIVEN_IDS
|
||||
.contains(&id)
|
||||
.then(|| {
|
||||
params.get("ids").filter(|v| !v.is_null()).cloned().or_else(|| given.map(|ids| ids.iter().map(|i| i.0).collect::<Vec<_>>().into()))
|
||||
})
|
||||
.flatten();
|
||||
let selected_before = select_first.as_ref().and(self.active()).map(|d| (d.uid, d.selection.clone(), d.active_layer));
|
||||
let run = |s: &mut Session| {
|
||||
if let Some(ids) = select_first {
|
||||
if let Some(ids) = &select_first {
|
||||
s.execute("select.set", &serde_json::json!({ "ids": ids }))?;
|
||||
}
|
||||
let r = (spec.run)(s, params);
|
||||
@@ -2784,8 +2790,13 @@ pub(crate) const SELECTS_GIVEN_IDS: &[&str] = &[
|
||||
];
|
||||
|
||||
/// The non-empty `ids` given to a command whose params description names `ids` as a key (the
|
||||
/// key callers outside the app may pass it, [`cmd::params::takes_key`]).
|
||||
/// key callers outside the app may pass it, [`cmd::params::takes_key`]), or its `id` as `ids` of
|
||||
/// one when the description names that key too and no `ids` are given, as [`cmd::targets`] reads
|
||||
/// them.
|
||||
fn given_ids(description: &'static str, params: &Value) -> Option<Vec<NodeId>> {
|
||||
let ids = cmd::ids_param(params, "ids").filter(|ids| !ids.is_empty())?;
|
||||
cmd::params::takes_key(description, "ids").then_some(ids)
|
||||
let by_id = params.get("ids").is_none_or(Value::is_null);
|
||||
if !cmd::params::takes_key(description, "ids") || (by_id && !cmd::params::takes_key(description, "id")) {
|
||||
return None;
|
||||
}
|
||||
cmd::given_objects(params).filter(|ids| !ids.is_empty())
|
||||
}
|
||||
|
||||
@@ -53,6 +53,78 @@ fn only_an_ids_key_counts() {
|
||||
// A word in what the description says about another key isn't the key.
|
||||
assert_eq!(given_ids("{name: of the swatch the ids of the selection take}", &p), None);
|
||||
assert_eq!(given_ids("{ids?: [id]}", &json!({"ids": []})), None);
|
||||
// `id` is `ids` of one where the description names both keys, and `ids` aren't given.
|
||||
let one = json!({"id": 3});
|
||||
assert_eq!(given_ids("{ids?|id?}", &one), Some(vec![NodeId(3)]));
|
||||
assert_eq!(given_ids("{id?, ids?: [id…] (instead of id)}", &one), Some(vec![NodeId(3)]));
|
||||
assert_eq!(given_ids("{ids?: [id…]}", &one), None, "a description without `id`");
|
||||
assert_eq!(given_ids("{id?: dimension id}", &one), None, "a description without `ids`");
|
||||
assert_eq!(given_ids("{ids?|id?}", &json!({"id": "a1"})), None, "not an object id");
|
||||
assert_eq!(given_ids("{ids?|id?}", &json!({"id": 3, "ids": [4]})), Some(vec![NodeId(4)]), "`ids` first");
|
||||
assert_eq!(given_ids("{ids?|id?}", &json!({"id": 3, "ids": null})), Some(vec![NodeId(3)]));
|
||||
}
|
||||
|
||||
/// `{"id": x}` with params besides it, and the same as `{"ids": [x]}`.
|
||||
fn id_and_ids(cmd: &str, x: u64, d: u64) -> (serde_json::Value, serde_json::Value) {
|
||||
let (mut one, mut list) = (params_for(cmd, d), params_for(cmd, d));
|
||||
one["id"] = json!(x);
|
||||
list["ids"] = json!([x]);
|
||||
(one, list)
|
||||
}
|
||||
|
||||
/// [`doc_with_nested_groups`] with point type `Ab` added, nothing selected → its session and the
|
||||
/// type's id.
|
||||
fn doc_with_type() -> (Session, [u64; 4], u64) {
|
||||
let (mut s, ids) = doc_with_nested_groups();
|
||||
let t = s.execute("text.create", &json!({"x": 10, "y": 120, "text": "Ab", "size": 20})).unwrap()["id"].as_u64().unwrap();
|
||||
s.execute("select.none", &json!({})).unwrap();
|
||||
(s, ids, t)
|
||||
}
|
||||
|
||||
/// What `cmd` gives in a fresh [`doc_with_type`] (nothing selected) with `params`: its result or
|
||||
/// error, and the document after it.
|
||||
fn run_fresh(
|
||||
cmd: &str,
|
||||
params: &serde_json::Value,
|
||||
) -> (std::result::Result<serde_json::Value, String>, Option<std::sync::Arc<vectorcraft_doc::Document>>) {
|
||||
let (mut s, ..) = doc_with_type();
|
||||
let r = s.execute(cmd, params).map_err(|e| e.to_string());
|
||||
(r, s.doc().ok().map(|d| d.doc.clone()))
|
||||
}
|
||||
|
||||
/// Every command whose params description names both `id` and `ids` (`{ids?|id?}`, `{id?, ids?:
|
||||
/// … (instead of id)}`), with nothing selected: `id` does what `ids` of that one object does (the
|
||||
/// same result or the same error, and the same document), for objects of the document and for an
|
||||
/// id the document doesn't have. Before, the commands that need a selection refused `id` as
|
||||
/// "nothing selected" and took `ids`.
|
||||
#[test]
|
||||
fn id_does_what_ids_of_one_does_with_nothing_selected() {
|
||||
let (fresh, [a, inner, _, d], t) = doc_with_type();
|
||||
let (mut checked, mut failures, mut refused, mut refused_ran) = (vec![], vec![], vec![], vec![]);
|
||||
for spec in command_specs() {
|
||||
if !cmd::params::takes_key(spec.params, "ids") || !cmd::params::takes_key(spec.params, "id") {
|
||||
continue;
|
||||
}
|
||||
// Those the menus refuse with nothing selected are the ones that refused `id`.
|
||||
if (spec.enabled)(&fresh).is_err() {
|
||||
refused.push(spec.id);
|
||||
}
|
||||
// A rectangle, a group, type, and an id the document doesn't have.
|
||||
for x in [a, inner, t, 999_999] {
|
||||
let (one, list) = id_and_ids(spec.id, x, d);
|
||||
let (with_id, with_ids) = (run_fresh(spec.id, &one), run_fresh(spec.id, &list));
|
||||
if with_id != with_ids {
|
||||
failures.push(format!("{} {x}: with id {:.160}; with ids {:.160}", spec.id, format!("{:?}", with_id.0), format!("{:?}", with_ids.0)));
|
||||
} else if refused.last() == Some(&spec.id) && x != 999_999 && with_id.0.is_ok() && !refused_ran.contains(&spec.id) {
|
||||
refused_ran.push(spec.id);
|
||||
}
|
||||
}
|
||||
checked.push(spec.id);
|
||||
}
|
||||
assert!(failures.is_empty(), "{} differ:\n{}", failures.len(), failures.join("\n"));
|
||||
// The commands that refused `id` with nothing selected (29), and those of them that run on
|
||||
// these objects (14: the rest need other art, as with `ids`).
|
||||
assert!(refused.len() >= 26 && refused_ran.len() >= 14, "{refused_ran:?} ran of {refused:?}; checked {}", checked.len());
|
||||
}
|
||||
|
||||
/// Every command that names `ids` and is refused with nothing selected: given the object as `ids`
|
||||
|
||||
+4
-2
@@ -347,8 +347,10 @@ currently describes them in prose, so MCP does not guess schemas or silently rem
|
||||
|
||||
Coordinates are points in document space: y points down, the origin is the first artboard's top-left, and a new
|
||||
document is 612 × 792 (US Letter). New objects become the selection. Most commands act on the selection or on
|
||||
explicit `ids`. 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.
|
||||
explicit `ids`, which stand in for the selection, so they work with nothing selected. Where a command's params say
|
||||
`ids?|id?` (or `id?, ids?: … (instead of id)`), `id` is `ids` of one object: `{"id": 7}` does what `{"ids": [7]}`
|
||||
does, with or without a selection. 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.
|
||||
|
||||
Paint values (`fill`, `stroke`) accept `"#rrggbb"`, `"none"`, `[r,g,b]` (0..1), `{"c","m","y","k"}`, `{"gray"}`,
|
||||
or a full `paint.setFill` params object (`{"gradient": …}`, `{"swatch": "name"}`).
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user