#1298: reserve system shortcuts

The keymap reserves craft-keymap's system chords for its platform (window switching and closing, the virtual consoles). Binding one is refused with "<chord> is reserved by the system"; an imported set's key on one is left out and the import summary counts it. No default shortcut sits on a reserved chord on any platform (test).

crates/ui-egui/src/keys.rs

@@ -6,7 +6,7 @@ use std::collections::{HashMap, HashSet};
 use std::sync::OnceLock;
 use std::sync::atomic::{AtomicU8, Ordering};
 
-use craft_keymap::{Bindable, Binding, Chord, CommandRef, Key, Keymap, Kind, Mods, Overrides, Platform, Policy, Scope, Scopes};
+use craft_keymap::{AssignError, Bindable, Binding, Chord, CommandRef, Key, Keymap, Kind, Mods, Overrides, Platform, Policy, Scope, Scopes};
 use serde_json::{Value, json};
 
 use crate::DesignApp;
@@ -135,7 +135,15 @@ pub fn note_os(os: egui::os::OperatingSystem) {
 
 /// The keymap with the user's overrides, for [`platform`].
 pub fn new_keymap(overrides: Overrides) -> Keymap {
-    Keymap::new(catalogue().to_vec(), Scopes::new().with_context(TEXT_CONTEXT), platform()).with_overrides(overrides)
+    keymap_for(platform(), overrides)
+}
+
+/// The keymap for `platform`: the system's own chords are reserved, so neither a user edit nor an
+/// imported set can take them.
+pub fn keymap_for(platform: Platform, overrides: Overrides) -> Keymap {
+    Keymap::new(catalogue().to_vec(), Scopes::new().with_context(TEXT_CONTEXT), platform)
+        .with_reserved(craft_keymap::reserved::system(platform))
+        .with_overrides(overrides)
 }
 
 /// Every bindable command, in dispatch order: where two share keys the earlier one runs. UI
@@ -280,7 +288,7 @@ pub fn set(app: &mut DesignApp, cmd: &CommandRef, shortcut: Option<&str>, policy
         }
         Some(sc) => {
             let binding = Binding::parse(sc).map_err(|e| format!("can't read shortcut `{sc}`: {e}"))?;
-            let done = app.keymap.set_bindings(cmd, vec![binding], policy).map_err(|e| format!("can't assign `{sc}`: {e}"))?;
+            let done = app.keymap.set_bindings(cmd, vec![binding], policy).map_err(|e| assign_error(&app.keymap, sc, &e))?;
             taken = done.taken_from.into_iter().map(|h| h.command).collect();
         }
     }
@@ -288,6 +296,15 @@ pub fn set(app: &mut DesignApp, cmd: &CommandRef, shortcut: Option<&str>, policy
     Ok(taken)
 }
 
+/// Why the keymap refused `sc`. A chord the system takes is named as the menus show it
+/// ("⌘Tab is reserved by the system").
+fn assign_error(keymap: &Keymap, sc: &str, e: &AssignError) -> String {
+    match e {
+        AssignError::Reserved(t) => format!("{} is reserved by the system", t.display(keymap.platform())),
+        e => format!("can't assign `{sc}`: {e}"),
+    }
+}
+
 /// Other commands on the keys `cmd` holds, by command key.
 pub fn conflicts(keymap: &Keymap, cmd: &CommandRef) -> Vec<String> {
     let mut out: Vec<String> = Vec::new();
@@ -309,6 +326,59 @@ pub fn label(keymap: &Keymap, key: &str) -> String {
 mod tests {
     use super::*;
 
+    const PLATFORMS: [Platform; 3] = [Platform::Mac, Platform::Windows, Platform::Linux];
+
+    /// No default shortcut sits on a chord an operating system takes before the app sees it.
+    #[test]
+    fn defaults_avoid_reserved_chords() {
+        for p in PLATFORMS {
+            let km = keymap_for(p, Overrides::new());
+            let reserved: Vec<String> = km
+                .catalogue()
+                .iter()
+                .flat_map(|b| km.default_bindings(&b.command).into_iter().map(move |d| (b.command.key(), d)))
+                .filter(|(_, d)| km.is_reserved(&d.trigger.chord))
+                .map(|(k, d)| format!("{k} {}", d.trigger))
+                .collect();
+            assert_eq!(reserved, Vec::<String>::new(), "{p:?}: defaults on reserved chords");
+        }
+    }
+
+    /// A chord the operating system takes, per platform.
+    fn os_chord(p: Platform) -> &'static str {
+        match p {
+            Platform::Mac => "Cmd+Tab",
+            Platform::Windows => "Alt+F4",
+            Platform::Linux => "Cmd+Alt+F2",
+        }
+    }
+
+    /// Binding a chord the system takes is refused with a message naming it, and a saved override
+    /// on one leaves the command its default.
+    #[test]
+    fn a_reserved_chord_is_refused() {
+        let cmd = CommandRef::new("app.palette");
+        for p in PLATFORMS {
+            let mut km = keymap_for(p, Overrides::new());
+            let before = km.bindings(&cmd);
+            let sc = os_chord(p);
+            let binding = Binding::parse(sc).unwrap();
+            let shown = binding.trigger.display(p);
+            let err = km.set_bindings(&cmd, vec![binding], Policy::Steal).unwrap_err();
+            assert_eq!(assign_error(&km, sc, &err), format!("{shown} is reserved by the system"), "{p:?}");
+            assert_eq!(km.bindings(&cmd), before, "{p:?}");
+            let mut saved = Overrides::new();
+            saved.insert(cmd.key(), vec![sc.to_string()]);
+            assert_eq!(keymap_for(p, saved).bindings(&cmd), before, "{p:?}: a saved reserved chord");
+        }
+        // Through `window.setShortcut`, as the Keyboard Shortcuts dialog and the control channel set keys.
+        let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());
+        let sc = os_chord(platform());
+        let r = menus::run_ui(&mut app, "window.setShortcut", &json!({"id": "app.palette", "shortcut": sc, "steal": true})).unwrap();
+        assert!(r.unwrap_err().ends_with("is reserved by the system"));
+        assert_eq!(menus::shortcut_of(&app, "app.palette").as_deref(), Some("Cmd+Enter"));
+    }
+
     /// The web build follows the operating system in the browser's user agent.
     #[test]
     fn browser_user_agent_picks_the_platform() {

crates/ui-egui/src/shortcut_import.rs

@@ -260,11 +260,19 @@ pub fn summary(lang: &str, report: &Value) -> String {
     let len = |k: &str| report[k].as_array().map_or(0, Vec::len).to_string();
     // Rows for keys in contexts DesignCraft lacks belong to items that did match.
     let unmatched = report["unmatched"].as_array().map_or(0, |rows| rows.iter().filter(|r| r["reason"] != "context").count());
-    crate::i18n::tr(lang, "Imported {count} shortcuts from “{set}”; {unmatched} items not matched, {unreadable} unreadable.")
+    let mut out = crate::i18n::tr(lang, "Imported {count} shortcuts from “{set}”; {unmatched} items not matched, {unreadable} unreadable.")
         .replace("{count}", &n("imported"))
         .replace("{set}", report["set"].as_str().unwrap_or_default())
         .replace("{unmatched}", &unmatched.to_string())
-        .replace("{unreadable}", &len("unreadable"))
+        .replace("{unreadable}", &len("unreadable"));
+    let reserved = report["skipped"].as_array().map_or(0, |rows| rows.iter().filter(|r| r["reason"] == "reserved").count());
+    if reserved > 0 {
+        out.push(' ');
+        out.push_str(
+            &crate::i18n::tr(lang, "{count} shortcuts are reserved by the system and were left out.").replace("{count}", &reserved.to_string()),
+        );
+    }
+    out
 }
 
 /// The Keyboard Shortcuts dialog's Import button: a native file picker imports at once and
@@ -330,7 +338,7 @@ mod tests {
             Product area : Tools\r\n\
             \tPen Tool --- Default: Shift+Ctrl+F3\r\n\
             Product area : File Menu\r\n\
-            \tExit --- Default: Ctrl+Alt+F12\r\n\
+            \tExit --- Default: Ctrl+Alt+F13\r\n\
             garbage line without a separator\r\n";
         let r = menus::run_ui(&mut app, "window.importShortcuts", &json!({"text": set})).unwrap().unwrap();
         assert_eq!(r["set"], "My Set");
@@ -362,7 +370,7 @@ mod tests {
         assert_eq!(crate::keys::shortcut(&app.keymap, &crate::keys::panel("swatches")).as_deref(), Some("Cmd+Shift+F2"));
         assert_eq!(crate::keys::shortcut(&app.keymap, &crate::keys::tool("pen")).as_deref(), Some("Cmd+Shift+F3"));
         // InDesign's name for a command, as an alias.
-        assert_eq!(menus::shortcut_of(&app, "app.quit").as_deref(), Some("Cmd+Alt+F12"));
+        assert_eq!(menus::shortcut_of(&app, "app.quit").as_deref(), Some("Cmd+Alt+F13"));
         // The numeric keypad key is bound as the main key of the same name, and reported.
         assert_eq!(r["approximated"][0]["item"], "File > Close", "{r}");
         assert_eq!(r["imported"], 7, "{r}");
@@ -371,6 +379,20 @@ mod tests {
         assert_eq!(menus::shortcut_of(&app, "app.palette").as_deref(), Some("Cmd+Enter"));
     }
 
+    /// A set's key on a chord the system takes is left out and the summary says so.
+    #[test]
+    fn a_reserved_chord_in_a_set_is_left_out() {
+        let mut app = app();
+        // Ctrl in a Windows set is ⌘ on a Mac.
+        let keys = if crate::keys::platform().is_mac() { "Ctrl+Tab" } else { "Alt+F4" };
+        let set = format!("Shortcut Set : OS\r\nProduct area : File Menu\r\n\tExit --- Default: {keys}\r\n");
+        let r = menus::run_ui(&mut app, "window.importShortcuts", &json!({ "text": set })).unwrap().unwrap();
+        assert_eq!(r["skipped"][0]["reason"], "reserved", "{r}");
+        assert_eq!(r["imported"], 0, "{r}");
+        assert_eq!(menus::shortcut_of(&app, "app.quit"), menus::default_shortcut("app.quit").map(str::to_string));
+        assert!(summary("en", &r).ends_with("1 shortcuts are reserved by the system and were left out."), "{}", summary("en", &r));
+    }
+
     #[test]
     fn a_panel_item_shares_its_key_with_the_command_that_opens_the_panel() {
         let mut app = app();