From 9c120c735ebd51d30f477e4c901bc1a7cfffcafe Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 17:21:56 -0800 Subject: [PATCH] Keyboard Shortcuts: commands a key cannot run without a value are not listed Edit > Keyboard Shortcuts listed commands whose params are all optional but that do nothing or fail when a key runs them with none, such as Transparency (transparency.set) and Opacity Mask Options (transparency.setOpacityMask). These are now left out while they have no shortcut, along with Align and Distribute, Delete Swatch, Move Swatch, Spot Options, Adjust Color Balance, the library and data set loaders, stroke.set, layer.setProps and menu queries that no menu item runs. A key saved for one of them before keeps it listed so it can be cleared. A shortcut for a command whose menu bar items all give it the same params now runs what a click on that item runs, so Clean Up..., Print Ink Fixes... and Find and Replace... open their dialogs. --- crates/ui-egui/src/shortcut_editor.rs | 156 +++++++++++++++++++++++--- crates/ui-egui/src/shortcuts.rs | 10 +- crates/ui-egui/src/shortcuts/tests.rs | 20 ++++ docs/mcp.md | 18 ++- 4 files changed, 181 insertions(+), 23 deletions(-) diff --git a/crates/ui-egui/src/shortcut_editor.rs b/crates/ui-egui/src/shortcut_editor.rs index f1d5cabb0..c8fac1790 100644 --- a/crates/ui-egui/src/shortcut_editor.rs +++ b/crates/ui-egui/src/shortcut_editor.rs @@ -392,36 +392,85 @@ fn listed(overrides: &Overrides, e: &Entry) -> bool { !needs_a_value(&e.key) || overrides.get(&crate::keymap::stored_key(&e.key)).is_some_and(|t| !t.is_empty()) } -/// Is `key` a command with a menu path and no default shortcut that a key press can't run: no -/// item of the menu bar runs it without params, it opens no dialog when run without them (as the -/// Variables panel's commands do), and its params doc names a field a call must give (File › -/// Document Setup › Units, `{units}`)? -fn needs_a_value(key: &str) -> bool { - fn run_bare(items: &[menus::Item], out: &mut HashSet<&'static str>) { +/// Commands whose params are all optional that do nothing, or fail, when a key press runs them +/// with none, and that no item of the menu bar runs: Transparency and Opacity Mask Options need +/// a value to set, Align and Distribute a direction, Delete Swatch and Move Swatch a swatch name, +/// and the library and data set loaders a file, which their panel menu items ask for. +const NEEDS_A_PARAM: [&str; 16] = [ + "transparency.set", + "transparency.setOpacityMask", + "stroke.set", + "layer.setProps", + "object.align", + "object.distribute", + "swatch.delete", + "swatch.move", + "swatch.spotOptions", + "edit.colors.adjustBalance", + "swatch.library.load", + "graphicStyle.loadLibrary", + "symbol.loadLibrary", + "brush.loadLibrary", + "dataset.import", + "variable.loadLibrary", +]; + +/// The (label, params) of each menu bar item, by command id. +fn menu_bar_items() -> &'static HashMap<&'static str, Vec<(&'static str, Value)>> { + fn walk(items: &[menus::Item], out: &mut HashMap<&'static str, Vec<(&'static str, Value)>>) { for it in items { match it { - menus::Item::Cmd(_, id, p) if p.as_object().is_none_or(|o| o.is_empty()) => { - out.insert(id); - } - menus::Item::Sub(_, children) => run_bare(children, out), + menus::Item::Cmd(label, id, p) => out.entry(*id).or_default().push((*label, p.clone())), + menus::Item::Sub(_, children) => walk(children, out), _ => {} } } } + static ITEMS: OnceLock>> = OnceLock::new(); + ITEMS.get_or_init(|| { + let mut out = HashMap::new(); + for (_, items) in menus::menu_tree() { + walk(&items, &mut out); + } + out + }) +} + +fn is_bare(p: &Value) -> bool { + p.as_object().is_none_or(|o| o.is_empty()) +} + +/// What a shortcut without params for command `id` runs when no menu bar item runs `id` without +/// params and every item that runs it gives the same params: what a click on that item runs +/// ([`menus::click_target`]), so Print Ink Fixes runs with its menu item's params and Clean Up… +/// opens its dialog. +pub(crate) fn menu_target(id: &str) -> Option<(String, Value)> { + let items = menu_bar_items().get(id)?; + let (label, p) = items.first().filter(|(_, p)| !is_bare(p))?; + items.iter().all(|(_, q)| q == p).then(|| menus::click_target(label, id, p)) +} + +/// Is `key` a command with no default shortcut that a key press can't run: no item of the menu +/// bar runs it without params or with one set of params ([`menu_target`]), it opens no dialog when +/// run without them (as the Variables panel's commands do), and it is a query, whose answer a key +/// press drops (Select All Unused), a command of [`NEEDS_A_PARAM`], or a menu command whose params +/// doc names a field a call must give (File › Document Setup › Units, `{units}`)? +fn needs_a_value(key: &str) -> bool { static KEYS: OnceLock> = OnceLock::new(); KEYS.get_or_init(|| { - let mut bare = HashSet::new(); - for (_, items) in menus::menu_tree() { - run_bare(&items, &mut bare); - } + // A menu item that runs the command itself with params of its own counts only when the + // params doc names no required field: Edit › Find Next gives `find` empty, which it rejects. + let runs = |c: &vectorcraft_engine::CommandSpec| { + menu_bar_items().get(c.id).is_some_and(|ps| ps.iter().any(|(_, p)| is_bare(p))) + || menu_target(c.id).is_some_and(|(id, _)| id != c.id || !names_a_required_field(c.params)) + }; vectorcraft_engine::command_specs() .iter() .filter(|c| { - !c.menu.is_empty() - && c.shortcut.is_none() - && !bare.contains(c.id) + c.shortcut.is_none() + && !runs(c) && !crate::dialogs::variables::opens(c.id) - && names_a_required_field(c.params) + && (!c.journal || NEEDS_A_PARAM.contains(&c.id) || (!c.menu.is_empty() && names_a_required_field(c.params))) }) .map(|c| c.id) .collect() @@ -1465,6 +1514,77 @@ mod tests { } } + /// Commands whose params are all optional but that a key press can't run are not listed while + /// they have no shortcut: Transparency and Opacity Mask Options set nothing without a value, + /// Align needs a direction, Other Library… a file, and a query's answer (Select All Unused) + /// is dropped. A key saved for one before keeps it listed, where it can be cleared. + #[test] + fn commands_a_key_press_cannot_run_are_not_listed() { + let none = Overrides::new(); + // Edit › Find Next's menu item gives an empty `find`, which it rejects. + let others = ["swatch.unused", "graphicStyle.unused", "document.info", "view.proofSetup", "magicWand.set", "edit.findNext"]; + for id in NEEDS_A_PARAM.iter().chain(&others) { + let e = entry(id).unwrap_or_else(|| panic!("{id} has an entry")); + assert!(!listed(&none, e), "{id}"); + assert_eq!(default_of(id), None, "{id}"); + } + let mut ov = Overrides::new(); + crate::keymap::set_override(&mut ov, "transparency.set", "Cmd+Alt+Shift+F10"); + assert!(entry("transparency.set").is_some_and(|e| listed(&ov, e))); + let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default()); + for id in ["transparency.set", "transparency.setOpacityMask"] { + let list = run_command(&mut app, "shortcuts.list", &json!({ "query": id })).unwrap().unwrap(); + assert_eq!(list, json!([]), "{id}"); + } + // Commands that work without params stay listed: panel menu commands that act on the + // selection, toggles, and a menu item that runs with params of its own (Clean Up…). + for id in [ + "transparency.releaseOpacityMask", + "transparency.toggleNewMasksClipping", + "object.path.cleanUp", + "addon.color.printFixes", + "edit.findReplace", + ] { + assert!(entry(id).is_some_and(|e| listed(&none, e)), "{id}"); + } + assert_eq!(menu_target("object.path.cleanUp").map(|(id, _)| id).as_deref(), Some("ui.paramDialog")); + assert_eq!( + menu_target("addon.color.printFixes").map(|(id, p)| (id, p["command"].clone(), p["params"]["inkLimit"].clone())), + Some(("ui.paramDialog".into(), json!("addon.color.printFixes"), json!(300))) + ); + assert_eq!(menu_target("edit.findReplace").map(|(id, _)| id).as_deref(), Some("ui.paramDialog")); + assert_eq!(menu_target("object.align"), None, "seven items, each with its own params"); + assert_eq!(menu_target("edit.undo"), None, "runs without params"); + } + + /// Every command the editor lists, run as a key press runs it on a document with one or with + /// several objects selected, reports no missing param. + #[test] + fn listed_commands_run_from_a_key_without_a_missing_param() { + use vectorcraft_testkit::fixtures::Fixture; + // These need a state the fixtures lack: text being edited, a pattern being edited, an + // object linked to a graphic style, a plug-ins folder. + let context = ["text.insertInline", "pattern.options", "graphicStyle.redefine", "plugin.reload"]; + let none = Overrides::new(); + let mut failures = vec![]; + for e in entries().iter().filter(|e| !e.is_tool && !e.key.starts_with("panel:") && listed(&none, e)) { + if ["file.", "app.", "help."].iter().any(|p| e.key.starts_with(p)) || context.contains(&e.key.as_str()) { + continue; + } + for fx in [Fixture::Single, Fixture::Multi] { + let mut app = VectorcraftApp::new(fx.session(), Default::default()); + app.ui.status.clear(); + let (id, p) = menu_target(&e.key).unwrap_or_else(|| (e.key.clone(), json!({}))); + menus::invoke(&mut app, &id, p); + let s = &app.ui.status; + if s.starts_with("invalid parameters for") && (s.contains(": give ") || s.contains(": missing ")) { + failures.push(format!("{} [{fx:?}]: {s}", e.key)); + } + } + } + assert!(failures.is_empty(), "{failures:#?}"); + } + /// #2991: Enter Isolation Mode has no default key but runs on the selection, so it belongs in /// Edit › Keyboard Shortcuts. #[test] diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index e8e5d07fe..e9e2e3d2b 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -400,6 +400,7 @@ fn single_key_shortcuts(app: &mut VectorcraftApp, ctx: &egui::Context) { let Some(hit) = hits.iter().find(|c| !c.id.starts_with("tool:")).or(hits.first()) else { continue }; match hit.id.strip_prefix("tool:") { Some(tool) => app.select_tool(tool), + None if let Some((id, p)) = menu_target(hit) => crate::menus::invoke(app, &id, p), None if hit.params.is_null() => { let _ = app.run(&hit.id, json!({})); } @@ -778,6 +779,12 @@ fn dispatch(app: &mut VectorcraftApp, ctx: &egui::Context, accept: impl FnMut(&H }) } +/// What a shortcut without params runs when its command's menu bar item gives params +/// ([`crate::shortcut_editor::menu_target`]): what a click on that item runs. +fn menu_target(c: &CommandRef) -> Option<(String, serde_json::Value)> { + c.params.as_object().is_none_or(serde_json::Map::is_empty).then(|| crate::shortcut_editor::menu_target(&c.id)).flatten() +} + /// Params used when a shortcut invokes its command. The named panel function keys retain their /// toggle action when their bindings are reassigned. fn shortcut_params(c: &CommandRef) -> serde_json::Value { @@ -818,7 +825,8 @@ fn run_hit(app: &mut VectorcraftApp, ctx: &egui::Context, c: &CommandRef) { // every artboard instead of the art. crate::panels::artboards::select_all(app); } else { - crate::menus::invoke(app, &c.id, shortcut_params(c)); + let (id, p) = menu_target(c).unwrap_or_else(|| (c.id.clone(), shortcut_params(c))); + crate::menus::invoke(app, &id, p); } } diff --git a/crates/ui-egui/src/shortcuts/tests.rs b/crates/ui-egui/src/shortcuts/tests.rs index 27e0f5bbf..d24a5ad3e 100644 --- a/crates/ui-egui/src/shortcuts/tests.rs +++ b/crates/ui-egui/src/shortcuts/tests.rs @@ -225,6 +225,26 @@ fn presses_follow_the_users_shortcuts() { assert!(runs_a_shortcut("=", "Equal", cmd()), "the defaults again"); } +/// A user's shortcut for a command whose menu bar item gives params runs what a click on that item +/// runs: Clean Up…, Print Ink Fixes… and Find and Replace… open their dialogs instead of running +/// without the values the dialog asks for. +#[test] +fn a_shortcut_runs_what_its_menu_item_runs() { + let chord = vec![ + egui::Event::ModifiersChanged(cmd() | Modifiers::ALT | Modifiers::SHIFT), + egui::Event::Key { key: Key::F9, physical_key: None, pressed: true, repeat: false, modifiers: cmd() | Modifiers::ALT | Modifiers::SHIFT }, + ]; + for id in ["object.path.cleanUp", "addon.color.printFixes", "edit.findReplace"] { + let mut app = VectorcraftApp::new(vectorcraft_testkit::fixtures::Fixture::Single.session(), Default::default()); + crate::keymap::set_override(&mut app.ui.shortcut_overrides, id, "Cmd+Alt+Shift+F9"); + app.ui.status.clear(); + frame(&mut app, chord.clone()); + crate::shortcut_editor::sync(&crate::state::UiState::default()); + let d = app.ui.dialog.as_ref().unwrap_or_else(|| panic!("{id} opens its dialog: {}", app.ui.status)); + assert_eq!(d.fields["__command"], json!(id), "{:?}", d.fields); + } +} + /// #2094: Cmd+Shift+O is Type › Create Outlines (not File › Open's Cmd+O, not the Artboard /// tool's Shift+O), with the Selection tool or while the Type tool edits the text. #[test] diff --git a/docs/mcp.md b/docs/mcp.md index 703a8c416..79afd672b 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -3081,11 +3081,21 @@ the Align To choice, and while there is a key object or a key guide they send th `spacing`. Edit › Keyboard Shortcuts lists them under Align, with no default shortcut, and `shortcuts.set {id, shortcut}` gives one a shortcut. -Edit › Keyboard Shortcuts and `shortcuts.list` leave out a command with a menu path and no default shortcut when its -params doc has a required field (one with no `?` and no `=` default), no item of the menu bar runs it without params, -and it opens no dialog when run without them, such as `document.setUnits {units}` (File › Document Setup › Units). +Edit › Keyboard Shortcuts and `shortcuts.list` leave out a command with no default shortcut when a key press cannot +run it: no item of the menu bar runs it, it opens no dialog when run without params, and it is one of these: + +- a query with a menu path, such as `swatch.unused`; +- a command with a menu path whose params doc has a required field (one with no `?` and no `=` default), such as + `document.setUnits {units}` (File › Document Setup › Units); +- a command that sets nothing or fails without a param: `transparency.set`, `transparency.setOpacityMask`, + `stroke.set`, `layer.setProps`, `object.align`, `object.distribute`, `swatch.delete`, `swatch.move`, + `swatch.spotOptions`, `edit.colors.adjustBalance`, `swatch.library.load`, `graphicStyle.loadLibrary`, + `symbol.loadLibrary`, `brush.loadLibrary`, `dataset.import` and `variable.loadLibrary`. + They list such a command while the user has changed its shortcut (`overridden: true` in `shortcuts.list`), and -`shortcuts.set` can change or clear it. +`shortcuts.set` can change or clear it. When every menu bar item that runs a command gives it the same params, a +shortcut for that command runs what a click on the item runs: Object › Path › Clean Up…, Print Ink Fixes… and Edit › +Find and Replace… open their dialogs. The desktop app adds its active artboard, the one the Artboards panel highlights, to the `object.align`, `object.distribute`, `object.distributeSpacing` and `select.allOnArtboard` calls it runs without `artboard`, as it does