mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-17 04:39:54 +02:00
feat(commands): find the Elliptical Marquee by 'ellipse' or 'marquee' (#5140)
* feat(commands): find the Elliptical Marquee by 'ellipse' or 'marquee' (#5060) The Elliptical and Single Row/Column Marquee are options of select.rect ("ellipse":true, "line"), but the command-list filters only looked at the id and the label ("Rectangular Selection"), so 'ellipse' and 'marquee' found nothing. Commands can now carry search keywords. command.list / engine.commands rows include them as "keywords", and the three filters (CLI commands --filter, the engine.commands RPC and the MCP command_list) share one matcher over id, label and keywords. select.rect gets the Photoshop marquee names as keywords. * Command list replies: document the optional keywords key The reply-shape test from #3684 pins the exact keys of each row; select.rect now also carries keywords, so the test and docs/control-protocol.md allow that one optional string.
This commit is contained in:
parent
3888e95e47
commit
56c5b355a4
|
|
@ -541,9 +541,7 @@ fn commands(a: &Args, out: &mut dyn Write) -> R {
|
|||
.map(|v| v.iter().collect::<Vec<_>>())
|
||||
.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);
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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}");
|
||||
|
|
|
|||
|
|
@ -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<String>,
|
||||
/// 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)))
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -143,10 +143,32 @@ pub fn command_specs() -> &'static [CommandSpec] {
|
|||
pub fn list_entry(s: &Session, c: &CommandSpec, reason: Option<String>) -> 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> {
|
||||
|
|
|
|||
|
|
@ -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<String> {
|
||||
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() {
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user