Refactor keyboard manager (#833)

* refactor(keyboard_manager): Refactor KeyboardManager

This refactoring changes nothing in key handling behaviour, but moves
some code out of handle_event, to make thigs clearer.

* style(keyboard_manager): Move private functions to bottom

src/window/window_wrapper/keyboard_manager.rs

@@ -6,80 +6,6 @@ use winit::platform::modifier_supplement::KeyEventExtModifierSupplement;
 use crate::bridge::UiCommand;
 use crate::channel_utils::LoggingTx;
 
-#[cfg(not(target_os = "windows"))]
-fn use_logo(logo: bool) -> bool {
-    logo
-}
-
-// The Windows key is used for OS-level shortcuts,
-// so we want to ignore the logo key on this platform.
-#[cfg(target_os = "windows")]
-fn use_logo(_: bool) -> bool {
-    false
-}
-
-#[cfg(not(target_os = "macos"))]
-fn use_alt(alt: bool) -> bool {
-    alt
-}
-
-// The option or alt key is used on Macos for character set changes
-// and does not operate the same as other systems.
-#[cfg(target_os = "macos")]
-fn use_alt(_: bool) -> bool {
-    false
-}
-
-fn or_empty(condition: bool, text: &str) -> &str {
-    if condition {
-        text
-    } else {
-        ""
-    }
-}
-
-fn is_control_key(key: Key<'static>) -> Option<&str> {
-    match key {
-        Key::Backspace => Some("BS"),
-        Key::Escape => Some("Esc"),
-        Key::Delete => Some("Del"),
-        Key::ArrowUp => Some("Up"),
-        Key::ArrowDown => Some("Down"),
-        Key::ArrowLeft => Some("Left"),
-        Key::ArrowRight => Some("Right"),
-        Key::F1 => Some("F1"),
-        Key::F2 => Some("F2"),
-        Key::F3 => Some("F3"),
-        Key::F4 => Some("F4"),
-        Key::F5 => Some("F5"),
-        Key::F6 => Some("F6"),
-        Key::F7 => Some("F7"),
-        Key::F8 => Some("F8"),
-        Key::F9 => Some("F9"),
-        Key::F10 => Some("F10"),
-        Key::F11 => Some("F11"),
-        Key::F12 => Some("F12"),
-        Key::Insert => Some("Insert"),
-        Key::Home => Some("Home"),
-        Key::End => Some("End"),
-        Key::PageUp => Some("PageUp"),
-        Key::PageDown => Some("PageDown"),
-        _ => None,
-    }
-}
-
-fn is_special(text: &str) -> Option<&str> {
-    match text {
-        " " => Some("Space"),
-        "<" => Some("lt"),
-        "\\" => Some("Bslash"),
-        "|" => Some("Bar"),
-        "\t" => Some("Tab"),
-        "\n" => Some("CR"),
-        _ => None,
-    }
-}
-
 pub struct KeyboardManager {
     command_sender: LoggingTx<UiCommand>,
     shift: bool,
@@ -103,19 +29,6 @@ impl KeyboardManager {
         }
     }
 
-    fn format_keybinding_string(&self, special: bool, use_shift: bool, text: &str) -> String {
-        let special = special || self.ctrl || use_alt(self.alt) || use_logo(self.logo);
-
-        let open = or_empty(special, "<");
-        let shift = or_empty(self.shift && use_shift, "S-");
-        let ctrl = or_empty(self.ctrl, "C-");
-        let alt = or_empty(use_alt(self.alt), "M-");
-        let logo = or_empty(use_logo(self.logo), "D-");
-        let close = or_empty(special, ">");
-
-        format!("{}{}{}{}{}{}{}", open, shift, ctrl, alt, logo, text, close)
-    }
-
     pub fn handle_event(&mut self, event: &Event<()>) {
         match event {
             Event::WindowEvent {
@@ -155,20 +68,33 @@ impl KeyboardManager {
                     for key_event in self.queued_key_events.iter() {
                         // And a key was pressed
                         if key_event.state == ElementState::Pressed {
+                            if let Some(keybinding) = self.maybe_get_keybinding(key_event) {
+                                self.command_sender
+                                    .send(UiCommand::Keyboard(keybinding))
+                                    .expect("Could not send keyboard ui command");
+                            }
+                        }
+                    }
+                }
+
+                // Regardless of whether this was a valid keyboard input or not, rest ignoring and
+                // whatever event was queued.
+                self.ignore_input_this_frame = false;
+                self.queued_key_events.clear();
+            }
+            _ => {}
+        }
+    }
+
+    fn maybe_get_keybinding(&self, key_event: &KeyEvent) -> Option<String> {
         // Determine if this key event represents a key which won't ever
         // present text.
         if let Some(key_text) = is_control_key(key_event.logical_key) {
-                                let keybinding_string =
+            Some(self.format_keybinding_string(true, true, key_text))
-                                    self.format_keybinding_string(true, true, key_text);
-
-                                self.command_sender
-                                    .send(UiCommand::Keyboard(keybinding_string))
-                                    .expect("Could not send keyboard ui command");
         } else {
-                                let is_dead_key = key_event.text_with_all_modifiers().is_some()
+            let is_dead_key =
-                                    && key_event.text.is_none();
+                key_event.text_with_all_modifiers().is_some() && key_event.text.is_none();
-                                let key_text =
+            let key_text = if (self.alt || is_dead_key) && cfg!(target_os = "macos") {
-                                    if (self.alt || is_dead_key) && cfg!(target_os = "macos") {
                 key_event.text_with_all_modifiers()
             } else {
                 key_event.text
@@ -177,28 +103,103 @@ impl KeyboardManager {
             if let Some(key_text) = key_text {
                 // This is not a control key, so we rely upon winit to determine if
                 // this is a deadkey or not.
-                                    let keybinding_string =
+                let keybinding_string = if let Some(escaped_text) = is_special(key_text) {
-                                        if let Some(escaped_text) = is_special(key_text) {
                     self.format_keybinding_string(true, false, escaped_text)
                 } else {
                     self.format_keybinding_string(false, false, key_text)
                 };
 
-                                    self.command_sender
+                Some(keybinding_string)
-                                        .send(UiCommand::Keyboard(keybinding_string))
+            } else {
-                                        .expect("Could not send keyboard ui command");
+                None
             }
         }
     }
+
+    fn format_keybinding_string(&self, special: bool, use_shift: bool, text: &str) -> String {
+        let special = special || self.ctrl || use_alt(self.alt) || use_logo(self.logo);
+
+        let open = or_empty(special, "<");
+        let shift = or_empty(self.shift && use_shift, "S-");
+        let ctrl = or_empty(self.ctrl, "C-");
+        let alt = or_empty(use_alt(self.alt), "M-");
+        let logo = or_empty(use_logo(self.logo), "D-");
+        let close = or_empty(special, ">");
+
+        format!("{}{}{}{}{}{}{}", open, shift, ctrl, alt, logo, text, close)
     }
 }
 
-                // Regardless of whether this was a valid keyboard input or not, rest ignoring and
+#[cfg(not(target_os = "windows"))]
-                // whatever event was queued.
+fn use_logo(logo: bool) -> bool {
-                self.ignore_input_this_frame = false;
+    logo
-                self.queued_key_events.clear();
 }
-            _ => {}
+
+// The Windows key is used for OS-level shortcuts,
+// so we want to ignore the logo key on this platform.
+#[cfg(target_os = "windows")]
+fn use_logo(_: bool) -> bool {
+    false
+}
+
+#[cfg(not(target_os = "macos"))]
+fn use_alt(alt: bool) -> bool {
+    alt
+}
+
+// The option or alt key is used on Macos for character set changes
+// and does not operate the same as other systems.
+#[cfg(target_os = "macos")]
+fn use_alt(_: bool) -> bool {
+    false
 }
+
+fn or_empty(condition: bool, text: &str) -> &str {
+    if condition {
+        text
+    } else {
+        ""
+    }
+}
+
+fn is_control_key(key: Key<'static>) -> Option<&str> {
+    match key {
+        Key::Backspace => Some("BS"),
+        Key::Escape => Some("Esc"),
+        Key::Delete => Some("Del"),
+        Key::ArrowUp => Some("Up"),
+        Key::ArrowDown => Some("Down"),
+        Key::ArrowLeft => Some("Left"),
+        Key::ArrowRight => Some("Right"),
+        Key::F1 => Some("F1"),
+        Key::F2 => Some("F2"),
+        Key::F3 => Some("F3"),
+        Key::F4 => Some("F4"),
+        Key::F5 => Some("F5"),
+        Key::F6 => Some("F6"),
+        Key::F7 => Some("F7"),
+        Key::F8 => Some("F8"),
+        Key::F9 => Some("F9"),
+        Key::F10 => Some("F10"),
+        Key::F11 => Some("F11"),
+        Key::F12 => Some("F12"),
+        Key::Insert => Some("Insert"),
+        Key::Home => Some("Home"),
+        Key::End => Some("End"),
+        Key::PageUp => Some("PageUp"),
+        Key::PageDown => Some("PageDown"),
+        _ => None,
+    }
+}
+
+fn is_special(text: &str) -> Option<&str> {
+    match text {
+        " " => Some("Space"),
+        "<" => Some("lt"),
+        "\\" => Some("Bslash"),
+        "|" => Some("Bar"),
+        "\t" => Some("Tab"),
+        "\n" => Some("CR"),
+        _ => None,
     }
 }