keys: improve key config parsing

The recent switch to DeferredKeyCode introduced a bit of ambiguity
when parsing certain keys.

One issue was with the map that was used for some parsing didn't have
consistent/distinct entries for physical vs. mapped.

Another issue is that the key resolution for the simple case where
a key had the same physical and mapped representations would always
return the mapped one even when physical mode was set as a preference.

This commit kills the ambiguous map in favor of an string conversion
method on KeyCode.

It's possible that this will help with https://github.com/wez/wezterm/issues/1826
a little, but I started looking at this because there were a couple of
comments about Alt-Enter and some numpad keys no longer working in the
Element channel.

config/src/keys.rs

@@ -1,7 +1,6 @@
 use crate::keyassignment::{KeyAssignment, MouseEventTrigger};
 use luahelper::impl_lua_conversion;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
-use std::collections::HashMap;
 use std::convert::TryFrom;
 use wezterm_input_types::{KeyCode, Modifiers, PhysKeyCode};
 
@@ -22,31 +21,27 @@ impl Default for KeyMapPreference {
 #[serde(into = "String", try_from = "String")]
 pub enum DeferredKeyCode {
     KeyCode(KeyCode),
-    Either { physical: KeyCode, mapped: KeyCode },
+    Either {
+        physical: KeyCode,
+        mapped: KeyCode,
+        original: String,
+    },
 }
 
 impl DeferredKeyCode {
-    pub fn resolve(&self, position: KeyMapPreference) -> &KeyCode {
+    pub fn resolve(&self, position: KeyMapPreference) -> KeyCode {
         match (self, position) {
-            (Self::KeyCode(key), _) => key,
-            (Self::Either { mapped, .. }, KeyMapPreference::Mapped) => mapped,
-            (Self::Either { physical, .. }, KeyMapPreference::Physical) => physical,
-        }
-    }
-
-    fn as_string(key: &KeyCode) -> String {
-        if let Some(s) = INV_KEYCODE_MAP.get(key) {
-            s.to_string()
-        } else {
-            key.to_string()
+            (Self::KeyCode(key), KeyMapPreference::Physical) => match key.to_phys() {
+                Some(p) => KeyCode::Physical(p),
+                None => key.clone(),
+            },
+            (Self::KeyCode(key), _) => key.clone(),
+            (Self::Either { mapped, .. }, KeyMapPreference::Mapped) => mapped.clone(),
+            (Self::Either { physical, .. }, KeyMapPreference::Physical) => physical.clone(),
         }
     }
 
     fn parse_str(s: &str) -> anyhow::Result<KeyCode> {
-        if let Some(c) = KEYCODE_MAP.get(s) {
-            return Ok(c.clone());
-        }
-
         if let Some(phys) = s.strip_prefix("phys:") {
             let phys = PhysKeyCode::try_from(phys).map_err(|_| {
                 anyhow::anyhow!("expected phys:CODE physical keycode string, got: {}", s)
@@ -62,39 +57,18 @@ impl DeferredKeyCode {
         }
 
         if let Some(mapped) = s.strip_prefix("mapped:") {
-            let chars: Vec<char> = mapped.chars().collect();
-            return if chars.len() == 1 {
-                Ok(KeyCode::Char(chars[0]))
-            } else {
-                anyhow::bail!("invalid KeyCode string {}", s);
-            };
+            return KeyCode::try_from(mapped).map_err(|err| anyhow::anyhow!("{}", err));
         }
 
-        let chars: Vec<char> = s.chars().collect();
-        if chars.len() == 1 {
-            let k = KeyCode::Char(chars[0]);
-            if let Some(phys) = k.to_phys() {
-                Ok(KeyCode::Physical(phys))
-            } else {
-                Ok(k)
-            }
-        } else {
-            anyhow::bail!("invalid KeyCode string {}", s);
-        }
+        KeyCode::try_from(s).map_err(|err| anyhow::anyhow!("{}", err))
     }
 }
 
 impl Into<String> for DeferredKeyCode {
     fn into(self) -> String {
         match self {
-            DeferredKeyCode::KeyCode(key) => Self::as_string(&key),
-            DeferredKeyCode::Either { mapped, .. } => {
-                let mapped = Self::as_string(&mapped);
-                mapped
-                    .strip_prefix("mapped:")
-                    .expect("to have mapped: prefix")
-                    .to_string()
-            }
+            DeferredKeyCode::KeyCode(key) => key.to_string(),
+            DeferredKeyCode::Either { original, .. } => original.to_string(),
         }
     }
 }
@@ -118,7 +92,11 @@ impl TryFrom<&str> for DeferredKeyCode {
         let phys = Self::parse_str(&format!("phys:{}", s));
 
         match (mapped, phys) {
-            (Ok(mapped), Ok(physical)) => Ok(DeferredKeyCode::Either { mapped, physical }),
+            (Ok(mapped), Ok(physical)) => Ok(DeferredKeyCode::Either {
+                mapped,
+                physical,
+                original: s.to_string(),
+            }),
             (Ok(mapped), Err(_)) => Ok(DeferredKeyCode::KeyCode(mapped)),
             (Err(_), Ok(phys)) => Ok(DeferredKeyCode::KeyCode(phys)),
             (Err(a), Err(b)) => anyhow::bail!("invalid keycode {}: {:#}, {:#}", s, a, b),
@@ -172,138 +150,6 @@ pub struct Mouse {
 }
 impl_lua_conversion!(Mouse);
 
-fn make_map() -> HashMap<String, KeyCode> {
-    let mut map = HashMap::new();
-
-    macro_rules! m {
-        ($($val:ident),* $(,)?) => {
-            $(
-                let v = KeyCode::$val;
-                if let Some(phys) = v.to_phys() {
-                    map.insert(format!("phys:{}", stringify!($val)), KeyCode::Physical(phys));
-                    map.insert(format!("mapped:{}", stringify!($val)), v);
-                } else {
-                    map.insert(format!("mapped:{}", stringify!($val)), v);
-                }
-            )*
-        }
-    }
-
-    m!(
-        Hyper,
-        Super,
-        Meta,
-        Cancel,
-        Clear,
-        Shift,
-        LeftShift,
-        RightShift,
-        Control,
-        LeftControl,
-        RightControl,
-        Alt,
-        LeftAlt,
-        RightAlt,
-        Pause,
-        CapsLock,
-        VoidSymbol,
-        PageUp,
-        PageDown,
-        End,
-        Home,
-        LeftArrow,
-        RightArrow,
-        UpArrow,
-        DownArrow,
-        Select,
-        Print,
-        Execute,
-        PrintScreen,
-        Insert,
-        Help,
-        LeftWindows,
-        RightWindows,
-        Applications,
-        Sleep,
-        Multiply,
-        Add,
-        Separator,
-        Subtract,
-        Decimal,
-        Divide,
-        NumLock,
-        ScrollLock,
-        Copy,
-        Cut,
-        Paste,
-        BrowserBack,
-        BrowserForward,
-        BrowserRefresh,
-        BrowserStop,
-        BrowserSearch,
-        BrowserFavorites,
-        BrowserHome,
-        VolumeMute,
-        VolumeDown,
-        VolumeUp,
-        MediaNextTrack,
-        MediaPrevTrack,
-        MediaStop,
-        MediaPlayPause,
-        ApplicationLeftArrow,
-        ApplicationRightArrow,
-        ApplicationUpArrow,
-        ApplicationDownArrow,
-    );
-
-    for (label, phys) in &[
-        ("Backspace", PhysKeyCode::Backspace),
-        ("Delete", PhysKeyCode::Delete),
-        ("Enter", PhysKeyCode::Return),
-        ("Escape", PhysKeyCode::Escape),
-        ("Tab", PhysKeyCode::Tab),
-    ] {
-        map.insert(format!("phys:{}", label), KeyCode::Physical(*phys));
-        map.insert(format!("mapped:{}", label), phys.to_key_code());
-    }
-
-    for i in 0..=9 {
-        let k = KeyCode::Numpad(i);
-        map.insert(
-            format!("phys:Numpad{}", i),
-            KeyCode::Physical(k.to_phys().unwrap()),
-        );
-        // Not sure how likely someone is to remap the numpad, but...
-        map.insert(format!("mapped:Numpad{}", i), k);
-    }
-
-    for i in 1..=24 {
-        let k = KeyCode::Function(i);
-        if let Some(phys) = k.to_phys() {
-            map.insert(format!("phys:F{}", i), KeyCode::Physical(phys));
-            map.insert(format!("mapped:F{}", i), k);
-        } else {
-            // 21 and up don't have phys equivalents
-            map.insert(format!("mapped:F{}", i), k);
-        }
-    }
-
-    map
-}
-
-fn make_inv_map() -> HashMap<KeyCode, String> {
-    let mut map = HashMap::new();
-    for (k, v) in KEYCODE_MAP.iter() {
-        map.insert(v.clone(), k.clone());
-    }
-    map
-}
-
-lazy_static::lazy_static! {
-    static ref KEYCODE_MAP: HashMap<String, KeyCode> = make_map();
-    static ref INV_KEYCODE_MAP: HashMap<KeyCode, String> = make_inv_map();
-}
-
 pub(crate) fn ser_modifiers<S>(mods: &Modifiers, serializer: S) -> Result<S::Ok, S::Error>
 where
     S: Serializer,

wezterm-input-types/src/lib.rs

@@ -300,6 +300,126 @@ impl KeyCode {
     }
 }
 
+impl TryFrom<&str> for KeyCode {
+    type Error = String;
+    fn try_from(s: &str) -> std::result::Result<Self, String> {
+        macro_rules! m {
+            ($($val:ident),* $(,)?) => {
+                match s {
+                $(
+                    stringify!($val) => return Ok(Self::$val),
+                )*
+                    _ => {}
+                }
+            }
+        }
+
+        m!(
+            Hyper,
+            Super,
+            Meta,
+            Cancel,
+            Clear,
+            Shift,
+            LeftShift,
+            RightShift,
+            Control,
+            LeftControl,
+            RightControl,
+            Alt,
+            LeftAlt,
+            RightAlt,
+            Pause,
+            CapsLock,
+            VoidSymbol,
+            PageUp,
+            PageDown,
+            End,
+            Home,
+            LeftArrow,
+            RightArrow,
+            UpArrow,
+            DownArrow,
+            Select,
+            Print,
+            Execute,
+            PrintScreen,
+            Insert,
+            Help,
+            LeftWindows,
+            RightWindows,
+            Applications,
+            Sleep,
+            Multiply,
+            Add,
+            Separator,
+            Subtract,
+            Decimal,
+            Divide,
+            NumLock,
+            ScrollLock,
+            Copy,
+            Cut,
+            Paste,
+            BrowserBack,
+            BrowserForward,
+            BrowserRefresh,
+            BrowserStop,
+            BrowserSearch,
+            BrowserFavorites,
+            BrowserHome,
+            VolumeMute,
+            VolumeDown,
+            VolumeUp,
+            MediaNextTrack,
+            MediaPrevTrack,
+            MediaStop,
+            MediaPlayPause,
+            ApplicationLeftArrow,
+            ApplicationRightArrow,
+            ApplicationUpArrow,
+            ApplicationDownArrow,
+        );
+
+        match s {
+            "Backspace" => return Ok(KeyCode::Char('\u{8}')),
+            "Tab" => return Ok(KeyCode::Char('\t')),
+            "Return" | "Enter" => return Ok(KeyCode::Char('\r')),
+            "Escape" => return Ok(KeyCode::Char('\u{1b}')),
+            "Delete" => return Ok(KeyCode::Char('\u{7f}')),
+            _ => {}
+        };
+
+        if let Some(n) = s.strip_prefix("Numpad") {
+            let n: u8 = n
+                .parse()
+                .map_err(|err| format!("parsing Numpad<NUMBER>: {:#}", err))?;
+            if n > 9 {
+                return Err("Numpad numbers must be in range 0-9".to_string());
+            }
+            return Ok(KeyCode::Numpad(n));
+        }
+
+        if let Some(n) = s.strip_prefix("F") {
+            let n: u8 = n
+                .parse()
+                .map_err(|err| format!("parsing F<NUMBER>: {:#}", err))?;
+            if n == 0 || n > 24 {
+                return Err("Function key numbers must be in range 1-24".to_string());
+            }
+            return Ok(KeyCode::Function(n));
+        }
+
+        let chars: Vec<char> = s.chars().collect();
+        if chars.len() == 1 {
+            let k = KeyCode::Char(chars[0]);
+            Ok(k)
+        } else {
+            Err(format!("invalid KeyCode string {}", s))
+        }
+    }
+}
+
 impl ToString for KeyCode {
     fn to_string(&self) -> String {
         match self {