Merge branch 'craft-keymap' into keymap-ui

# Conflicts:
#	crates/keymap/src/egui.rs

crates/keymap/README.md

@@ -376,7 +376,9 @@ read-only rows for keys outside the catalogue, the app's import and export forma
   egui's Copy, Cut and Paste events stay with the app by default. On the desktop, egui-winit
   sends them in place of the Cmd+C, Cmd+X and Cmd+V presses (Ctrl off macOS), so a binding on
   those chords runs only with `with_clipboard_events(true)`. Then the events count as their
-  chords outside text fields, and a Paste event's text moves to `Hit::pasted`. An app whose
+  chords outside text fields, and a Paste event's text moves to `Hit::pasted`. The web build
+  sends the key press and then the event; the press runs the command and takes the event, with
+  its text, along. An app whose
   paste command asks egui for a paste gets a Paste event back, which runs that command again;
   such an app keeps the default.
 

crates/keymap/src/egui.rs

@@ -172,7 +172,8 @@ pub struct Hit {
     /// the shifted key for a Shift binding (`:` for `Shift+;`), Shift held to type a symbol, or
     /// `+` and `=` standing for each other.
     pub exact: bool,
-    /// The text of the Paste event that ran the command ([`Dispatcher::with_clipboard_events`]).
+    /// The text of the Paste event that ran the command, or on the web the Paste event that came
+    /// with the press that ran it ([`Dispatcher::with_clipboard_events`]).
     pub pasted: Option<String>,
 }
 
@@ -239,7 +240,8 @@ impl Dispatcher {
     /// Cmd+X, Cmd+V; Shift+Delete and Shift+Insert for Cut and Paste with Shift alone) outside text
     /// fields. Off by default: the events stay in egui's input for the app's own handlers. With it
     /// on, an event that runs a command is removed, and a Paste event's text moves to
-    /// [`Hit::pasted`]. An app that asks egui for a paste (`ViewportCommand::RequestPaste`) to run
+    /// [`Hit::pasted`]. On the web, the key press runs the command and its clipboard event goes
+    /// with it the same way; when the press runs nothing, or the hit is declined, both stay. An app that asks egui for a paste (`ViewportCommand::RequestPaste`) to run
     /// its paste command gets a Paste event back, which with this on runs that command again.
     pub fn with_clipboard_events(mut self, on: bool) -> Dispatcher {
         self.clipboard = on;
@@ -265,11 +267,12 @@ impl Dispatcher {
         let mut kept = Vec::with_capacity(input.events.len());
         // The Text event egui sends after a key press belongs to that press.
         let mut drop_text = false;
-        // The web build sends both a clipboard key press and the clipboard event it causes.
-        let mut clipboard_press = None;
         let events = std::mem::take(&mut input.events);
         let typed = if active.typing { typed_text(&events) } else { Vec::new() };
-        let causes_event = if self.clipboard { Vec::new() } else { clipboard_event_follows(&events, platform) };
+        // The web build sends both a clipboard key press and the clipboard event it causes. The
+        // press decides: its event goes with it when it runs a command and stays otherwise.
+        let causes = clipboard_event_follows(&events, platform);
+        let mut caused_taken: Vec<Option<bool>> = vec![None; events.len()];
         for (i, event) in events.into_iter().enumerate() {
             match &event {
                 Event::Key { key, pressed: true, repeat, modifiers, .. } if !*repeat || self.repeat => {
@@ -280,16 +283,22 @@ impl Dispatcher {
                         let keep = if active.typing {
                             typed.get(i) == Some(&true) || clip.is_some() || may_be_alt_gr(key, held, platform)
                         } else {
-                            causes_event.get(i) == Some(&true)
+                            !self.clipboard && causes[i].is_some()
                         };
                         if keep {
                             kept.push(event);
                             continue;
                         }
-                        if clip.is_some() {
-                            clipboard_press = clip;
+                        let caused = causes[i].clone();
+                        let hit = self.press(key, held, *repeat, now, keymap, active).map(|mut hit| {
+                            hit.pasted = caused.as_ref().and_then(|(_, text)| text.clone());
+                            hit
+                        });
+                        let hit = hit.filter(&mut accept);
+                        if let Some((j, _)) = caused {
+                            caused_taken[j] = Some(hit.is_some());
                         }
-                        if let Some(hit) = self.press(key, held, *repeat, now, keymap, active).filter(&mut accept) {
+                        if let Some(hit) = hit {
                             hits.push(hit);
                             drop_text = !(held.cmd || held.ctrl);
                             continue;
@@ -302,9 +311,11 @@ impl Dispatcher {
                 }
                 // A focused text field copies and pastes on these events itself.
                 Event::Copy | Event::Cut | Event::Paste(_) if self.clipboard && !active.typing => {
-                    let (key, held, clip) = clipboard_chord(&event, frame_mods);
-                    if clipboard_press == Some(clip) {
-                        clipboard_press = None;
+                    let (key, held, _) = clipboard_chord(&event, frame_mods);
+                    if let Some(taken) = caused_taken[i] {
+                        if taken {
+                            continue;
+                        }
                     } else if let Some(mut hit) = self.press(key, held, false, now, keymap, active) {
                         if let Event::Paste(text) = &event {
                             hit.pasted = Some(text.clone());
@@ -348,19 +359,19 @@ pub(crate) enum Clip {
     Paste,
 }
 
-/// For each event, whether it is a clipboard key press whose clipboard event comes later in the
-/// frame, as the web build sends them.
-fn clipboard_event_follows(events: &[Event], platform: Platform) -> Vec<bool> {
-    let mut out = vec![false; events.len()];
-    let mut ahead = [false; 3];
-    for (event, follows) in events.iter().zip(out.iter_mut()).rev() {
+/// For each clipboard key press whose clipboard event comes later in the frame, as the web build
+/// sends them: the index of that event, and the text of a Paste event.
+fn clipboard_event_follows(events: &[Event], platform: Platform) -> Vec<Option<(usize, Option<String>)>> {
+    let mut out = vec![None; events.len()];
+    let mut ahead: [Option<usize>; 3] = [None; 3];
+    for (i, event) in events.iter().enumerate().rev() {
         if let Some(seen) = event_clip(event).and_then(|c| ahead.get_mut(c as usize)) {
-            *seen = true;
+            *seen = Some(i);
         } else if let Event::Key { key, pressed: true, modifiers, .. } = event {
             let clip = Key::from_egui(*key).and_then(|k| clipboard_kind(k, Mods::from_egui(modifiers, platform)));
-            if let Some(seen) = clip.and_then(|c| ahead.get_mut(c as usize)) {
-                *follows = *seen;
-                *seen = false;
+            if let Some(j) = clip.and_then(|c| ahead.get_mut(c as usize)).and_then(Option::take) {
+                let text = if let Some(Event::Paste(text)) = events.get(j) { Some(text.clone()) } else { None };
+                out[i] = Some((j, text));
             }
         }
     }

crates/keymap/src/import.rs

@@ -279,8 +279,7 @@ pub struct Failed {
     pub command: CommandRef,
     pub label: String,
     pub change: FailedChange,
-    /// The set's shortcuts for [`FailedChange::Bind`], the one left on the command for
-    /// [`FailedChange::Unbind`]; empty for [`FailedChange::Clear`].
+    /// The set's shortcuts for [`FailedChange::Bind`]; empty for [`FailedChange::Clear`].
     pub shortcuts: Vec<String>,
 }
 
@@ -288,13 +287,11 @@ pub struct Failed {
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub enum FailedChange {
-    /// Giving the command the set's bindings.
+    /// Giving the command the set's bindings, or taking one of them from another command.
     Bind,
     /// Leaving the command without shortcuts: the set has it without keys and
     /// [`NoneDefined::Clear`] applies.
     Clear,
-    /// Taking a chord the set gives to another command; both commands keep it.
-    Unbind,
 }
 
 /// Why keys of an entry were not used.
@@ -548,7 +545,8 @@ impl ImportPlan {
     ///
     /// A change that finds the overrides full is left out and reported in
     /// [`ImportReport::failed`]; the others still apply, and a chord it would have taken from an
-    /// earlier entry stays with that entry. With [`Start::Defaults`] that cannot happen.
+    /// earlier entry stays with that entry. That includes taking a chord from another command:
+    /// both commands keep the bindings they had. With [`Start::Defaults`] that cannot happen.
     pub fn apply(&self, keymap: &mut Keymap) -> ImportReport {
         let before: Vec<Vec<Binding>> = self.commands.iter().map(|p| keymap.bindings(&p.command)).collect();
         // Dropping every entry leaves room for all of them: each catalogue command takes at most one
@@ -598,7 +596,10 @@ impl ImportPlan {
                 decided.insert(p.command.key());
             }
 
-            for (p, list) in self.commands.iter().zip(&accepted) {
+            // A command that cannot take a chord from another command is dropped, so both keep
+            // the bindings they had.
+            let mut unbind_failed = false;
+            for (c, (p, list)) in self.commands.iter().zip(&accepted).enumerate() {
                 if list.is_empty() || !decided.contains(&p.command.key()) {
                     continue;
                 }
@@ -612,10 +613,22 @@ impl ImportPlan {
                     if keymap.remove(&h.command, &h.binding) {
                         report.conflicts_cleared.push(Cleared { command: h.command, label, shortcut });
                     } else {
-                        report.failed.push(Failed { command: h.command, label, change: FailedChange::Unbind, shortcuts: vec![shortcut] });
+                        let home = home_of(keymap, &p.command);
+                        let list = in_default_order(keymap, &p.command, list);
+                        let shortcuts = list.iter().map(|b| b.text(&home)).collect();
+                        dropped.insert(
+                            c,
+                            Failed { command: p.command.clone(), label: label_of(keymap, &p.command), change: FailedChange::Bind, shortcuts },
+                        );
+                        unbind_failed = true;
+                        break;
                     }
                 }
             }
+            if unbind_failed {
+                *keymap = start.clone();
+                continue 'attempt;
+            }
 
             for (p, from) in self.commands.iter().zip(&before) {
                 if !decided.contains(&p.command.key()) {

crates/keymap/src/ui/review.rs

@@ -284,7 +284,6 @@ impl Draw<'_, '_> {
         match change {
             FailedChange::Bind => self.ui("the set's shortcuts could not be added"),
             FailedChange::Clear => self.ui("the shortcuts could not be removed"),
-            FailedChange::Unbind => self.ui("the shortcut could not be taken from it"),
         }
     }
 

crates/keymap/src/ui/text.rs

@@ -289,7 +289,6 @@ pub const STRINGS: &[&str] = &[
     "the scope is unknown",
     "the set's shortcuts could not be added",
     "the shortcuts could not be removed",
-    "the shortcut could not be taken from it",
     "Copy Report",
     "Done",
 ];

crates/keymap/tests/egui.rs

@@ -193,6 +193,31 @@ fn clipboard_events_run_their_chords_outside_text_fields_when_turned_on() {
     }
 }
 
+#[test]
+fn a_web_paste_carries_its_text_and_takes_its_event() {
+    let km = dispatch_keymap();
+    let mut d = Dispatcher::new().with_clipboard_events(true);
+    // The web build sends the Cmd+V press and then the Paste event with the clipboard text.
+    let events = vec![press(EguiKey::V, CTRL), Event::Paste("text".into()), press(EguiKey::C, CTRL), Event::Copy];
+    let mut input = frame(1.0, CTRL, events.clone());
+    let mut seen = Vec::new();
+    let hits = d.dispatch_filtered(&mut input, &km, &Active::NONE, |hit| {
+        seen.push(hit.pasted.clone());
+        true
+    });
+    let ran: Vec<_> = hits.iter().map(|h| (h.command.id.as_str(), h.pasted.as_deref())).collect();
+    assert_eq!(ran, [("edit.paste", Some("text")), ("edit.copy", None)]);
+    assert_eq!(seen, [Some("text".to_string()), None], "the filter sees the pasted text");
+    assert!(input.events.is_empty(), "{:?}", input.events);
+
+    // A declined press leaves the press and its event in place, and the event does not run again.
+    let mut input = frame(2.0, CTRL, events.clone());
+    let hits = d.dispatch_filtered(&mut input, &km, &Active::NONE, |hit| hit.command.id != "edit.paste");
+    let ran: Vec<_> = hits.iter().map(|h| h.command.id.as_str()).collect();
+    assert_eq!(ran, ["edit.copy"]);
+    assert_eq!(input.events, events[..2]);
+}
+
 #[test]
 fn declined_hits_leave_their_events_in_order() {
     let km = dispatch_keymap();

crates/keymap/tests/import.rs

@@ -330,10 +330,11 @@ fn changes_that_find_the_overrides_full_are_reported() {
     };
     let options = |start| ImportOptions { contexts: vec![("Default".into(), ContextRule::Home)], start, ..ImportOptions::default() };
 
-    // file.close has an entry to rewrite; the other changes each need a new one.
+    // file.close has an entry to rewrite, but taking Cmd+J from view.goToPage needs a new one, so
+    // both stay as they were. The other changes each need a new entry too.
     let mut km = keymap(Platform::Windows).with_overrides(full());
     let r = plan(&set, &km, &options(Start::Current)).apply(&mut km);
-    has(&km, "file.close", &["Cmd+J"]);
+    has(&km, "file.close", &["Cmd+Q"]);
     has(&km, "view.goToPage", &["Cmd+J"]);
     has(&km, "file.save", &["Cmd+S"]);
     has(&km, "edit.duplicate", &["Cmd+Alt+Shift+D"]);
@@ -346,16 +347,16 @@ fn changes_that_find_the_overrides_full_are_reported() {
     assert_eq!(
         r.failed,
         [
+            row("file.close", "Close", FailedChange::Bind, &["Cmd+J"]),
             row("file.save", "Save", FailedChange::Bind, &["Cmd+Alt+S"]),
             row("edit.duplicate", "Duplicate", FailedChange::Clear, &[]),
-            row("view.goToPage", "Go to Page…", FailedChange::Unbind, &["Cmd+J"]),
         ]
     );
-    assert_eq!(r.imported, 1);
+    assert_eq!(r.imported, 0);
     assert!(r.conflicts_cleared.is_empty());
     let v = serde_json::to_value(&r).unwrap();
     assert_eq!(
-        v["failed"][0],
+        v["failed"][1],
         json!({"command": {"id": "file.save", "params": null}, "label": "Save", "change": "bind", "shortcuts": ["Cmd+Alt+S"]})
     );