From 3c0b142c4fe9b33af02ebee84cace8207b0c4cf9 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 19 Nov 2019 08:29:57 +0000 Subject: [PATCH 1/5] warnings: Print at runtime, crash at test time --- examples/test_layout.rs | 37 ++++++++-- src/data.rs | 150 ++++++++++++++++++++++++++-------------- src/lib.rs | 2 +- src/util.rs | 5 ++ 4 files changed, 134 insertions(+), 60 deletions(-) diff --git a/examples/test_layout.rs b/examples/test_layout.rs index b8fe0a1b..fec1284f 100644 --- a/examples/test_layout.rs +++ b/examples/test_layout.rs @@ -3,16 +3,37 @@ extern crate xkbcommon; use std::env; -use rs::data::{ Layout, LoadError }; - +use rs::data::Layout; use xkbcommon::xkb; +use rs::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) + } +} fn check_layout(name: &str) { - let layout = Layout::from_resource(name) - .and_then(|layout| layout.build().map_err(LoadError::BadKeyMap)) - .expect("layout broken"); - + let handler = CountAndPrint::new(); + let layout = Layout::from_resource(name).expect("Invalid layout file"); + 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 @@ -45,6 +66,10 @@ fn check_layout(name: &str) { } } } + + if handler.0 > 0 { + panic!("Layout contains mistakes"); + } } fn main() -> () { diff --git a/src/data.rs b/src/data.rs index 85276346..541fc2d9 100644 --- a/src/data.rs +++ b/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) ) }, } @@ -305,8 +314,8 @@ impl Layout { serde_yaml::from_reader(infile).map_err(Error::Yaml) } - pub fn build(self) - -> Result<::layout::LayoutData, FormattingError> + pub fn build(self, mut warning_handler: H) + -> (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 - = HashMap::::from_iter( - button_states - ); + let button_states = HashMap::::from_iter( + 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, - keymap_str: { - CString::new(keymap_str) - .expect("Invalid keymap string generated") - }, - }) + ( + Ok(::layout::LayoutData { + views: views, + keymap_str: { + CString::new(keymap_str) + .expect("Invalid keymap string generated") + }, + }), + warning_handler, + ) } } -fn create_action( +fn create_action( button_info: &HashMap, 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( + 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( button_info: &HashMap, outlines: &HashMap, name: &str, state: Rc>, + 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())), - } + }, ); } } diff --git a/src/lib.rs b/src/lib.rs index 21bfde8a..54fc66b7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,5 +24,5 @@ mod outputs; mod popover; mod resources; mod submission; -mod util; +pub mod util; mod xdg; diff --git a/src/util.rs b/src/util.rs index baec594e..1a86330d 100644 --- a/src/util.rs +++ b/src/util.rs @@ -177,6 +177,11 @@ impl Borrow> for Pointer { } } +pub trait WarningHandler { + /// Handle a warning + fn handle(&mut self, warning: &str); +} + #[cfg(test)] mod tests { use super::*; From f834f174d80f76abcbe801fd628ea1f921c703ba Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 19 Nov 2019 09:35:03 +0000 Subject: [PATCH 2/5] cargo: Copy target with properties, find filename automatically --- cargo.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cargo.sh b/cargo.sh index ef4da1f6..2a01532a 100755 --- a/cargo.sh +++ b/cargo.sh @@ -20,5 +20,6 @@ shift cargo "$@" if [ -n "${OUT_PATH}" ]; then - cp "${CARGO_TARGET_DIR}"/debug/librs.a "${OUT_PATH}" + FILENAME="$(basename "${OUT_PATH}")" + cp -a "${CARGO_TARGET_DIR}"/debug/"${FILENAME}" "${OUT_PATH}" fi From 9571adb1072115126c11402fc85f62c87eb8e1ac Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 19 Nov 2019 09:47:32 +0000 Subject: [PATCH 3/5] tests: Executable for testing layouts --- examples/test_layout.rs | 75 +++------------------------------------ src/bin/test_layout.rs | 8 +++++ src/data.rs | 2 +- src/lib.rs | 1 + src/meson.build | 12 ++++++- src/tests.rs | 78 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 103 insertions(+), 73 deletions(-) create mode 100644 src/bin/test_layout.rs create mode 100644 src/tests.rs diff --git a/examples/test_layout.rs b/examples/test_layout.rs index fec1284f..e2f0dc25 100644 --- a/examples/test_layout.rs +++ b/examples/test_layout.rs @@ -1,77 +1,10 @@ extern crate rs; -extern crate xkbcommon; +use rs::tests::check_builtin_layout; use std::env; -use rs::data::Layout; -use xkbcommon::xkb; - -use rs::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) - } -} - -fn check_layout(name: &str) { - let handler = CountAndPrint::new(); - let layout = Layout::from_resource(name).expect("Invalid layout file"); - 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"); - } -} - 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() + ); } diff --git a/src/bin/test_layout.rs b/src/bin/test_layout.rs new file mode 100644 index 00000000..3ec5f366 --- /dev/null +++ b/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()); +} diff --git a/src/data.rs b/src/data.rs index 541fc2d9..f7ca6865 100644 --- a/src/data.rs +++ b/src/data.rs @@ -305,7 +305,7 @@ impl Layout { .map_err(LoadError::BadResource) } - fn from_file(path: PathBuf) -> Result { + pub fn from_file(path: PathBuf) -> Result { let infile = BufReader::new( fs::OpenOptions::new() .read(true) diff --git a/src/lib.rs b/src/lib.rs index 54fc66b7..d1b7f1b2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,5 +24,6 @@ mod outputs; mod popover; mod resources; mod submission; +pub mod tests; pub mod util; mod xdg; diff --git a/src/meson.build b/src/meson.build index e1a98ab9..525fba06 100644 --- a/src/meson.build +++ b/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@', 'build', '--lib'] ) build_rstests = custom_target( @@ -124,3 +124,13 @@ 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: ['test_layout'], + console: true, + command: [cargo_script, '@OUTPUT@', 'build', '--bin', 'test_layout'], + install_dir: bindir, +) diff --git a/src/tests.rs b/src/tests.rs new file mode 100644 index 00000000..19922fe7 --- /dev/null +++ b/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"); + } +} From 1db561d33aa458f57c9b7e6e280d3c44677974bf Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 19 Nov 2019 10:01:21 +0000 Subject: [PATCH 4/5] build: Handle output files better --- cargo.sh | 14 ++++++++++---- src/meson.build | 6 +++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cargo.sh b/cargo.sh index 2a01532a..ae3a3c08 100755 --- a/cargo.sh +++ b/cargo.sh @@ -11,15 +11,21 @@ SOURCE_DIR="$(dirname "$SCRIPT_PATH")" CARGO_TARGET_DIR="$(pwd)" export CARGO_TARGET_DIR -if [ -n "${1}" ]; then - OUT_PATH="$(realpath "$1")" +if [ "${1}" = "--rename" ]; then + 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 - FILENAME="$(basename "${OUT_PATH}")" cp -a "${CARGO_TARGET_DIR}"/debug/"${FILENAME}" "${OUT_PATH}" fi diff --git a/src/meson.build b/src/meson.build index 525fba06..f17b1622 100644 --- a/src/meson.build +++ b/src/meson.build @@ -58,7 +58,7 @@ rslibs = custom_target( output: ['librs.a'], install: false, console: true, - command: [cargo_script, '@OUTPUT@', 'build', '--lib'] + command: [cargo_script, '--output', '@OUTPUT@', 'build', '--lib'] ) build_rstests = custom_target( @@ -129,8 +129,8 @@ test_layout = custom_target('squeekboard_test_layout', build_by_default: true, # meson doesn't track all inputs, cargo does build_always_stale: true, - output: ['test_layout'], + output: ['squeekboard_test_layout'], console: true, - command: [cargo_script, '@OUTPUT@', 'build', '--bin', 'test_layout'], + command: [cargo_script, '--rename', 'test_layout', '@OUTPUT@', 'build', '--bin', 'test_layout'], install_dir: bindir, ) From 9b271a69190c8f2e8c23f5b7674fe0d1ea8d9548 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 19 Nov 2019 10:01:38 +0000 Subject: [PATCH 5/5] devel: Package squeekboard-test-layout --- debian/control | 10 ++++++++++ debian/squeekboard-devel.install | 1 + debian/squeekboard.install | 2 ++ src/meson.build | 5 +++-- 4 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 debian/squeekboard-devel.install create mode 100644 debian/squeekboard.install diff --git a/debian/control b/debian/control index 69615a26..f1031040 100644 --- a/debian/control +++ b/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 diff --git a/debian/squeekboard-devel.install b/debian/squeekboard-devel.install new file mode 100644 index 00000000..eca02621 --- /dev/null +++ b/debian/squeekboard-devel.install @@ -0,0 +1 @@ +usr/bin/squeekboard-test-layout /usr/bin diff --git a/debian/squeekboard.install b/debian/squeekboard.install new file mode 100644 index 00000000..c87f05b7 --- /dev/null +++ b/debian/squeekboard.install @@ -0,0 +1,2 @@ +usr/bin/squeekboard-real /usr/bin +usr/bin/squeekboard /usr/bin diff --git a/src/meson.build b/src/meson.build index f17b1622..36f817bb 100644 --- a/src/meson.build +++ b/src/meson.build @@ -125,12 +125,13 @@ squeekboard = executable('squeekboard-real', '-DEEK_COMPILATION=1'], ) -test_layout = custom_target('squeekboard_test_layout', +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'], + output: ['squeekboard-test-layout'], console: true, command: [cargo_script, '--rename', 'test_layout', '@OUTPUT@', 'build', '--bin', 'test_layout'], + install: true, install_dir: bindir, )