From 3c0b142c4fe9b33af02ebee84cace8207b0c4cf9 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Tue, 19 Nov 2019 08:29:57 +0000 Subject: [PATCH] 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::*;