diff --git a/apps/photocraft-cli/src/lib.rs b/apps/photocraft-cli/src/lib.rs index a81cce61ce..b93d4ccc8f 100644 --- a/apps/photocraft-cli/src/lib.rs +++ b/apps/photocraft-cli/src/lib.rs @@ -541,9 +541,7 @@ fn commands(a: &Args, out: &mut dyn Write) -> R { .map(|v| v.iter().collect::>()) .unwrap_or_default() .into_iter() - .filter(|c| { - needle.as_ref().is_none_or(|n| format!("{} {}", c["id"].as_str().unwrap_or(""), c["label"].as_str().unwrap_or("")).to_lowercase().contains(n)) - }) + .filter(|c| needle.as_ref().is_none_or(|n| photocraft_engine::commands::list_entry_matches(c, n))) .collect(); if a.has("--json") { return print_json(out, &Value::Array(items.into_iter().cloned().collect()), false); diff --git a/apps/photocraft-cli/tests/cli.rs b/apps/photocraft-cli/tests/cli.rs index 231eb2c321..3c28a8196e 100644 --- a/apps/photocraft-cli/tests/cli.rs +++ b/apps/photocraft-cli/tests/cli.rs @@ -90,6 +90,11 @@ fn unknown_flags_are_usage_errors() { // Every flag still works in both forms. let (out, _) = ok(bin().args(["commands", "--json", "--filter=gaussian"])); assert!(out.contains("filter.blur.gaussianBlur"), "{out}"); + // The Elliptical Marquee is a `select.rect` option, found by its keywords (#5060). + for needle in ["ellipse", "marquee"] { + let (out, _) = ok(bin().args(["commands", "--filter", needle])); + assert!(out.contains("select.rect"), "{needle}: {out}"); + } ok(bin().args(["batch", "--actions"]).arg(&actions).arg(format!("--in={}", input.display())).arg("--out").arg(&out_dir).args([ "--format=jpg", "--quality", diff --git a/crates/automation/src/rpc.rs b/crates/automation/src/rpc.rs index 507010b9c8..dc01f0f2b4 100644 --- a/crates/automation/src/rpc.rs +++ b/crates/automation/src/rpc.rs @@ -91,17 +91,9 @@ impl Headless { let all = self.command_list(); Ok(match str_of(&p, "filter").map(str::to_lowercase) { None => all, - Some(n) => Value::Array( - all.as_array() - .into_iter() - .flatten() - .filter(|c| { - let hay = format!("{} {}", c["id"].as_str().unwrap_or(""), c["label"].as_str().unwrap_or("")); - hay.to_lowercase().contains(&n) - }) - .cloned() - .collect(), - ), + Some(n) => { + Value::Array(all.as_array().into_iter().flatten().filter(|c| photocraft_engine::commands::list_entry_matches(c, &n)).cloned().collect()) + } }) } "session.list" => Ok(self.session_list()), @@ -365,7 +357,7 @@ mod tests { /// Pins the reply shape documented in docs/control-protocol.md › Command list replies /// (#3684): a bare JSON array (not `{"commands": [...]}`), one object per command with - /// exactly these keys, from `engine.commands` (filtered or not) and `command.list`. + /// exactly these keys (plus an optional `keywords` string), from `engine.commands` (filtered or not) and `command.list`. #[test] fn command_list_replies_are_documented_array_of_objects() { let h = session(); @@ -377,8 +369,10 @@ mod tests { let items = reply.as_array().unwrap_or_else(|| panic!("{name} must reply with a bare array, got {reply}")); assert!(!items.is_empty(), "{name}"); for c in items { - let mut keys: Vec<&str> = c.as_object().unwrap().keys().map(String::as_str).collect(); + // `keywords` is optional: only commands with other search names carry it (#5060). + let mut keys: Vec<&str> = c.as_object().unwrap().keys().map(String::as_str).filter(|k| *k != "keywords").collect(); keys.sort_unstable(); + assert!(c.get("keywords").is_none_or(Value::is_string), "{name}: {c}"); assert_eq!(keys, ["disabledReason", "enabled", "id", "label", "menu", "needsDocument", "params", "shortcut"], "{name}: {c}"); assert!(c["id"].is_string() && c["label"].is_string() && c["params"].is_string() && c["enabled"].is_boolean(), "{c}"); assert!(c["needsDocument"].is_boolean(), "{c}"); diff --git a/crates/automation/src/server.rs b/crates/automation/src/server.rs index b31507b005..f115473c3e 100644 --- a/crates/automation/src/server.rs +++ b/crates/automation/src/server.rs @@ -205,7 +205,7 @@ pub struct SelectParams { #[derive(Debug, Default, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct ListParams { - /// Only commands whose id or label contains this text (case-insensitive). + /// Only commands whose id, label or keywords contain this text (case-insensitive). #[serde(default)] pub filter: Option, /// Only commands that can run right now. @@ -821,9 +821,8 @@ impl PhotocraftMcp { .unwrap_or_else(|| all.get("commands").and_then(Value::as_array).cloned().unwrap_or_default()) .into_iter() .filter(|c| { - let hay = - format!("{} {}", c.get("id").and_then(Value::as_str).unwrap_or(""), c.get("label").and_then(Value::as_str).unwrap_or("")).to_lowercase(); - needle.as_ref().is_none_or(|n| hay.contains(n)) && (!enabled_only || c.get("enabled").and_then(Value::as_bool).unwrap_or(true)) + needle.as_ref().is_none_or(|n| photocraft_engine::commands::list_entry_matches(c, n)) + && (!enabled_only || c.get("enabled").and_then(Value::as_bool).unwrap_or(true)) }) .collect(); Ok(ok_json(&Value::Array(items))) diff --git a/crates/automation/tests/mcp.rs b/crates/automation/tests/mcp.rs index ca34d9f915..7386f3d58c 100644 --- a/crates/automation/tests/mcp.rs +++ b/crates/automation/tests/mcp.rs @@ -167,6 +167,9 @@ async fn command_list_filters() { let blur = json_of(&call(&client, "command_list", json!({"filter": "blur"})).await); assert!(!blur.as_array().unwrap().is_empty() && blur.as_array().unwrap().len() < n_all); assert!(blur.as_array().unwrap().iter().all(|c| c["params"].is_string())); + // The Elliptical Marquee is a `select.rect` option, found by its keywords (#5060). + let marquee = json_of(&call(&client, "command_list", json!({"filter": "ellipse"})).await); + assert!(marquee.as_array().unwrap().iter().any(|c| c["id"] == "select.rect"), "{marquee}"); let enabled = json_of(&call(&client, "command_list", json!({"enabled_only": true})).await); assert!(enabled.as_array().unwrap().iter().any(|c| c["id"] == "file.new")); assert!(!enabled.as_array().unwrap().iter().any(|c| c["id"] == "layer.new.layer"), "needs a document"); diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 38a1b4d7e0..48ce4a9332 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -143,10 +143,32 @@ pub fn command_specs() -> &'static [CommandSpec] { pub fn list_entry(s: &Session, c: &CommandSpec, reason: Option) -> Value { let needs_document = s.active().is_none() && reason.as_deref().is_some_and(|r| ["no document", "no active document", "no open document"].iter().any(|p| r.starts_with(p))); - json!({ + let mut entry = json!({ "id": c.id, "label": c.label, "menu": c.menu, "shortcut": c.shortcut, "params": c.params, "enabled": reason.is_none(), "disabledReason": reason, "needsDocument": needs_document, - }) + }); + if let (Some(k), Some(o)) = (search_keywords(c.id), entry.as_object_mut()) { + o.insert("keywords".into(), json!(k)); + } + entry +} + +/// Other names a command is looked for by, beyond its id and label: the Photoshop tools and +/// options it stands for (#5060). `select.rect` is also the Elliptical and Single Row/Column +/// Marquee, which an agent searches for as "ellipse" or "marquee". +pub(crate) const SEARCH_KEYWORDS: &[(&str, &str)] = + &[("select.rect", "Rectangular Marquee, Elliptical Marquee (\"ellipse\":true), ellipse, oval, circle, Single Row/Column Marquee (\"line\")")]; + +/// [`SEARCH_KEYWORDS`] of command `id`, if it has any. +pub fn search_keywords(id: &str) -> Option<&'static str> { + SEARCH_KEYWORDS.iter().find(|(k, _)| *k == id).map(|(_, words)| *words) +} + +/// Does a [`list_entry`] row match a command-list filter (`commands --filter`, `engine.commands`, +/// MCP `command_list`)? Its id, label or keywords contain `needle`, ignoring case. +pub fn list_entry_matches(entry: &Value, needle: &str) -> bool { + let needle = needle.to_lowercase(); + ["id", "label", "keywords"].iter().any(|k| entry.get(*k).and_then(Value::as_str).is_some_and(|v| v.to_lowercase().contains(&needle))) } pub fn find(id: &str) -> Option<&'static CommandSpec> { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 83458f05a6..ee7dd3435b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -767,6 +767,38 @@ fn command_list_reports_enablement() { assert_eq!(find("layer.new.layer")["enabled"], false); } +/// The Elliptical and Single Row/Column Marquee are `select.rect` options: a command-list filter +/// for "ellipse" or "marquee" finds it through its keywords (#5060). +#[test] +fn command_list_filter_finds_the_marquees_by_keyword() { + let mut s = Session::new(); + let list = s.execute("command.list", json!({})).unwrap(); + let rows = list.as_array().unwrap(); + let found = |needle: &str| -> Vec { + rows.iter().filter(|c| crate::commands::list_entry_matches(c, needle)).map(|c| c["id"].as_str().unwrap().to_string()).collect() + }; + for needle in ["ellipse", "Elliptical Marquee", "marquee", "single row", "SELECT.RECT"] { + assert!(found(needle).contains(&"select.rect".to_string()), "{needle}: {:?}", found(needle)); + } + let rect = rows.iter().find(|c| c["id"] == "select.rect").unwrap(); + assert!(rect["keywords"].as_str().is_some_and(|k| k.contains("Elliptical Marquee")), "{rect}"); + // Commands without keywords don't get an empty field, and the filter still reads id and label. + assert!(rows.iter().find(|c| c["id"] == "file.new").unwrap().get("keywords").is_none()); + assert_eq!(found("gaussian blur"), ["filter.blur.gaussianBlur"]); + assert!(found("no command is called this").is_empty()); +} + +/// Every keyword entry names a real command, so a renamed id can't leave one behind. +#[test] +fn search_keywords_name_real_commands() { + for (id, words) in crate::commands::SEARCH_KEYWORDS { + assert!(crate::commands::find(id).is_some(), "keywords for unknown command {id}"); + assert!(!words.trim().is_empty(), "{id}"); + } + assert!(crate::commands::search_keywords("select.rect").is_some()); + assert!(crate::commands::search_keywords("no.such.command").is_none()); +} + /// With no document open, a document command says so instead of only `enabled: false` (#3664). #[test] fn command_list_tells_no_document_from_other_preconditions() { diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 771fec92e9..4fa1c60c6e 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -196,9 +196,10 @@ Each element describes one command, in registry order: | `enabled` | boolean | Whether the command can run in the current state | | `disabledReason` | string or `null` | Why the command can't run now (`null` when `enabled`) | | `needsDocument` | boolean | `true` when it is off because no document is open (#3664) | +| `keywords` | string, optional | Other names the command is found by (#5060), e.g. `select.rect`'s Elliptical Marquee; only present on commands that have them | Filtering narrows the same array without changing its elements: `serve`'s `engine.commands -{filter}` and the MCP tool's `filter` keep commands whose id or label contains the text +{filter}` and the MCP tool's `filter` keep commands whose id, label or keywords contain the text (case-insensitive), and the MCP tool's `enabled_only` keeps those with `enabled: true`. The desktop control channel's `engine.commands` takes no filter and always returns every command. The shape is pinned by a test in `crates/automation/src/rpc.rs` (#3684).