Merge branch 'multi_ke' into 'master'

Improve generation of key maps

See merge request Librem5/squeekboard!388

src/data.rs

@@ -382,56 +382,43 @@ impl Layout {
                 )
             )}).collect();
 
-        let keymap: HashMap<String, u32> = generate_keycodes(
+        let symbolmap: HashMap<String, u32> = generate_keycodes(
-            button_actions.iter()
+            extract_symbol_names(&button_actions)
-                .filter_map(|(_name, action)| {
-                    match action {
-                        ::action::Action::Submit {
-                            text: _, keys,
-                        } => Some(keys),
-                        _ => None,
-                    }
-                })
-                .flatten()
-                .map(|named_keysym| named_keysym.0.as_str())
         );
 
-        let button_states = button_actions.into_iter().map(|(name, action)| {
-            let keycodes = match &action {
-                ::action::Action::Submit { text: _, keys } => {
-                    keys.iter().map(|named_keycode| {
-                        *keymap.get(named_keycode.0.as_str())
-                            .expect(
-                                format!(
-                                    "keycode {} in key {} missing from keymap",
-                                    named_keycode.0,
-                                    name
-                                ).as_str()
-                            )
-                    }).collect()
-                },
-                action::Action::Erase => vec![
-                    *keymap.get("BackSpace")
-                        .expect(&format!("BackSpace missing from keymap")),
-                ],
-                _ => Vec::new(),
-            };
-            (
-                name.into(),
-                KeyState {
-                    pressed: PressType::Released,
-                    keycodes,
-                    action,
-                }
-            )
-        });
-
         let button_states = HashMap::<String, KeyState>::from_iter(
-            button_states
+            button_actions.into_iter().map(|(name, action)| {
+                let keycodes = match &action {
+                    ::action::Action::Submit { text: _, keys } => {
+                        keys.iter().map(|named_keysym| {
+                            *symbolmap.get(named_keysym.0.as_str())
+                                .expect(
+                                    format!(
+                                        "keysym {} in key {} missing from symbol map",
+                                        named_keysym.0,
+                                        name
+                                    ).as_str()
+                                )
+                        }).collect()
+                    },
+                    action::Action::Erase => vec![
+                        *symbolmap.get("BackSpace")
+                            .expect(&format!("BackSpace missing from symbol map")),
+                    ],
+                    _ => Vec::new(),
+                };
+                (
+                    name.into(),
+                    KeyState {
+                        pressed: PressType::Released,
+                        keycodes,
+                        action,
+                    }
+                )
+            })
         );
 
-        // TODO: generate from symbols
+        let keymap_str = match generate_keymap(symbolmap) {
-        let keymap_str = match generate_keymap(&button_states) {
             Err(e) => { return (Err(e), warning_handler) },
             Ok(v) => v,
         };
@@ -734,6 +721,22 @@ fn create_button<H: logging::Handler>(
     }
 }
 
+fn extract_symbol_names<'a>(actions: &'a [(&str, action::Action)])
+    -> impl Iterator<Item=&'a str>
+{
+    actions.iter()
+        .filter_map(|(_name, act)| {
+            match act {
+                action::Action::Submit {
+                    text: _, keys,
+                } => Some(keys),
+                _ => None,
+            }
+        })
+        .flatten()
+        .map(|named_keysym| named_keysym.0.as_str())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -862,6 +865,23 @@ mod tests {
         );
     }
 
+    /// Test if erase yields a useable keycode
+    #[test]
+    fn test_layout_erase() {
+        let out = Layout::from_file(path_from_root("tests/layout_erase.yaml"))
+            .unwrap()
+            .build(ProblemPanic).0
+            .unwrap();
+        assert_eq!(
+            out.views["base"].1
+                .get_rows()[0].1
+                .buttons[0].1
+                .state.borrow()
+                .keycodes.len(),
+            1
+        );
+    }
+
     #[test]
     fn parsing_fallback() {
         assert!(Layout::from_resource(FALLBACK_LAYOUT_NAME)
@@ -939,4 +959,35 @@ mod tests {
             }
         );
     }
+
+    #[test]
+    fn test_extract_symbols() {
+        let actions = [(
+            "ac",
+            action::Action::Submit {
+                text: None,
+                keys: vec![
+                    action::KeySym("a".into()),
+                    action::KeySym("c".into()),
+                ],
+            },
+        )];
+        assert_eq!(
+            extract_symbol_names(&actions[..]).collect::<Vec<_>>(),
+            vec!["a", "c"],
+        );
+    }
+
+    #[test]
+    fn test_extract_symbols_erase() {
+        let actions = [(
+            "Erase",
+            action::Action::Erase,
+        )];
+        assert_eq!(
+            extract_symbol_names(&actions[..]).collect::<Vec<_>>(),
+            Vec::<&str>::new(), //"BackSpace"], // TODO: centralize handling of BackSpace
+        );
+    }
+
 }

src/keyboard.rs

@@ -9,7 +9,6 @@ use std::rc::Rc;
 use std::string::FromUtf8Error;
 
 use ::action::Action;
-use ::logging;
 
 // Traits
 use std::io::Write;
@@ -92,7 +91,7 @@ fn sorted<'a, I: Iterator<Item=&'a str>>(
 /// HACK: starting from 9, because 8 results in keycode 0,
 /// which the compositor likes to discard
 pub fn generate_keycodes<'a, C: IntoIterator<Item=&'a str>>(
-    key_names: C
+    key_names: C,
 ) -> HashMap<String, u32> {
     let special_keysyms = ["BackSpace", "Return"].iter().map(|&s| s);
     HashMap::from_iter(
@@ -125,11 +124,9 @@ impl From<io::Error> for FormattingError {
 }
 
 /// Generates a de-facto single level keymap.
-// TODO: don't rely on keys and their order,
+/// Key codes must not repeat and should remain between 9 and 255.
-// but rather on what keysyms and keycodes are in use.
-// Iterating actions makes it hard to deduplicate keysyms.
 pub fn generate_keymap(
-    keystates: &HashMap::<String, KeyState>
+    symbolmap: HashMap::<String, KeyCode>,
 ) -> Result<String, FormattingError> {
     let mut buf: Vec<u8> = Vec::new();
     writeln!(
@@ -138,88 +135,75 @@ pub fn generate_keymap(
 
     xkb_keycodes \"squeekboard\" {{
         minimum = 8;
-        maximum = 255;"
+        maximum = 999;"
     )?;
     
-    for (name, state) in keystates.iter() {
+    // Xorg can only consume up to 255 keys, so this may not work in Xwayland.
-        match &state.action {
+    // Two possible solutions:
-            Action::Submit { text: _, keys } => {
+    // - use levels to cram multiple characters into one key
-                if let 0 = keys.len() {
+    // - swap layouts on key presses
-                    log_print!(
+    for keycode in symbolmap.values() {
-                        logging::Level::Warning,
+        write!(
-                        "Key {} has no keysyms", name,
+            buf,
-                    );
+            "
-                };
+        <I{}> = {0};",
-                for (named_keysym, keycode) in keys.iter().zip(&state.keycodes) {
+            keycode,
-                    write!(
+        )?;
-                        buf,
-                        "
-        <{}> = {};",
-                        named_keysym.0,
-                        keycode,
-                    )?;
-                }
-            },
-            Action::Erase => {
-                let mut keycodes = state.keycodes.iter();
-                write!(
-                    buf,
-                    "
-        <BackSpace> = {};",
-                    keycodes.next().expect("Erase key has no keycode"),
-                )?;
-                if let Some(_) = keycodes.next() {
-                    log_print!(
-                        logging::Level::Bug,
-                        "Erase key has multiple keycodes",
-                    );
-                }
-            },
-            _ => {},
-        }
     }
-    
+
     writeln!(
         buf,
         "
+        indicator 1 = \"Caps Lock\"; // Xwayland won't accept without it.
     }};
     
     xkb_symbols \"squeekboard\" {{
-
+"
-        name[Group1] = \"Letters\";
-        name[Group2] = \"Numbers/Symbols\";
-        
-        key <BackSpace> {{ [ BackSpace ] }};"
     )?;
     
-    for (_name, state) in keystates.iter() {
+    for (name, keycode) in symbolmap.iter() {
-        if let Action::Submit { text: _, keys } = &state.action {
+        write!(
-            for keysym in keys.iter() {
+            buf,
-                write!(
+            "
-                    buf,
+key <I{}> {{ [ {} ] }};",
-                    "
+            keycode,
-        key <{}> {{ [ {0} ] }};",
+            name,
-                    keysym.0,
+        )?;
-                )?;
-            }
-        }
     }
+
     writeln!(
         buf,
         "
     }};
 
     xkb_types \"squeekboard\" {{
+        virtual_modifiers Squeekboard; // No modifiers! Needed for Xorg for some reason.
+    
+        // Those names are needed for Xwayland.
+        type \"ONE_LEVEL\" {{
+            modifiers= none;
+            level_name[Level1]= \"Any\";
+        }};
+        type \"TWO_LEVEL\" {{
+            level_name[Level1]= \"Base\";
+        }};
+        type \"ALPHABETIC\" {{
+            level_name[Level1]= \"Base\";
+        }};
+        type \"KEYPAD\" {{
+            level_name[Level1]= \"Base\";
+        }};
+        type \"SHIFT+ALT\" {{
+            level_name[Level1]= \"Base\";
+        }};
 
-	type \"TWO_LEVEL\" {{
-            modifiers = Shift;
-            map[Shift] = Level2;
-            level_name[Level1] = \"Base\";
-            level_name[Level2] = \"Shift\";
-	}};
     }};
 
     xkb_compatibility \"squeekboard\" {{
+        // Needed for Xwayland again.
+        interpret Any+AnyOf(all) {{
+            action= SetMods(modifiers=modMapMods,clearLocks);
+        }};
     }};
 }};"
     )?;
@@ -234,21 +218,13 @@ mod tests {
     
     use xkbcommon::xkb;
 
-    use ::action::KeySym;
-
     #[test]
     fn test_keymap_multi() {
         let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS);
 
-        let keymap_str = generate_keymap(&hashmap!{
+        let keymap_str = generate_keymap(hashmap!{
-            "ac".into() => KeyState {
+            "a".into() => 9,
-                action: Action::Submit {
+            "c".into() => 10,
-                    text: None,
-                    keys: vec!(KeySym("a".into()), KeySym("c".into())),
-                },
-                keycodes: vec!(9, 10),
-                pressed: PressType::Released,
-            },
         }).unwrap();
 
         let keymap = xkb::Keymap::new_from_string(

tests/layout_erase.yaml

@@ -0,0 +1,10 @@
+---
+# Erase only
+views:
+    base:
+        - "BackSpace"
+outlines:
+    default: { width: 0, height: 0 }
+buttons:
+    BackSpace:
+        action: erase