From c024fca8f3a5017809d8894ba51cede4d0e8d0c5 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 12 Nov 2026 01:22:15 -0800 Subject: [PATCH] =?UTF-8?q?Keyboard=20Shortcuts:=20a=20command=20that=20a?= =?UTF-8?q?=20menu=20bar=20item=20runs=20can=20take=20a=20shortcut,=20Set?= =?UTF-8?q?=20Corners=E2=80=A6=20among=20them?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A shortcut given to Set Corners… (`addon.pathtools.cornerSet`) ran nothing, on a single key or a chord. The keymap is built from the list in Edit › Keyboard Shortcuts, and that list left the command out. An engine command without a menu path of its own was listed only when it had a default key, or when it was an undoable command, not an alias, whose params doc ends each field with `?`. A UI command was listed only when it had a default key or a params doc of `{}` or `{path…}`, or was Rename. Set Corners… needs a radius, so it was left out. The same rules left out other commands that items of the menu bar run: - View › Perspective Grid's Lock Grid, Lock Station Point, Show Rulers and Snap to Grid, and Object › Perspective's Attach to Active Plane, Move Plane to Match Object and Edit Text. - Edit › Edit Custom Dictionary…, a query. - UI commands: Recolor Artwork…, the Edit menu's four preset dialogs, Apply SVG Filter…, Export As…, Export Selection…, Edit Original, Apply Bullets, Apply Numbers, the four library saves, and the Window menu's New Toolbar…, Double Column, Keep Tools Beside the Canvas, New Workspace…, Float in Window and Consolidate All Windows. Such a command is now listed when an item of the menu bar runs it as a key press would: with no params, or with the one set of params all its items give. `needs_a_value` already used this check, and it moves into `a_menu_bar_item_runs` so that both use it. The command is listed in the group named by the first part of its id, next to the commands of its family that were listed before (Set Corners… under Addon with Corner Tool, the grid toggles under Perspective). File › Export Selection… was the row of `document.exportSelection`, which no item of the menu bar runs. That row now runs `file.exportSelection`, as the menu item does, and `document.exportSelection` is listed as Document › Export Selection…. A shortcut already given to it keeps running it. The Window menu's Learn item gets no row, because the Learn panel's row already has its name. Tests: on F and on a chord, Set Corners…, Edit Custom Dictionary…, Recolor Artwork…, the four preset dialogs and Apply SVG Filter… open the dialog a click on their menu item opens. Snap to Grid turns off and on from F, and Export Selection… on F opens Export for Screens. Every command the menu bar runs this way is listed, none of them shares its group and label with another entry, and a shortcut on four of them is kept through a saved set's export and import. The other Object › Plug-ins dialogs, Select by Link Status… and Composite Fonts… already opened from a shortcut and are checked the same way. --- crates/ui-egui/src/shortcut_editor.rs | 142 ++++++++++++++++++++++---- crates/ui-egui/src/shortcuts/tests.rs | 67 ++++++++++-- docs/mcp.md | 24 +++++ 3 files changed, 209 insertions(+), 24 deletions(-) diff --git a/crates/ui-egui/src/shortcut_editor.rs b/crates/ui-egui/src/shortcut_editor.rs index 7ada99ee5c..36ddfe8cb5 100644 --- a/crates/ui-egui/src/shortcut_editor.rs +++ b/crates/ui-egui/src/shortcut_editor.rs @@ -449,17 +449,17 @@ fn fixed_entries() -> &'static (Vec, Vec) { .map(|t| Entry { key: format!("tool:{}", t.id), label: t.label.to_string(), group: "Tools".into(), is_tool: true }) .collect(); let mut seen = HashSet::new(); + // The commands listed only because an item of the menu bar runs them as a key press would + // (Set Corners…). + let mut by_the_menu_bar = HashSet::new(); for c in vectorcraft_engine::command_specs() { - // Commands outside the menus (panel menu items) are - // listed when a key alone can run them; queries and aliases never are. - let listed = if c.menu.is_empty() { - c.shortcut.is_some() || (c.journal && !vectorcraft_engine::cmd::is_alias(c.id) && runs_without_params(c.params)) - } else { - true - }; - if !listed || !seen.insert(c.id) { + let own = listed_on_its_own(c); + if !(own || a_menu_bar_item_runs(c.id, c.params)) || !seen.insert(c.id) { continue; } + if !own { + by_the_menu_bar.insert(c.id); + } let group = if c.menu.is_empty() { id_group(c.id) } else { c.menu.join(" › ") }; v.push(Entry { key: c.id.into(), label: c.label.into(), group, is_tool: false }); } @@ -472,13 +472,36 @@ fn fixed_entries() -> &'static (Vec, Vec) { // Each effect of the Effect menu comes next, by its own key (#3296): see [`snapshot`]. let mut tail = vec![]; for (id, label, sc, params) in menus::UI_COMMANDS { - // Rename has no default key, and a key press names the selection: it needs no `id`. - let key_runs_it = !sc.is_empty() || params.starts_with("{}") || params.starts_with("{path") || *id == "ui.rename"; - if !seen.insert(id) || !key_runs_it { + let own = ui_listed_on_its_own(id, sc, params); + if !seen.insert(id) || !(own || a_menu_bar_item_runs(id, params)) { continue; } + if !own { + by_the_menu_bar.insert(id); + } tail.push(Entry { key: id.to_string(), label: label.to_string(), group: id_group(id), is_tool: false }); } + // A command listed only because an item of the menu bar runs it can have the group and + // label of another entry. When that entry is an engine command that no item of the menu bar + // runs, the command the item runs keeps the name and the other entry is shown in the group + // of its id: File › Export Selection… runs `file.exportSelection`, as the menu item does, + // and `document.exportSelection` shows as Document › Export Selection…. Otherwise the + // command is left out: Window › Learn stays the Learn panel's entry. + let name = |e: &Entry| (e.group.clone(), e.label.clone()); + let new_names: HashSet<_> = v.iter().chain(&tail).filter(|e| by_the_menu_bar.contains(e.key.as_str())).map(name).collect(); + for e in v.iter_mut().chain(tail.iter_mut()) { + if new_names.contains(&name(e)) + && !by_the_menu_bar.contains(e.key.as_str()) + && vectorcraft_engine::find_command(&e.key).is_some() + && !menu_bar_items().contains_key(e.key.as_str()) + { + e.group = id_group(&e.key); + } + } + let names: HashSet<_> = v.iter().chain(&tail).filter(|e| !by_the_menu_bar.contains(e.key.as_str())).map(name).collect(); + let keep = |e: &Entry| !by_the_menu_bar.contains(e.key.as_str()) || !names.contains(&name(e)); + v.retain(keep); + tail.retain(keep); (v, tail) }) } @@ -694,17 +717,11 @@ pub(crate) fn menu_target(id: &str) -> Option<(String, Value)> { fn needs_a_value(key: &str) -> bool { static KEYS: OnceLock> = OnceLock::new(); KEYS.get_or_init(|| { - // 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.shortcut.is_none() - && !runs(c) + && !a_menu_bar_item_runs(c.id, c.params) && !crate::dialogs::variables::opens(c.id) && (!c.journal || NEEDS_A_PARAM.contains(&c.id) || (!c.menu.is_empty() && names_a_required_field(c.params))) }) @@ -714,6 +731,31 @@ fn needs_a_value(key: &str) -> bool { .contains(key) } +/// Does an item of the menu bar run command `id`, whose params doc is `params`, as a key press runs +/// it: with no params, or with the one set of params all its items give ([`menu_target`])? 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. +fn a_menu_bar_item_runs(id: &str, params: &str) -> bool { + menu_bar_items().get(id).is_some_and(|ps| ps.iter().any(|(_, p)| is_bare(p))) + || menu_target(id).is_some_and(|(target, _)| target != id || !names_a_required_field(params)) +} + +/// Is engine command `c` listed in Edit › Keyboard Shortcuts whether or not an item of the menu bar +/// runs it? A command with a menu path of its own is. One without (a panel menu item) is when a key +/// alone can run it: it has a default key, or it is an undoable command, not an alias, whose params +/// doc ends each field with `?`. +fn listed_on_its_own(c: &vectorcraft_engine::CommandSpec) -> bool { + !c.menu.is_empty() || c.shortcut.is_some() || (c.journal && !vectorcraft_engine::cmd::is_alias(c.id) && runs_without_params(c.params)) +} + +/// Is UI command `id` (default key `sc`, params doc `params`) listed in Edit › Keyboard Shortcuts +/// whether or not an item of the menu bar runs it? It is when it has a default key or a params doc +/// of `{}` or `{path…}`. Rename has no default key, and a key press names the selection: it needs +/// no `id`. +fn ui_listed_on_its_own(id: &str, sc: &str, params: &str) -> bool { + !sc.is_empty() || params.starts_with("{}") || params.starts_with("{path") || id == "ui.rename" +} + /// Does a menu bar item run the command with no params, so that it does what its label says and opens /// no dialog? A command no menu bar item runs (the Crop Image item runs `ui.cropImage`), a menu item /// that gives it params, a [`NEEDS_A_PARAM`] command and a field a call must give each rule it out. @@ -2310,6 +2352,70 @@ mod tests { assert_eq!(effective_in(&ov, "object.isolate").as_deref(), Some("Cmd+Alt+Shift+F11")); } + /// Every engine or UI command that an item of the menu bar runs as a key press runs it (without + /// params, or with the one set of params all its items give) is listed in Edit › Keyboard + /// Shortcuts, also when the command has no menu path of its own: Set Corners…, whose params doc + /// names a field a call must give, the Perspective Grid toggles, Edit Custom Dictionary…, a + /// query, and Recolor Artwork…, a UI command. Such a command is listed in the group of its id, + /// next to the commands of its family that were listed before. A shortcut given to one survives + /// a saved set's round trip. Find Next is left out: its menu item gives `find` empty, which the + /// command rejects. When another entry has the same group and label, the command the menu item + /// runs keeps the name if no menu item runs the other: File › Export Selection… is + /// `file.exportSelection`, and `document.exportSelection` moves to Document. Otherwise the + /// command is left out: Window › Learn is the Learn panel's entry, not `window.learn`. + #[test] + fn every_command_the_menu_bar_runs_can_be_given_a_shortcut() { + let none = Overrides::new(); + let mut missing: Vec<&str> = menu_bar_items() + .iter() + .filter(|(id, items)| items.iter().any(|(_, p)| is_bare(p)) || menu_target(id).is_some()) + .map(|(id, _)| *id) + .filter(|id| *id != "edit.findNext" && *id != "window.learn") + .filter(|id| vectorcraft_engine::find_command(id).is_some() || menus::UI_COMMANDS.iter().any(|c| c.0 == *id)) + .filter(|id| !entry(id).is_some_and(|e| listed(&none, &e))) + .collect(); + missing.sort_unstable(); + assert!(missing.is_empty(), "{missing:?}"); + assert!(entry("window.learn").is_none(), "Window › Learn is the Learn panel's entry"); + let shown = |id: &str| entry(id).map(|e| (e.group, e.label)); + assert_eq!(shown("file.exportSelection"), Some(("File".into(), "Export Selection…".into()))); + assert_eq!(shown("document.exportSelection"), Some(("Document".into(), "Export Selection…".into()))); + // No command listed only because an item of the menu bar runs it shares its group and label + // with another entry. + let own = |k: &str| { + vectorcraft_engine::command_specs() + .iter() + .find(|c| c.id == k) + .map(listed_on_its_own) + .or_else(|| menus::UI_COMMANDS.iter().find(|c| c.0 == k).map(|(id, _, sc, p)| ui_listed_on_its_own(id, sc, p))) + .unwrap_or(true) + }; + let (v, tail) = fixed_entries(); + let mut names: HashMap<(&str, &str), Vec<&str>> = HashMap::new(); + for e in v.iter().chain(tail) { + names.entry((e.group.as_str(), e.label.as_str())).or_default().push(e.key.as_str()); + } + let shared: Vec<_> = names.iter().filter(|(_, keys)| keys.len() > 1 && keys.iter().any(|k| !own(k))).collect(); + assert!(shared.is_empty(), "{shared:?}"); + for (id, group) in [ + (crate::addons::pathtools::set_corners::COMMAND, "Addon"), + ("perspective.grid.snap", "Perspective"), + (crate::dialogs::custom_dictionary::COMMAND, "Text"), + ("ui.recolorDialog", "Ui"), + ] { + let e = entry(id).unwrap_or_else(|| panic!("{id} is listed")); + assert_eq!(e.group, group, "{id}"); + } + for id in + [crate::addons::pathtools::set_corners::COMMAND, "perspective.grid.lock", crate::dialogs::custom_dictionary::COMMAND, "ui.recolorDialog"] + { + let mut ov = Overrides::new(); + assign(&mut ov, id, Some("Cmd+Alt+Shift+F11"), false).unwrap(); + let (_, back) = import_json(&export_json("Mine", &ov)).unwrap(); + assert_eq!(effective_in(&back, id).as_deref(), Some("Cmd+Alt+Shift+F11"), "{id}"); + } + } + /// `shortcuts.set`, `shortcuts.list` and `shortcuts.preset` reject a supplied param of the wrong /// type with an error naming it, and change nothing; an omitted or null param keeps its default. #[test] diff --git a/crates/ui-egui/src/shortcuts/tests.rs b/crates/ui-egui/src/shortcuts/tests.rs index 8d32a4de22..d598c6bb19 100644 --- a/crates/ui-egui/src/shortcuts/tests.rs +++ b/crates/ui-egui/src/shortcuts/tests.rs @@ -294,9 +294,13 @@ fn a_shortcut_runs_what_its_menu_item_runs() { /// Commands whose menu item opens a slider or a dialog before anything changes: the sliders of /// Smooth and Simplify, Offset Path, the Object › Transform dialogs, Average, Split Into Grid, -/// Vector Halftone, Rearrange All Artboards, Pathfinder Options, Arrange in Grid, Divide into -/// Dashes, Find Nearest Swatch, and four "…" items whose menu entry gives params. -const MENU_DIALOG_COMMANDS: [&str; 22] = [ +/// Vector Halftone, Rearrange All Artboards, Pathfinder Options, the Object › Plug-ins dialogs +/// (Arrange in Grid, Replace with Top Object, the size, opacity and sort dialogs of the Arrange +/// add-on, Set Corners…, Divide into Dashes, Find Nearest Swatch, the swatch legend, the type +/// tools, the select add-on and Select by Link Status…), four "…" items whose menu entry gives +/// params, Edit Custom Dictionary…, Composite Fonts…, Recolor Artwork…, the Edit menu's preset +/// dialogs and Apply SVG Filter…. +const MENU_DIALOG_COMMANDS: [&str; 39] = [ "object.path.smooth", "object.path.simplify", "object.path.offsetPath", @@ -313,12 +317,29 @@ const MENU_DIALOG_COMMANDS: [&str; 22] = [ "artboard.rearrange", "object.pathfinder.options", "addon.arrange.grid", + "addon.arrange.replace", + "addon.arrange.size", + "addon.arrange.randomizeOpacity", + "addon.arrange.sort", + crate::addons::pathtools::set_corners::COMMAND, crate::addons::pathtools::dashes::COMMAND, crate::addons::color::nearest::COMMAND, + crate::addons::color::legend::COMMAND, + crate::addons::typetools::COMMAND, + crate::addons::select::COMMAND, + crate::addons::select::link_status::COMMAND, "object.path.cleanUp", "object.mesh.create", "type.smartPunctuation", "edit.findReplace", + crate::dialogs::custom_dictionary::COMMAND, + crate::dialogs::composite_fonts::COMMAND, + "ui.recolorDialog", + "ui.flattenerPresetsDialog", + "ui.printPresetsDialog", + "ui.pdfPresetsDialog", + "ui.perspectivePresetsDialog", + "ui.svgFilterDialog", ]; /// F pressed without a modifier. @@ -342,8 +363,33 @@ fn undo_len(app: &VectorcraftApp) -> usize { app.session.active().unwrap().history.undo.len() } +/// View › Perspective Grid › Snap to Grid, whose command has no menu path of its own, turns off and +/// on again from a shortcut given to it, as its menu item does. +#[test] +fn a_shortcut_on_a_perspective_grid_toggle_runs_it() { + let snaps = |app: &VectorcraftApp| crate::menus::checked(app, "perspective.grid.snap", &serde_json::Value::Null); + let mut app = app_with_shortcut("perspective.grid.snap", "F"); + assert_eq!(snaps(&app), Some(true), "on by default"); + frame(&mut app, press_f()); + assert_eq!(snaps(&app), Some(false), "F turns it off: {}", app.ui.status); + frame(&mut app, vec![]); + frame(&mut app, press_f()); + assert_eq!(snaps(&app), Some(true), "and on again: {}", app.ui.status); +} + +/// A shortcut on File › Export Selection… does what a click on the menu item does: it adds the +/// selection to Export for Screens' assets and opens Export for Screens. +#[test] +fn a_shortcut_on_export_selection_opens_export_for_screens() { + let mut app = app_with_shortcut("file.exportSelection", "F"); + frame(&mut app, press_f()); + assert_eq!(open_dialog(&app).as_deref(), Some("exportForScreens"), "{}", app.ui.status); +} + /// A command given a key without a modifier runs what its menu item runs, as it does on a chord: -/// its slider or dialog opens and nothing changes until it is confirmed. +/// its slider or dialog opens and nothing changes until it is confirmed. The dialog is the one a +/// click on the command's menu item opens (its item without params, else its first item), also for +/// commands without a menu path of their own (Set Corners…, Recolor Artwork…). #[test] fn a_single_key_shortcut_opens_the_dialog_its_menu_item_opens() { let chord = vec![ @@ -364,8 +410,17 @@ fn a_single_key_shortcut_opens_the_dialog_its_menu_item_opens() { }; let by_key = opened("F", press_f()); let by_chord = opened("Cmd+Alt+Shift+F9", chord.clone()); - if by_key.is_none() || by_key != by_chord { - failures.push(format!("{id}: F opens {by_key:?}, the chord {by_chord:?}")); + let mut app = app_with_a_selected_rectangle(); + // Type › Composite Fonts… shows while East Asian options are shown. + app.session.prefs.show_east_asian_options = true; + let items: Vec<_> = crate::menus::menu_entries(&app).into_iter().filter(|e| e.command.as_deref() == Some(id)).collect(); + let item = items.iter().find(|e| e.params.as_object().is_none_or(|o| o.is_empty())).or(items.first()); + // Pathfinder Options… is an item of the Pathfinder panel's menu, which runs it without params. + let (menu_id, p) = item.map_or_else(|| (id.to_string(), json!({})), |item| crate::menus::click_target(&item.label, id, &item.params)); + crate::menus::invoke(&mut app, &menu_id, p); + let by_menu = app.ui.dialog.as_ref().map(|d| (d.kind.clone(), d.fields.get("__command").cloned())); + if by_key.is_none() || by_key != by_chord || by_chord != by_menu { + failures.push(format!("{id}: F opens {by_key:?}, the chord {by_chord:?}, the menu item {by_menu:?}")); } } assert!(failures.is_empty(), "{failures:#?}"); diff --git a/docs/mcp.md b/docs/mcp.md index 93ee31576b..a53916a5cf 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -4792,6 +4792,30 @@ They list such a command while the user has changed its shortcut (`overridden: t 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. +A command without a menu path of its own is listed when an item of the menu bar runs it with no params, or when every +item that runs it gives it the same params. It is listed in the group named by the first part of its id. These +commands are listed this way: + +- `addon.pathtools.cornerSet` (Object › Plug-ins › Path Precision Tools › Set Corners…, which opens its dialog), + under Addon; +- `perspective.grid.lock`, `perspective.grid.lockStation`, `perspective.grid.rulers` and `perspective.grid.snap` + (View › Perspective Grid), and `perspective.attach`, `perspective.editText` and `perspective.plane.matchObject` + (Object › Perspective), under Perspective; +- `text.spelling.dictionary` (Edit › Edit Custom Dictionary…), under Text; +- the UI commands `ui.recolorDialog` (Edit › Edit Colors › Recolor Artwork…), `ui.flattenerPresetsDialog`, + `ui.printPresetsDialog`, `ui.pdfPresetsDialog`, `ui.perspectivePresetsDialog`, `ui.svgFilterDialog`, + `ui.saveSwatchLibrary`, `ui.saveBrushLibrary`, `ui.saveGraphicStyleLibrary` and `ui.saveSymbolLibrary`, under Ui; + `file.exportAs` and `file.exportSelection`, under File; `links.editOriginal`, under Links; `type.applyBullets` and + `type.applyNumbers`, under Type; and `window.toolbar.new`, `window.toolbarColumns`, `window.toolsBesideCanvas`, + `window.workspace.new`, `window.arrange.floatInWindow` and `window.arrange.consolidateAllWindows`, under Window. + +When such a command has the same group and label as an engine command that no item of the menu bar runs, the +command the menu item runs keeps the name, and the engine command is listed in the group named by the first part of +its id: File › Export Selection… runs `file.exportSelection`, as the menu item does, and `document.exportSelection` +is listed as Document › Export Selection…. A shortcut already given to `document.exportSelection` keeps running it. +When the other entry is anything else, the command is not listed: `window.learn` is left out, and Window › Learn is +the Learn panel's entry. + Each installed filter plug-in's Object › Plug-ins item is its own row, with id `plugin:` (the plug-in's id), its menu label and no default shortcut, so two filters with the same name have two rows. `shortcuts.set {id: "plugin:", shortcut}`, `shortcuts.list` and Import Shortcut Set use that id, `shortcuts.exportText` writes one line