From c75e085dc8e1dc1755316b0568ab7451ad1c8488 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Fri, 17 Jan 2020 12:25:39 +0000 Subject: [PATCH] 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();