From ea84f4f0310a95e2ea9b3ab1fc2c5586a7a25abe Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Thu, 16 Jan 2020 15:57:46 +0000 Subject: [PATCH 1/4] logging: Try to improve common operations This adds sugar for logging `Result`s with a handler, makes names evoke something closer to "logging" than "warning", tries to remove any redundant `Logging` where the module name will do, and introduces a type strictly for bad things happening. --- src/data.rs | 106 ++++++++++++++++++++++---------------------- src/logging.rs | 117 +++++++++++++++++++++++++++++++++++++++---------- src/popover.rs | 8 +++- src/style.rs | 7 ++- src/tests.rs | 19 +++++--- 5 files changed, 168 insertions(+), 89 deletions(-) diff --git a/src/data.rs b/src/data.rs index 0fa30751..bd32ed88 100644 --- a/src/data.rs +++ b/src/data.rs @@ -21,7 +21,7 @@ use ::keyboard::{ }; use ::layout; use ::layout::ArrangementKind; -use ::logging::PrintWarnings; +use ::logging; use ::resources; use ::util::c::as_str; use ::util::hash_map_map; @@ -31,7 +31,7 @@ use ::xdg; use serde::Deserialize; use std::io::BufReader; use std::iter::FromIterator; -use ::logging::WarningHandler; +use ::logging::Warn; /// Gathers stuff defined in C or called by C pub mod c { @@ -157,7 +157,7 @@ fn list_layout_sources( fn load_layout_data(source: DataSource) -> Result<::layout::LayoutData, LoadError> { - let handler = PrintWarnings{}; + let handler = logging::Print {}; match source { DataSource::File(path) => { Layout::from_file(path.clone()) @@ -330,7 +330,7 @@ impl Layout { serde_yaml::from_reader(infile).map_err(Error::Yaml) } - pub fn build(self, mut warning_handler: H) + pub fn build(self, mut warning_handler: H) -> (Result<::layout::LayoutData, FormattingError>, H) { let button_names = self.views.values() @@ -464,7 +464,7 @@ impl Layout { } } -fn create_action( +fn create_action( button_info: &HashMap, name: &str, view_names: Vec<&String>, @@ -494,15 +494,18 @@ fn create_action( (None, None, Some(text)) => SubmitData::Text(text.clone()), (None, None, None) => SubmitData::Text(name.into()), _ => { - warning_handler.handle(&format!( - "Button {} has more than one of (action, keysym, text)", - name - )); + warning_handler.handle( + logging::Level::Warning, + &format!( + "Button {} has more than one of (action, keysym, text)", + name, + ), + ); SubmitData::Text("".into()) }, }; - fn filter_view_name( + fn filter_view_name( button_name: &str, view_name: String, view_names: &Vec<&String>, @@ -511,10 +514,13 @@ fn create_action( if view_names.contains(&&view_name) { view_name } else { - warning_handler.handle(&format!("Button {} switches to missing view {}", - button_name, - view_name, - )); + warning_handler.handle( + logging::Level::Warning, + &format!("Button {} switches to missing view {}", + button_name, + view_name, + ), + ); "base".into() } } @@ -553,27 +559,24 @@ fn create_action( match keysym_valid(keysym.as_str()) { true => keysym.clone(), false => { - warning_handler.handle(&format!( - "Keysym name invalid: {}", - keysym, - )); + warning_handler.handle( + logging::Level::Warning, + &format!( + "Keysym name invalid: {}", + keysym, + ), + ); "space".into() // placeholder }, } )), }, SubmitData::Text(text) => ::action::Action::Submit { - text: { - CString::new(text.clone()) - .map_err(|e| { - warning_handler.handle(&format!( - "Text {} contains problems: {:?}", - text, - e - )); - e - }).ok() - }, + text: CString::new(text.clone()).or_warn( + warning_handler, + logging::Problem::Warning, + &format!("Text {} contains problems", text), + ), keys: text.chars().map(|codepoint| { let codepoint_string = codepoint.to_string(); ::action::KeySym(match keysym_valid(codepoint_string.as_str()) { @@ -587,7 +590,7 @@ 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, @@ -611,14 +614,11 @@ fn create_button( } else if let Some(text) = &button_meta.text { ::layout::Label::Text( CString::new(text.as_str()) - .unwrap_or_else(|e| { - warning_handler.handle(&format!( - "Text {} is invalid: {}", - text, - e, - )); - CString::new("").unwrap() - }) + .or_warn( + warning_handler, + logging::Problem::Warning, + &format!("Text {} is invalid", text), + ).unwrap_or_else(|| CString::new("").unwrap()) ) } else { ::layout::Label::Text(cname.clone()) @@ -629,7 +629,10 @@ fn create_button( if outlines.contains_key(outline) { outline.clone() } else { - warning_handler.handle(&format!("Outline named {} does not exist! Using default for button {}", outline, name)); + warning_handler.handle( + logging::Level::Warning, + &format!("Outline named {} does not exist! Using default for button {}", outline, name) + ); "default".into() } } @@ -638,12 +641,11 @@ fn create_button( let outline = outlines.get(&outline_name) .map(|outline| (*outline).clone()) - .unwrap_or_else(|| { - warning_handler.handle( - &format!("No default outline defined! Using 1x1!") - ); - Outline { width: 1f64, height: 1f64 } - }); + .or_warn( + warning_handler, + logging::Problem::Warning, + "No default outline defined! Using 1x1!", + ).unwrap_or(Outline { width: 1f64, height: 1f64 }); layout::Button { name: cname, @@ -663,7 +665,7 @@ mod tests { use super::*; use std::error::Error as ErrorTrait; - use ::logging::PanicWarn; + use ::logging::ProblemPanic; #[test] fn test_parse_path() { @@ -733,7 +735,7 @@ mod tests { fn test_layout_punctuation() { let out = Layout::from_file(PathBuf::from("tests/layout_key1.yaml")) .unwrap() - .build(PanicWarn).0 + .build(ProblemPanic).0 .unwrap(); assert_eq!( out.views["base"] @@ -748,7 +750,7 @@ mod tests { fn test_layout_unicode() { let out = Layout::from_file(PathBuf::from("tests/layout_key2.yaml")) .unwrap() - .build(PanicWarn).0 + .build(ProblemPanic).0 .unwrap(); assert_eq!( out.views["base"] @@ -764,7 +766,7 @@ mod tests { fn test_layout_unicode_multi() { let out = Layout::from_file(PathBuf::from("tests/layout_key3.yaml")) .unwrap() - .build(PanicWarn).0 + .build(ProblemPanic).0 .unwrap(); assert_eq!( out.views["base"] @@ -779,7 +781,7 @@ mod tests { #[test] fn parsing_fallback() { assert!(Layout::from_resource(FALLBACK_LAYOUT_NAME) - .map(|layout| layout.build(PanicWarn).0.unwrap()) + .map(|layout| layout.build(ProblemPanic).0.unwrap()) .is_ok() ); } @@ -827,7 +829,7 @@ mod tests { }, ".", Vec::new(), - &mut PanicWarn, + &mut ProblemPanic, ), ::action::Action::Submit { text: Some(CString::new(".").unwrap()), @@ -840,7 +842,7 @@ mod tests { fn test_layout_margins() { let out = Layout::from_file(PathBuf::from("tests/layout_margins.yaml")) .unwrap() - .build(PanicWarn).0 + .build(ProblemPanic).0 .unwrap(); assert_eq!( out.margins, diff --git a/src/logging.rs b/src/logging.rs index 7beb9ff1..ba0ed2b8 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -26,13 +26,15 @@ * 4. logging to an immutable destination type * * Same as above, except it can be parallelized. + * Logs being outputs, they get returned + * instead of being misleadingly passed back through arguments. * It seems more difficult to pass the logger around, * but this may be a solved problem from the area of functional programming. * * This library generally aims at the approach in 3. * */ -use std::error::Error; +use std::fmt::Display; /// Levels are not in order. pub enum Level { @@ -66,18 +68,72 @@ pub enum Level { Debug, } -/// Sugar for logging errors in results. -/// Approach 2. -pub trait Warn { - type Value; - fn or_warn(self, msg: &str) -> Option; +impl Level { + fn as_str(&self) -> &'static str { + match self { + Level::Panic => "Panic", + Level::Bug => "Bug", + Level::Error => "Error", + Level::Warning => "Warning", + Level::Surprise => "Surprise", + Level::Info => "Info", + Level::Debug => "Debug", + } + } } -impl Warn for Result { +impl From for Level { + fn from(problem: Problem) -> Level { + use self::Level::*; + match problem { + Problem::Panic => Panic, + Problem::Bug => Bug, + Problem::Error => Error, + Problem::Warning => Warning, + Problem::Surprise => Surprise, + } + } +} + +/// Only levels which indicate problems +/// To use with `Result::Err` handlers, +/// which are needed only when something went off the optimal path. +/// A separate type ensures that `Err` +/// can't end up misclassified as a benign event like `Info`. +pub enum Problem { + Panic, + Bug, + Error, + Warning, + Surprise, +} + +/// Sugar for logging errors in results. +pub trait Warn where Self: Sized{ + type Value; + /// Approach 2. + fn or_print(self, level: Problem, message: &str) -> Option { + self.or_warn(&mut Print {}, level.into(), message) + } + /// Approach 3. + fn or_warn( + self, + handler: &mut H, + level: Problem, + message: &str, + ) -> Option; +} + +impl Warn for Result { type Value = T; - fn or_warn(self, msg: &str) -> Option { + fn or_warn( + self, + handler: &mut H, + level: Problem, + message: &str, + ) -> Option { self.map_err(|e| { - eprintln!("{}: {}", msg, e); + handler.handle(level.into(), &format!("{}: {}", message, e)); e }).ok() } @@ -85,9 +141,14 @@ impl Warn for Result { impl Warn for Option { type Value = T; - fn or_warn(self, msg: &str) -> Option { + fn or_warn( + self, + handler: &mut H, + level: Problem, + message: &str, + ) -> Option { self.or_else(|| { - eprintln!("{}", msg); + handler.handle(level.into(), message); None }) } @@ -95,26 +156,34 @@ impl Warn for Option { /// A mutable handler for text warnings. /// Approach 3. -pub trait WarningHandler { - /// Handle a warning - fn handle(&mut self, warning: &str); +pub trait Handler { + /// Handle a log message + fn handle(&mut self, level: Level, message: &str); } -/// Prints warnings to stderr -pub struct PrintWarnings; +/// Prints info to stdout, everything else to stderr +pub struct Print; -impl WarningHandler for PrintWarnings { - fn handle(&mut self, warning: &str) { - eprintln!("{}", warning); +impl Handler for Print { + fn handle(&mut self, level: Level, message: &str) { + match level { + Level::Info => println!("Info: {}", message), + l => eprintln!("{}: {}", l.as_str(), message), + } } } -/// Warning handler that will panic at any warning. +/// Warning handler that will panic +/// at any warning, error, surprise, bug, or panic. /// Don't use except in tests -pub struct PanicWarn; +pub struct ProblemPanic; -impl WarningHandler for PanicWarn { - fn handle(&mut self, warning: &str) { - panic!("{}", warning); +impl Handler for ProblemPanic { + fn handle(&mut self, level: Level, message: &str) { + use self::Level::*; + match level { + Panic | Bug | Error | Warning | Surprise => panic!("{}", message), + l => Print{}.handle(l, message), + } } } diff --git a/src/popover.rs b/src/popover.rs index c0b53086..6132b4ab 100644 --- a/src/popover.rs +++ b/src/popover.rs @@ -7,6 +7,7 @@ use ::layout::c::{ Bounds, EekGtkKeyboard }; use ::locale; use ::locale::{ OwnedTranslation, Translation, compare_current_locale }; use ::locale_config::system_locale; +use ::logging; use ::manager; use ::resources; @@ -242,10 +243,13 @@ fn translate_layout_names(layouts: &Vec) -> Vec { .as_ref() .to_owned() ) - .or_warn("No locale detected") + .or_print(logging::Problem::Surprise, "No locale detected") .and_then(|lang| { resources::get_layout_names(lang.as_str()) - .or_warn(&format!("No translations for locale {}", lang)) + .or_print( + logging::Problem::Surprise, + &format!("No translations for locale {}", lang), + ) }); match builtin_translations { diff --git a/src/style.rs b/src/style.rs index 381dd045..2e033a7e 100644 --- a/src/style.rs +++ b/src/style.rs @@ -19,6 +19,7 @@ /*! CSS data loading. */ use std::env; +use ::logging; use glib::object::ObjectExt; use logging::Warn; @@ -94,15 +95,13 @@ fn get_theme_name(settings: >k::Settings) -> GtkTheme { None => GtkTheme { name: { settings.get_property("gtk-theme-name") - // maybe TODO: is this worth a warning? - .or_warn("No theme name") + .or_print(logging::Problem::Surprise, "No theme name") .and_then(|value| value.get::()) .unwrap_or(DEFAULT_THEME_NAME.into()) }, variant: { settings.get_property("gtk-application-prefer-dark-theme") - // maybe TODO: is this worth a warning? - .or_warn("No settings key") + .or_print(logging::Problem::Surprise, "No settings key") .and_then(|value| value.get::()) .and_then(|dark_preferred| match dark_preferred { true => Some("dark".into()), diff --git a/src/tests.rs b/src/tests.rs index 3de54ccf..568fe29a 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -1,17 +1,22 @@ /*! Testing functionality */ use ::data::Layout; +use ::logging; use xkbcommon::xkb; -use ::logging::WarningHandler; - pub struct CountAndPrint(u32); -impl WarningHandler for CountAndPrint { - fn handle(&mut self, warning: &str) { - self.0 = self.0 + 1; - println!("{}", warning); +impl logging::Handler for CountAndPrint { + fn handle(&mut self, level: logging::Level, warning: &str) { + use logging::Level::*; + match level { + Panic | Bug | Error | Warning | Surprise => { + self.0 += 1; + }, + _ => {} + } + logging::Print{}.handle(level, warning) } } @@ -34,7 +39,7 @@ fn check_layout(layout: Layout) { let (layout, handler) = layout.build(handler); if handler.0 > 0 { - println!("{} mistakes in layout", handler.0) + println!("{} problems while parsing layout", handler.0) } let layout = layout.expect("layout broken"); From cc418c36095bb3edbd62a022467f99bbb3b12737 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Fri, 17 Jan 2020 11:59:47 +0000 Subject: [PATCH 2/4] imservice: Return something more resembling an Error on failure The error type is expected to be printable by logging utilities. --- src/imservice.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/imservice.rs b/src/imservice.rs index d3e4c5d0..2f39476a 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -1,5 +1,6 @@ use std::boxed::Box; use std::ffi::CString; +use std::fmt; use std::num::Wrapping; use std::string::String; @@ -246,10 +247,17 @@ pub enum ContentPurpose { Terminal = 13, } +// Utilities from ::logging need a printable error type +pub struct UnrecognizedValue; + +impl fmt::Display for UnrecognizedValue { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "Unrecognized value") + } +} + impl TryFrom for ContentPurpose { - // There's only one way to fail: number not in protocol, - // so no special error type is needed - type Error = (); + type Error = UnrecognizedValue; fn try_from(num: u32) -> Result { use self::ContentPurpose::*; match num { @@ -267,7 +275,7 @@ impl TryFrom for ContentPurpose { 11 => Ok(Time), 12 => Ok(Datetime), 13 => Ok(Terminal), - _ => Err(()), + _ => Err(UnrecognizedValue), } } } @@ -280,14 +288,12 @@ pub enum ChangeCause { } impl TryFrom for ChangeCause { - // There's only one way to fail: number not in protocol, - // so no special error type is needed - type Error = (); + type Error = UnrecognizedValue; fn try_from(num: u32) -> Result { match num { 0 => Ok(ChangeCause::InputMethod), 1 => Ok(ChangeCause::Other), - _ => Err(()) + _ => Err(UnrecognizedValue) } } } From c75e085dc8e1dc1755316b0568ab7451ad1c8488 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Fri, 17 Jan 2020 12:25:39 +0000 Subject: [PATCH 3/4] logging: Unified to remove random eprint calls --- src/data.rs | 6 ++++-- src/imservice.rs | 32 ++++++++++++++++++------------ src/keyboard.rs | 8 +++++++- src/layout.rs | 34 ++++++++++++++++++++++++-------- src/lib.rs | 4 +++- src/locale.rs | 7 +++++++ src/logging.rs | 14 ++++++++++++- src/outputs.rs | 51 ++++++++++++++++++++++++++++++++---------------- src/popover.rs | 25 ++++++++++++------------ src/style.rs | 5 ++++- 10 files changed, 130 insertions(+), 56 deletions(-) diff --git a/src/data.rs b/src/data.rs index bd32ed88..c190e54f 100644 --- a/src/data.rs +++ b/src/data.rs @@ -190,11 +190,13 @@ fn load_layout_data_with_fallback( ( LoadError::BadData(Error::Missing(e)), DataSource::File(file) - ) => eprintln!( // TODO: print in debug logging level + ) => log_print!( + logging::Level::Debug, "Tried file {:?}, but it's missing: {}", file, e ), - (e, source) => eprintln!( + (e, source) => log_print!( + logging::Level::Warning, "Failed to load layout from {}: {}, skipping", source, e ), diff --git a/src/imservice.rs b/src/imservice.rs index 2f39476a..d29c45a8 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -4,10 +4,12 @@ use std::fmt; use std::num::Wrapping; use std::string::String; +use ::logging; use ::util::c::into_cstring; // Traits use std::convert::TryFrom; +use ::logging::Warn; /// Gathers stuff defined in C or called by C @@ -102,16 +104,20 @@ pub mod c { let imservice = check_imservice(imservice, im).unwrap(); imservice.pending = IMProtocolState { content_hint: { - ContentHint::from_bits(hint).unwrap_or_else(|| { - eprintln!("Warning: received invalid hint flags"); - ContentHint::NONE - }) + ContentHint::from_bits(hint) + .or_print( + logging::Problem::Warning, + "Received invalid hint flags", + ) + .unwrap_or(ContentHint::NONE) }, content_purpose: { - ContentPurpose::try_from(purpose).unwrap_or_else(|_e| { - eprintln!("Warning: Received invalid purpose value"); - ContentPurpose::Normal - }) + ContentPurpose::try_from(purpose) + .or_print( + logging::Problem::Warning, + "Received invalid purpose value", + ) + .unwrap_or(ContentPurpose::Normal) }, ..imservice.pending.clone() }; @@ -126,10 +132,12 @@ pub mod c { let imservice = check_imservice(imservice, im).unwrap(); imservice.pending = IMProtocolState { text_change_cause: { - ChangeCause::try_from(cause).unwrap_or_else(|_e| { - eprintln!("Warning: received invalid cause value"); - ChangeCause::InputMethod - }) + ChangeCause::try_from(cause) + .or_print( + logging::Problem::Warning, + "Received invalid cause value", + ) + .unwrap_or(ChangeCause::InputMethod) }, ..imservice.pending.clone() }; diff --git a/src/keyboard.rs b/src/keyboard.rs index 79cd36c4..bff1ac16 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -7,6 +7,7 @@ use std::io; use std::string::FromUtf8Error; use ::action::Action; +use ::logging; use std::io::Write; use std::iter::{ FromIterator, IntoIterator }; @@ -110,7 +111,12 @@ pub fn generate_keymap( for (name, state) in keystates.iter() { if let Action::Submit { text: _, keys } = &state.action { - if let 0 = keys.len() { eprintln!("Key {} has no keysyms", name); }; + if let 0 = keys.len() { + log_print!( + logging::Level::Warning, + "Key {} has no keysyms", name, + ); + }; for (named_keysym, keycode) in keys.iter().zip(&state.keycodes) { write!( buf, diff --git a/src/layout.rs b/src/layout.rs index 76f830c1..057f7674 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -20,17 +20,21 @@ use std::cell::RefCell; use std::collections::{ HashMap, HashSet }; use std::ffi::CString; +use std::fmt; use std::rc::Rc; use std::vec::Vec; use ::action::Action; use ::drawing; use ::keyboard::{ KeyState, PressType }; +use ::logging; use ::manager; use ::submission::{ Timestamp, VirtualKeyboard }; use ::util::find_max_double; +// Traits use std::borrow::Borrow; +use ::logging::Warn; /// Gathers stuff defined in C or called by C pub mod c { @@ -623,6 +627,12 @@ pub struct LayoutData { #[derive(Debug)] struct NoSuchView; +impl fmt::Display for NoSuchView { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "No such view") + } +} + // Unfortunately, changes are not atomic due to mutability :( // An error will not be recoverable // The usage of &mut on Rc> doesn't mean anything special. @@ -660,7 +670,10 @@ impl Layout { time: Timestamp, ) { if !self.pressed_keys.insert(::util::Pointer(rckey.clone())) { - eprintln!("Warning: key {:?} was already pressed", rckey); + log_print!( + logging::Level::Bug, + "Key {:?} was already pressed", rckey, + ); } let mut key = rckey.borrow_mut(); virtual_keyboard.switch( @@ -806,9 +819,10 @@ mod seat { fn try_set_view(layout: &mut Layout, view_name: String) { layout.set_view(view_name.clone()) - .map_err(|e| - eprintln!("Bad view {} ({:?}), ignoring", view_name, e) - ).ok(); + .or_print( + logging::Problem::Bug, + &format!("Bad view {}, ignoring", view_name), + ); } /// A vessel holding an obligation to switch view. @@ -850,9 +864,10 @@ mod seat { Action::LockView { lock: _, unlock: view } => { new_view = Some(view.clone()); }, - a => eprintln!( - "BUG: action {:?} was found inside locked keys", - a + a => log_print!( + logging::Level::Bug, + "Non-locking action {:?} was found inside locked keys", + a, ), }; key.locked = false; @@ -937,7 +952,10 @@ mod seat { } } }, - Action::SetModifier(_) => eprintln!("Modifiers unsupported"), + Action::SetModifier(_) => log_print!( + logging::Level::Bug, + "Modifiers unsupported", + ), }; let pointer = ::util::Pointer(rckey.clone()); diff --git a/src/lib.rs b/src/lib.rs index 7f422c6a..579ec478 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,6 +15,9 @@ extern crate regex; extern crate serde; extern crate xkbcommon; +#[macro_use] +mod logging; + mod action; pub mod data; mod drawing; @@ -24,7 +27,6 @@ mod keyboard; mod layout; mod locale; mod locale_config; -mod logging; mod manager; mod outputs; mod popover; diff --git a/src/locale.rs b/src/locale.rs index 6b87f68c..03e6bd9c 100644 --- a/src/locale.rs +++ b/src/locale.rs @@ -8,6 +8,7 @@ use std::cmp; use std::ffi::{ CStr, CString }; +use std::fmt; use std::os::raw::c_char; use std::ptr; use std::str::Utf8Error; @@ -47,6 +48,12 @@ pub enum Error { NoInfo, } +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&self, f) + } +} + pub struct XkbInfo(c::GnomeXkbInfo); impl XkbInfo { diff --git a/src/logging.rs b/src/logging.rs index ba0ed2b8..7864d2a7 100644 --- a/src/logging.rs +++ b/src/logging.rs @@ -108,8 +108,20 @@ pub enum Problem { Surprise, } +/// Sugar for approach 2 +// TODO: avoid, deprecate. +// Handler instances should be long lived, not one per call. +macro_rules! log_print { + ($level:expr, $($arg:tt)*) => (::logging::print($level, &format!($($arg)*))) +} + +/// Approach 2 +pub fn print(level: Level, message: &str) { + Print{}.handle(level, message) +} + /// Sugar for logging errors in results. -pub trait Warn where Self: Sized{ +pub trait Warn where Self: Sized { type Value; /// Approach 2. fn or_print(self, level: Problem, message: &str) -> Option { diff --git a/src/outputs.rs b/src/outputs.rs index 9692e954..08ac590d 100644 --- a/src/outputs.rs +++ b/src/outputs.rs @@ -1,7 +1,10 @@ /*! Managing Wayland outputs */ use std::vec::Vec; +use ::logging; +// traits +use ::logging::Warn; /// Gathers stuff defined in C or called by C pub mod c { @@ -113,14 +116,11 @@ pub mod c { _make: *const c_char, _model: *const c_char, transform: i32, ) { - let transform = Transform::from_u32(transform as u32).unwrap_or_else( - || { - eprintln!( - "Warning: received invalid wl_output.transform value" - ); - Transform::Normal - } - ); + let transform = Transform::from_u32(transform as u32) + .or_print( + logging::Problem::Warning, + "Received invalid wl_output.transform value", + ).unwrap_or(Transform::Normal); let outputs = outputs.clone_ref(); let mut collection = outputs.borrow_mut(); @@ -129,7 +129,10 @@ pub mod c { .map(|o| &mut o.pending); match output_state { Some(state) => { state.transform = Some(transform) }, - None => eprintln!("Wayland error: Got mode on unknown output"), + None => log_print!( + logging::Level::Warning, + "Got geometry on unknown output", + ), }; } @@ -141,10 +144,12 @@ pub mod c { height: i32, _refresh: i32, ) { - let flags = Mode::from_bits(flags).unwrap_or_else(|| { - eprintln!("Warning: received invalid wl_output.mode flags"); - Mode::NONE - }); + let flags = Mode::from_bits(flags) + .or_print( + logging::Problem::Warning, + "Received invalid wl_output.mode flags", + ).unwrap_or(Mode::NONE); + let outputs = outputs.clone_ref(); let mut collection = outputs.borrow_mut(); let output_state: Option<&mut OutputState> @@ -156,7 +161,10 @@ pub mod c { state.current_mode = Some(super::Mode { width, height}); } }, - None => eprintln!("Wayland error: Got mode on unknown output"), + None => log_print!( + logging::Level::Warning, + "Got mode on unknown output", + ), }; } @@ -169,7 +177,10 @@ pub mod c { let output = find_output_mut(&mut collection, wl_output); match output { Some(output) => { output.current = output.pending.clone(); } - None => eprintln!("Wayland error: Got done on unknown output"), + None => log_print!( + logging::Level::Warning, + "Got done on unknown output", + ), }; } @@ -185,7 +196,10 @@ pub mod c { .map(|o| &mut o.pending); match output_state { Some(state) => { state.scale = factor; } - None => eprintln!("Wayland error: Got done on unknown output"), + None => log_print!( + logging::Level::Warning, + "Got scale on unknown output", + ), }; } @@ -258,7 +272,10 @@ pub mod c { } }, _ => { - eprintln!("Not enough info registered on output"); + log_print!( + logging::Level::Surprise, + "Not enough info received on output", + ); 0 }, } diff --git a/src/popover.rs b/src/popover.rs index 6132b4ab..5f0f735f 100644 --- a/src/popover.rs +++ b/src/popover.rs @@ -222,14 +222,10 @@ fn translate_layout_names(layouts: &Vec) -> Vec { LayoutId::System { name, kind: _ } => { xkb_translator.get_display_name(name) .map(|s| Status::Translated(OwnedTranslation(s))) - .unwrap_or_else(|e| { - eprintln!( - "No display name for xkb layout {}: {:?}", - name, - e, - ); - Status::Remaining(name.clone()) - }) + .or_print( + logging::Problem::Surprise, + &format!("No display name for xkb layout {}", name), + ).unwrap_or_else(|| Status::Remaining(name.clone())) }, LayoutId::Local(name) => Status::Remaining(name.clone()), }); @@ -365,10 +361,10 @@ pub fn show( match state { Some(v) => { v.get::() - .or_else(|| { - eprintln!("Variant is not string: {:?}", v); - None - }) + .or_print( + logging::Problem::Bug, + &format!("Variant is not string: {:?}", v) + ) .map(|state| { let (_id, layout) = choices.iter() .find( @@ -380,7 +376,10 @@ pub fn show( ) }); }, - None => eprintln!("No variant selected"), + None => log_print!( + logging::Level::Debug, + "No variant selected", + ), }; menu_inner.popdown(); }); diff --git a/src/style.rs b/src/style.rs index 2e033a7e..153431ae 100644 --- a/src/style.rs +++ b/src/style.rs @@ -85,7 +85,10 @@ fn get_theme_name(settings: >k::Settings) -> GtkTheme { match &e { env::VarError::NotPresent => {}, // maybe TODO: forward this warning? - e => eprintln!("GTK_THEME variable invalid: {}", e), + e => log_print!( + logging::Level::Surprise, + "GTK_THEME variable invalid: {}", e, + ), }; e }).ok(); From 2b65beba4492242b1fc6b189f92f4e3fc073470c Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Mon, 20 Jan 2020 15:40:30 +0000 Subject: [PATCH 4/4] press_key: Use proper logging --- src/layout.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/layout.rs b/src/layout.rs index 6ef223ef..7682d8ec 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -871,7 +871,10 @@ mod seat { rckey: &Rc>, ) { if !layout.pressed_keys.insert(::util::Pointer(rckey.clone())) { - eprintln!("Warning: key {:?} was already pressed", rckey); + log_print!( + logging::Level::Bug, + "Key {:?} was already pressed", rckey, + ); } let mut key = rckey.borrow_mut(); virtual_keyboard.switch(