Merge branch 'errors' into 'master'

Better layout checking

Closes #131

See merge request Librem5/squeekboard!255

cargo.sh

@@ -11,14 +11,21 @@ SOURCE_DIR="$(dirname "$SCRIPT_PATH")"
 
 CARGO_TARGET_DIR="$(pwd)"
 export CARGO_TARGET_DIR
-if [ -n "${1}" ]; then
+if [ "${1}" = "--rename" ]; then
-    OUT_PATH="$(realpath "$1")"
+    shift
+    FILENAME="${1}"
+    shift
+    OUT_PATH="$(realpath "${1}")"
+elif [ "${1}" = "--output" ]; then
+    shift
+    OUT_PATH="$(realpath "${1}")"
+    FILENAME="$(basename "${OUT_PATH}")"
 fi
+shift
 
 cd "$SOURCE_DIR"
-shift
 cargo "$@"
 
 if [ -n "${OUT_PATH}" ]; then
-    cp "${CARGO_TARGET_DIR}"/debug/librs.a "${OUT_PATH}"
+    cp -a "${CARGO_TARGET_DIR}"/debug/"${FILENAME}" "${OUT_PATH}"
 fi

debian/control

@@ -40,3 +40,13 @@ Depends:
  ${misc:Depends}
 Description: On-screen keyboard for Wayland
  Virtual keyboard supporting Wayland, built primarily for the Librem 5 phone.
+
+Package: squeekboard-devel
+Architecture: linux-any
+Depends:
+ ${shlibs:Depends}
+ ${misc:Depends}
+Description: Resources for making Squeekboard layouts
+ Tools for creating Squeekboard layouts:
+ .
+  * squeekboard-test-layout

debian/squeekboard-devel.install

@@ -0,0 +1 @@
+usr/bin/squeekboard-test-layout /usr/bin

debian/squeekboard.install

@@ -0,0 +1,2 @@
+usr/bin/squeekboard-real /usr/bin
+usr/bin/squeekboard /usr/bin

examples/test_layout.rs

@@ -1,52 +1,10 @@
 extern crate rs;
-extern crate xkbcommon;
 
+use rs::tests::check_builtin_layout;
 use std::env;
 
-use rs::data::{ Layout, LoadError };
-
-use xkbcommon::xkb;
-
-
-fn check_layout(name: &str) {
-    let layout = Layout::from_resource(name)
-        .and_then(|layout| layout.build().map_err(LoadError::BadKeyMap))
-        .expect("layout broken");
-    
-    let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS);
-    
-    let keymap_str = layout.keymap_str
-        .clone()
-        .into_string().expect("Failed to decode keymap string");
-    
-    let keymap = xkb::Keymap::new_from_string(
-        &context,
-        keymap_str.clone(),
-        xkb::KEYMAP_FORMAT_TEXT_V1,
-        xkb::KEYMAP_COMPILE_NO_FLAGS,
-    ).expect("Failed to create keymap");
-
-    let state = xkb::State::new(&keymap);
-    
-    // "Press" each button with keysyms
-    for view in layout.views.values() {
-        for row in &view.rows {
-            for button in &row.buttons {
-                let keystate = button.state.borrow();
-                for keycode in &keystate.keycodes {
-                    match state.key_get_one_sym(*keycode) {
-                        xkb::KEY_NoSymbol => {
-                            eprintln!("{}", keymap_str);
-                            panic!("Keysym {} on key {:?} can't be resolved", keycode, button.name);
-                        },
-                        _ => {},
-                    }
-                }
-            }
-        }
-    }
-}
-
 fn main() -> () {
-    check_layout(env::args().nth(1).expect("No argument given").as_str());
+    check_builtin_layout(
+        env::args().nth(1).expect("No argument given").as_str()
+    );
 }

src/bin/test_layout.rs

@@ -0,0 +1,8 @@
+extern crate rs;
+
+use rs::tests::check_layout_file;
+use std::env;
+
+fn main() -> () {
+    check_layout_file(env::args().nth(1).expect("No argument given").as_str());
+}

src/data.rs

@@ -26,8 +26,8 @@ use ::xdg;
 // traits, derives
 use std::io::BufReader;
 use std::iter::FromIterator;
-
 use serde::Deserialize;
+use util::WarningHandler;
 
 
 /// Gathers stuff defined in C or called by C
@@ -151,21 +151,30 @@ fn list_layout_sources(
     ret
 }
 
+struct PrintWarnings;
+
+impl WarningHandler for PrintWarnings {
+    fn handle(&mut self, warning: &str) {
+        println!("{}", warning);
+    }
+}
+
 fn load_layout_data(source: DataSource)
     -> Result<::layout::LayoutData, LoadError>
 {
+    let handler = PrintWarnings{};
     match source {
         DataSource::File(path) => {
             Layout::from_file(path.clone())
                 .map_err(LoadError::BadData)
                 .and_then(|layout|
-                    layout.build().map_err(LoadError::BadKeyMap)
+                    layout.build(handler).0.map_err(LoadError::BadKeyMap)
                 )
         },
         DataSource::Resource(name) => {
             Layout::from_resource(&name)
                 .and_then(|layout|
-                    layout.build().map_err(LoadError::BadKeyMap)
+                    layout.build(handler).0.map_err(LoadError::BadKeyMap)
                 )
         },
     }
@@ -296,7 +305,7 @@ impl Layout {
                     .map_err(LoadError::BadResource)
     }
 
-    fn from_file(path: PathBuf) -> Result<Layout, Error> {
+    pub fn from_file(path: PathBuf) -> Result<Layout, Error> {
         let infile = BufReader::new(
             fs::OpenOptions::new()
                 .read(true)
@@ -305,8 +314,8 @@ impl Layout {
         serde_yaml::from_reader(infile).map_err(Error::Yaml)
     }
 
-    pub fn build(self)
+    pub fn build<H: WarningHandler>(self, mut warning_handler: H)
-        -> Result<::layout::LayoutData, FormattingError>
+        -> (Result<::layout::LayoutData, FormattingError>, H)
     {
         let button_names = self.views.values()
             .flat_map(|rows| {
@@ -323,7 +332,8 @@ impl Layout {
                 create_action(
                     &self.buttons,
                     name,
-                    self.views.keys().collect()
+                    self.views.keys().collect(),
+                    &mut warning_handler,
                 )
             )}).collect();
 
@@ -368,13 +378,15 @@ impl Layout {
             )
         });
 
-        let button_states
+        let button_states = HashMap::<String, KeyState>::from_iter(
-            = HashMap::<String, KeyState>::from_iter(
+            button_states
-                button_states
+        );
-            );
 
         // TODO: generate from symbols
-        let keymap_str = generate_keymap(&button_states)?;
+        let keymap_str = match generate_keymap(&button_states) {
+            Err(e) => { return (Err(e), warning_handler) },
+            Ok(v) => v,
+        };
 
         let button_states_cache = hash_map_map(
             button_states,
@@ -405,7 +417,8 @@ impl Layout {
                                     name,
                                     button_states_cache.get(name.into())
                                         .expect("Button state not created")
-                                        .clone()
+                                        .clone(),
+                                    &mut warning_handler,
                                 ))
                             }).collect(),
                         })
@@ -414,42 +427,29 @@ impl Layout {
             )})
         );
 
-        Ok(::layout::LayoutData {
+        (
-            views: views,
+            Ok(::layout::LayoutData {
-            keymap_str: {
+                views: views,
-                CString::new(keymap_str)
+                keymap_str: {
-                    .expect("Invalid keymap string generated")
+                    CString::new(keymap_str)
-            },
+                        .expect("Invalid keymap string generated")
-        })
+                },
+            }),
+            warning_handler,
+        )
     }
 }
 
-fn create_action(
+fn create_action<H: WarningHandler>(
     button_info: &HashMap<String, ButtonMeta>,
     name: &str,
     view_names: Vec<&String>,
+    warning_handler: &mut H,
 ) -> ::action::Action {
     let default_meta = ButtonMeta::default();
     let symbol_meta = button_info.get(name)
         .unwrap_or(&default_meta);
-    
+
-    fn filter_view_name(
-        button_name: &str,
-        view_name: String,
-        view_names: &Vec<&String>
-    ) -> String {
-        if view_names.contains(&&view_name) {
-            view_name
-        } else {
-            eprintln!(
-                "Button {} switches to missing view {}",
-                button_name,
-                view_name
-            );
-            "base".into()
-        }
-    }
-    
     fn keysym_valid(name: &str) -> bool {
         xkb::keysym_from_name(name, xkb::KEYSYM_NO_FLAGS) != xkb::KEY_NoSymbol
     }
@@ -463,7 +463,10 @@ fn create_action(
             Some(keysym) => vec!(match keysym_valid(keysym.as_str()) {
                 true => keysym.clone(),
                 false => {
-                    eprintln!("Keysym name invalid: {}", keysym);
+                    warning_handler.handle(&format!(
+                        "Keysym name invalid: {}",
+                        keysym,
+                    ));
                     "space".into() // placeholder
                 },
             }),
@@ -479,7 +482,10 @@ fn create_action(
                     if name.chars().count() == 0 {
                         // A name read from yaml with no valid Unicode.
                         // Highly improbable, but let's be safe.
-                        eprintln!("Key {} doesn't have any characters", name);
+                        warning_handler.handle(&format!(
+                            "Key {} doesn't have any characters",
+                            name,
+                        ));
                         vec!("space".into()) // placeholder
                     } else {
                         name.chars().map(|codepoint| {
@@ -494,19 +500,45 @@ fn create_action(
             },
         },
     };
-    
+
+    fn filter_view_name<H: WarningHandler>(
+        button_name: &str,
+        view_name: String,
+        view_names: &Vec<&String>,
+        warning_handler: &mut H,
+    ) -> String {
+        if view_names.contains(&&view_name) {
+            view_name
+        } else {
+            warning_handler.handle(&format!("Button {} switches to missing view {}",
+                button_name,
+                view_name,
+            ));
+            "base".into()
+        }
+    }
+
     match &symbol_meta.action {
         Some(Action::SetView(view_name)) => ::action::Action::SetLevel(
-            filter_view_name(name, view_name.clone(), &view_names)
+            filter_view_name(
+                name, view_name.clone(), &view_names,
+                warning_handler,
+            )
         ),
         Some(Action::Locking {
             lock_view, unlock_view
         }) => ::action::Action::LockLevel {
-            lock: filter_view_name(name, lock_view.clone(), &view_names),
+            lock: filter_view_name(
+                name,
+                lock_view.clone(),
+                &view_names,
+                warning_handler,
+            ),
             unlock: filter_view_name(
                 name,
                 unlock_view.clone(),
-                &view_names
+                &view_names,
+                warning_handler,
             ),
         },
         Some(Action::ShowPrefs) => ::action::Action::ShowPreferences,
@@ -519,11 +551,12 @@ fn create_action(
 
 /// TODO: Since this will receive user-provided data,
 /// all .expect() on them should be turned into soft fails
-fn create_button(
+fn create_button<H: WarningHandler>(
     button_info: &HashMap<String, ButtonMeta>,
     outlines: &HashMap<String, Outline>,
     name: &str,
     state: Rc<RefCell<KeyState>>,
+    warning_handler: &mut H,
 ) -> ::layout::Button {
     let cname = CString::new(name.clone())
         .expect("Bad name");
@@ -548,7 +581,7 @@ fn create_button(
             if outlines.contains_key(outline) {
                 outline.clone()
             } else {
-                eprintln!("Outline named {} does not exist! Using default for button {}", outline, name);
+                warning_handler.handle(&format!("Outline named {} does not exist! Using default for button {}", outline, name));
                 "default".into()
             }
         }
@@ -558,7 +591,9 @@ fn create_button(
     let outline = outlines.get(&outline_name)
         .map(|outline| (*outline).clone())
         .unwrap_or_else(|| {
-            eprintln!("No default outline defied Using 1x1!");
+            warning_handler.handle(
+                &format!("No default outline defined! Using 1x1!")
+            );
             Outline {
                 bounds: Bounds { x: 0f64, y: 0f64, width: 1f64, height: 1f64 },
             }
@@ -585,6 +620,14 @@ mod tests {
     
     use std::error::Error as ErrorTrait;
 
+    struct PanicWarn;
+    
+    impl WarningHandler for PanicWarn {
+        fn handle(&mut self, warning: &str) {
+            panic!("{}", warning);
+        }
+    }
+
     #[test]
     fn test_parse_path() {
         assert_eq!(
@@ -656,7 +699,7 @@ mod tests {
     fn test_layout_punctuation() {
         let out = Layout::from_file(PathBuf::from("tests/layout_key1.yaml"))
             .unwrap()
-            .build()
+            .build(PanicWarn).0
             .unwrap();
         assert_eq!(
             out.views["base"]
@@ -671,7 +714,7 @@ mod tests {
     fn test_layout_unicode() {
         let out = Layout::from_file(PathBuf::from("tests/layout_key2.yaml"))
             .unwrap()
-            .build()
+            .build(PanicWarn).0
             .unwrap();
         assert_eq!(
             out.views["base"]
@@ -687,7 +730,7 @@ mod tests {
     fn test_layout_unicode_multi() {
         let out = Layout::from_file(PathBuf::from("tests/layout_key3.yaml"))
             .unwrap()
-            .build()
+            .build(PanicWarn).0
             .unwrap();
         assert_eq!(
             out.views["base"]
@@ -702,7 +745,7 @@ mod tests {
     #[test]
     fn parsing_fallback() {
         assert!(Layout::from_resource(FALLBACK_LAYOUT_NAME)
-            .and_then(|layout| layout.build().map_err(LoadError::BadKeyMap))
+            .map(|layout| layout.build(PanicWarn).0.unwrap())
             .is_ok()
         );
     }
@@ -748,12 +791,13 @@ mod tests {
                     }
                 },
                 ".",
-                Vec::new()
+                Vec::new(),
+                &mut PanicWarn,
             ),
             ::action::Action::Submit {
                 text: None,
                 keys: vec!(::action::KeySym("U002E".into())),
-            }
+            },
         );
     }
 }

src/lib.rs

@@ -24,5 +24,6 @@ mod outputs;
 mod popover;
 mod resources;
 mod submission;
-mod util;
+pub mod tests;
+pub mod util;
 mod xdg;

src/meson.build

@@ -58,7 +58,7 @@ rslibs = custom_target(
     output: ['librs.a'],
     install: false,
     console: true,
-    command: [cargo_script, '@OUTPUT@', 'build']
+    command: [cargo_script, '--output', '@OUTPUT@', 'build', '--lib']
 )
 
 build_rstests = custom_target(
@@ -124,3 +124,14 @@ squeekboard = executable('squeekboard-real',
     '-DEEKBOARD_COMPILATION=1',
     '-DEEK_COMPILATION=1'],
 )
+
+test_layout = custom_target('squeekboard-test-layout',
+    build_by_default: true,
+    # meson doesn't track all inputs, cargo does
+    build_always_stale: true,
+    output: ['squeekboard-test-layout'],
+    console: true,
+    command: [cargo_script, '--rename', 'test_layout', '@OUTPUT@', 'build', '--bin', 'test_layout'],
+    install: true,
+    install_dir: bindir,
+)

src/tests.rs

@@ -0,0 +1,78 @@
+/*! Testing functionality */
+
+use ::data::Layout;
+use xkbcommon::xkb;
+
+use ::util::WarningHandler;
+
+
+pub struct CountAndPrint(u32);
+
+impl WarningHandler for CountAndPrint {
+    fn handle(&mut self, warning: &str) {
+        self.0 = self.0 + 1;
+        println!("{}", warning);
+    }
+}
+
+impl CountAndPrint {
+    fn new() -> CountAndPrint {
+        CountAndPrint(0)
+    }
+}
+
+pub fn check_builtin_layout(name: &str) {
+    check_layout(Layout::from_resource(name).expect("Invalid layout data"))
+}
+
+pub fn check_layout_file(path: &str) {
+    check_layout(Layout::from_file(path.into()).expect("Invalid layout file"))
+}
+
+fn check_layout(layout: Layout) {
+    let handler = CountAndPrint::new();
+    let (layout, handler) = layout.build(handler);
+
+    if handler.0 > 0 {
+        println!("{} mistakes in layout", handler.0)
+    }
+
+    let layout = layout.expect("layout broken");
+
+    let context = xkb::Context::new(xkb::CONTEXT_NO_FLAGS);
+    
+    let keymap_str = layout.keymap_str
+        .clone()
+        .into_string().expect("Failed to decode keymap string");
+    
+    let keymap = xkb::Keymap::new_from_string(
+        &context,
+        keymap_str.clone(),
+        xkb::KEYMAP_FORMAT_TEXT_V1,
+        xkb::KEYMAP_COMPILE_NO_FLAGS,
+    ).expect("Failed to create keymap");
+
+    let state = xkb::State::new(&keymap);
+    
+    // "Press" each button with keysyms
+    for view in layout.views.values() {
+        for row in &view.rows {
+            for button in &row.buttons {
+                let keystate = button.state.borrow();
+                for keycode in &keystate.keycodes {
+                    match state.key_get_one_sym(*keycode) {
+                        xkb::KEY_NoSymbol => {
+                            eprintln!("{}", keymap_str);
+                            panic!("Keysym {} on key {:?} can't be resolved", keycode, button.name);
+                        },
+                        _ => {},
+                    }
+                }
+            }
+        }
+    }
+
+    if handler.0 > 0 {
+        panic!("Layout contains mistakes");
+    }
+}

src/util.rs

@@ -177,6 +177,11 @@ impl<T> Borrow<Rc<T>> for Pointer<T> {
     }
 }
 
+pub trait WarningHandler {
+    /// Handle a warning
+    fn handle(&mut self, warning: &str);
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;