s3rius/squeekboard
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

logging: Try to improve common operations

Browse Source 
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.
This commit is contained in:
Dorota Czaplejewicz
2020-01-16 15:57:46 +00:00
parent f3d852f552
commit ea84f4f031
5 changed files with 168 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
src/data.rs
View File

@ -21,7 +21,7 @@ use ::keyboard::{
}; };
use ::layout; use ::layout;
use ::layout::ArrangementKind; use ::layout::ArrangementKind;
use ::logging::PrintWarnings; use ::logging;
use ::resources; use ::resources;
use ::util::c::as_str; use ::util::c::as_str;
use ::util::hash_map_map; use ::util::hash_map_map;
@ -31,7 +31,7 @@ use ::xdg;
use serde::Deserialize; use serde::Deserialize;
use std::io::BufReader; use std::io::BufReader;
use std::iter::FromIterator; use std::iter::FromIterator;
use ::logging::WarningHandler; use ::logging::Warn;
/// Gathers stuff defined in C or called by C /// Gathers stuff defined in C or called by C
pub mod c { pub mod c {
@ -157,7 +157,7 @@ fn list_layout_sources(
fn load_layout_data(source: DataSource) fn load_layout_data(source: DataSource)
-> Result<::layout::LayoutData, LoadError> -> Result<::layout::LayoutData, LoadError>
{ {
let handler = PrintWarnings{}; let handler = logging::Print {};
match source { match source {
DataSource::File(path) => { DataSource::File(path) => {
Layout::from_file(path.clone()) Layout::from_file(path.clone())
@ -330,7 +330,7 @@ impl Layout {
serde_yaml::from_reader(infile).map_err(Error::Yaml) serde_yaml::from_reader(infile).map_err(Error::Yaml)
} }
pub fn build<H: WarningHandler>(self, mut warning_handler: H) pub fn build<H: logging::Handler>(self, mut warning_handler: H)
-> (Result<::layout::LayoutData, FormattingError>, H) -> (Result<::layout::LayoutData, FormattingError>, H)
{ {
let button_names = self.views.values() let button_names = self.views.values()
@ -464,7 +464,7 @@ impl Layout {
} }
} }
fn create_action<H: WarningHandler>( fn create_action<H: logging::Handler>(
button_info: &HashMap<String, ButtonMeta>, button_info: &HashMap<String, ButtonMeta>,
name: &str, name: &str,
view_names: Vec<&String>, view_names: Vec<&String>,
@ -494,15 +494,18 @@ fn create_action<H: WarningHandler>(
(None, None, Some(text)) => SubmitData::Text(text.clone()), (None, None, Some(text)) => SubmitData::Text(text.clone()),
(None, None, None) => SubmitData::Text(name.into()), (None, None, None) => SubmitData::Text(name.into()),
_ => { _ => {
warning_handler.handle(&format!( warning_handler.handle(
"Button {} has more than one of (action, keysym, text)", logging::Level::Warning,
name &format!(
)); "Button {} has more than one of (action, keysym, text)",
name,
),
);
SubmitData::Text("".into()) SubmitData::Text("".into())
}, },
}; };
fn filter_view_name<H: WarningHandler>( fn filter_view_name<H: logging::Handler>(
button_name: &str, button_name: &str,
view_name: String, view_name: String,
view_names: &Vec<&String>, view_names: &Vec<&String>,
@ -511,10 +514,13 @@ fn create_action<H: WarningHandler>(
if view_names.contains(&&view_name) { if view_names.contains(&&view_name) {
view_name view_name
} else { } else {
warning_handler.handle(&format!("Button {} switches to missing view {}", warning_handler.handle(
button_name, logging::Level::Warning,
view_name, &format!("Button {} switches to missing view {}",
)); button_name,
view_name,
),
);
"base".into() "base".into()
} }
} }
@ -553,27 +559,24 @@ fn create_action<H: WarningHandler>(
match keysym_valid(keysym.as_str()) { match keysym_valid(keysym.as_str()) {
true => keysym.clone(), true => keysym.clone(),
false => { false => {
warning_handler.handle(&format!( warning_handler.handle(
"Keysym name invalid: {}", logging::Level::Warning,
keysym, &format!(
)); "Keysym name invalid: {}",
keysym,
),
);
"space".into() // placeholder "space".into() // placeholder
}, },
} }
)), )),
}, },
SubmitData::Text(text) => ::action::Action::Submit { SubmitData::Text(text) => ::action::Action::Submit {
text: { text: CString::new(text.clone()).or_warn(
CString::new(text.clone()) warning_handler,
.map_err(|e| { logging::Problem::Warning,
warning_handler.handle(&format!( &format!("Text {} contains problems", text),
"Text {} contains problems: {:?}", ),
text,
e
));
e
}).ok()
},
keys: text.chars().map(|codepoint| { keys: text.chars().map(|codepoint| {
let codepoint_string = codepoint.to_string(); let codepoint_string = codepoint.to_string();
::action::KeySym(match keysym_valid(codepoint_string.as_str()) { ::action::KeySym(match keysym_valid(codepoint_string.as_str()) {
@ -587,7 +590,7 @@ fn create_action<H: WarningHandler>(
/// TODO: Since this will receive user-provided data, /// TODO: Since this will receive user-provided data,
/// all .expect() on them should be turned into soft fails /// all .expect() on them should be turned into soft fails
fn create_button<H: WarningHandler>( fn create_button<H: logging::Handler>(
button_info: &HashMap<String, ButtonMeta>, button_info: &HashMap<String, ButtonMeta>,
outlines: &HashMap<String, Outline>, outlines: &HashMap<String, Outline>,
name: &str, name: &str,
@ -611,14 +614,11 @@ fn create_button<H: WarningHandler>(
} else if let Some(text) = &button_meta.text { } else if let Some(text) = &button_meta.text {
::layout::Label::Text( ::layout::Label::Text(
CString::new(text.as_str()) CString::new(text.as_str())
.unwrap_or_else(|e| { .or_warn(
warning_handler.handle(&format!( warning_handler,
"Text {} is invalid: {}", logging::Problem::Warning,
text, &format!("Text {} is invalid", text),
e, ).unwrap_or_else(|| CString::new("").unwrap())
));
CString::new("").unwrap()
})
) )
} else { } else {
::layout::Label::Text(cname.clone()) ::layout::Label::Text(cname.clone())
@ -629,7 +629,10 @@ fn create_button<H: WarningHandler>(
if outlines.contains_key(outline) { if outlines.contains_key(outline) {
outline.clone() outline.clone()
} else { } 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() "default".into()
} }
} }
@ -638,12 +641,11 @@ fn create_button<H: WarningHandler>(
let outline = outlines.get(&outline_name) let outline = outlines.get(&outline_name)
.map(|outline| (*outline).clone()) .map(|outline| (*outline).clone())
.unwrap_or_else(|| { .or_warn(
warning_handler.handle( warning_handler,
&format!("No default outline defined! Using 1x1!") logging::Problem::Warning,
); "No default outline defined! Using 1x1!",
Outline { width: 1f64, height: 1f64 } ).unwrap_or(Outline { width: 1f64, height: 1f64 });
});
layout::Button { layout::Button {
name: cname, name: cname,
@ -663,7 +665,7 @@ mod tests {
use super::*; use super::*;
use std::error::Error as ErrorTrait; use std::error::Error as ErrorTrait;
use ::logging::PanicWarn; use ::logging::ProblemPanic;
#[test] #[test]
fn test_parse_path() { fn test_parse_path() {
@ -733,7 +735,7 @@ mod tests {
fn test_layout_punctuation() { fn test_layout_punctuation() {
let out = Layout::from_file(PathBuf::from("tests/layout_key1.yaml")) let out = Layout::from_file(PathBuf::from("tests/layout_key1.yaml"))
.unwrap() .unwrap()
.build(PanicWarn).0 .build(ProblemPanic).0
.unwrap(); .unwrap();
assert_eq!( assert_eq!(
out.views["base"] out.views["base"]
@ -748,7 +750,7 @@ mod tests {
fn test_layout_unicode() { fn test_layout_unicode() {
let out = Layout::from_file(PathBuf::from("tests/layout_key2.yaml")) let out = Layout::from_file(PathBuf::from("tests/layout_key2.yaml"))
.unwrap() .unwrap()
.build(PanicWarn).0 .build(ProblemPanic).0
.unwrap(); .unwrap();
assert_eq!( assert_eq!(
out.views["base"] out.views["base"]
@ -764,7 +766,7 @@ mod tests {
fn test_layout_unicode_multi() { fn test_layout_unicode_multi() {
let out = Layout::from_file(PathBuf::from("tests/layout_key3.yaml")) let out = Layout::from_file(PathBuf::from("tests/layout_key3.yaml"))
.unwrap() .unwrap()
.build(PanicWarn).0 .build(ProblemPanic).0
.unwrap(); .unwrap();
assert_eq!( assert_eq!(
out.views["base"] out.views["base"]
@ -779,7 +781,7 @@ mod tests {
#[test] #[test]
fn parsing_fallback() { fn parsing_fallback() {
assert!(Layout::from_resource(FALLBACK_LAYOUT_NAME) assert!(Layout::from_resource(FALLBACK_LAYOUT_NAME)
.map(|layout| layout.build(PanicWarn).0.unwrap()) .map(|layout| layout.build(ProblemPanic).0.unwrap())
.is_ok() .is_ok()
); );
} }
@ -827,7 +829,7 @@ mod tests {
}, },
".", ".",
Vec::new(), Vec::new(),
&mut PanicWarn, &mut ProblemPanic,
), ),
::action::Action::Submit { ::action::Action::Submit {
text: Some(CString::new(".").unwrap()), text: Some(CString::new(".").unwrap()),
@ -840,7 +842,7 @@ mod tests {
fn test_layout_margins() { fn test_layout_margins() {
let out = Layout::from_file(PathBuf::from("tests/layout_margins.yaml")) let out = Layout::from_file(PathBuf::from("tests/layout_margins.yaml"))
.unwrap() .unwrap()
.build(PanicWarn).0 .build(ProblemPanic).0
.unwrap(); .unwrap();
assert_eq!( assert_eq!(
out.margins, out.margins,

117
src/logging.rs
View File

@ -26,13 +26,15 @@
* 4. logging to an immutable destination type * 4. logging to an immutable destination type
* *
* Same as above, except it can be parallelized. * 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, * It seems more difficult to pass the logger around,
* but this may be a solved problem from the area of functional programming. * but this may be a solved problem from the area of functional programming.
* *
* This library generally aims at the approach in 3. * This library generally aims at the approach in 3.
* */ * */
use std::error::Error; use std::fmt::Display;
/// Levels are not in order. /// Levels are not in order.
pub enum Level { pub enum Level {
@ -66,18 +68,72 @@ pub enum Level {
Debug, Debug,
} }
/// Sugar for logging errors in results. impl Level {
/// Approach 2. fn as_str(&self) -> &'static str {
pub trait Warn { match self {
type Value; Level::Panic => "Panic",
fn or_warn(self, msg: &str) -> Option<Self::Value>; Level::Bug => "Bug",
Level::Error => "Error",
Level::Warning => "Warning",
Level::Surprise => "Surprise",
Level::Info => "Info",
Level::Debug => "Debug",
}
}
} }
impl<T, E: Error> Warn for Result<T, E> { impl From<Problem> 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::Value> {
self.or_warn(&mut Print {}, level.into(), message)
}
/// Approach 3.
fn or_warn<H: Handler>(
self,
handler: &mut H,
level: Problem,
message: &str,
) -> Option<Self::Value>;
}
impl<T, E: Display> Warn for Result<T, E> {
type Value = T; type Value = T;
fn or_warn(self, msg: &str) -> Option<T> { fn or_warn<H: Handler>(
self,
handler: &mut H,
level: Problem,
message: &str,
) -> Option<T> {
self.map_err(|e| { self.map_err(|e| {
eprintln!("{}: {}", msg, e); handler.handle(level.into(), &format!("{}: {}", message, e));
e e
}).ok() }).ok()
} }
@ -85,9 +141,14 @@ impl<T, E: Error> Warn for Result<T, E> {
impl<T> Warn for Option<T> { impl<T> Warn for Option<T> {
type Value = T; type Value = T;
fn or_warn(self, msg: &str) -> Option<T> { fn or_warn<H: Handler>(
self,
handler: &mut H,
level: Problem,
message: &str,
) -> Option<T> {
self.or_else(|| { self.or_else(|| {
eprintln!("{}", msg); handler.handle(level.into(), message);
None None
}) })
} }
@ -95,26 +156,34 @@ impl<T> Warn for Option<T> {
/// A mutable handler for text warnings. /// A mutable handler for text warnings.
/// Approach 3. /// Approach 3.
pub trait WarningHandler { pub trait Handler {
/// Handle a warning /// Handle a log message
fn handle(&mut self, warning: &str); fn handle(&mut self, level: Level, message: &str);
} }
/// Prints warnings to stderr /// Prints info to stdout, everything else to stderr
pub struct PrintWarnings; pub struct Print;
impl WarningHandler for PrintWarnings { impl Handler for Print {
fn handle(&mut self, warning: &str) { fn handle(&mut self, level: Level, message: &str) {
eprintln!("{}", warning); 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 /// Don't use except in tests
pub struct PanicWarn; pub struct ProblemPanic;
impl WarningHandler for PanicWarn { impl Handler for ProblemPanic {
fn handle(&mut self, warning: &str) { fn handle(&mut self, level: Level, message: &str) {
panic!("{}", warning); use self::Level::*;
match level {
Panic | Bug | Error | Warning | Surprise => panic!("{}", message),
l => Print{}.handle(l, message),
}
} }
} }

8
src/popover.rs
View File

@ -7,6 +7,7 @@ use ::layout::c::{ Bounds, EekGtkKeyboard };
use ::locale; use ::locale;
use ::locale::{ OwnedTranslation, Translation, compare_current_locale }; use ::locale::{ OwnedTranslation, Translation, compare_current_locale };
use ::locale_config::system_locale; use ::locale_config::system_locale;
use ::logging;
use ::manager; use ::manager;
use ::resources; use ::resources;
@ -242,10 +243,13 @@ fn translate_layout_names(layouts: &Vec<LayoutId>) -> Vec<OwnedTranslation> {
.as_ref() .as_ref()
.to_owned() .to_owned()
) )
.or_warn("No locale detected") .or_print(logging::Problem::Surprise, "No locale detected")
.and_then(|lang| { .and_then(|lang| {
resources::get_layout_names(lang.as_str()) 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 { match builtin_translations {

7
src/style.rs
View File

@ -19,6 +19,7 @@
/*! CSS data loading. */ /*! CSS data loading. */
use std::env; use std::env;
use ::logging;
use glib::object::ObjectExt; use glib::object::ObjectExt;
use logging::Warn; use logging::Warn;
@ -94,15 +95,13 @@ fn get_theme_name(settings: &gtk::Settings) -> GtkTheme {
None => GtkTheme { None => GtkTheme {
name: { name: {
settings.get_property("gtk-theme-name") settings.get_property("gtk-theme-name")
// maybe TODO: is this worth a warning? .or_print(logging::Problem::Surprise, "No theme name")
.or_warn("No theme name")
.and_then(|value| value.get::<String>()) .and_then(|value| value.get::<String>())
.unwrap_or(DEFAULT_THEME_NAME.into()) .unwrap_or(DEFAULT_THEME_NAME.into())
}, },
variant: { variant: {
settings.get_property("gtk-application-prefer-dark-theme") settings.get_property("gtk-application-prefer-dark-theme")
// maybe TODO: is this worth a warning? .or_print(logging::Problem::Surprise, "No settings key")
.or_warn("No settings key")
.and_then(|value| value.get::<bool>()) .and_then(|value| value.get::<bool>())
.and_then(|dark_preferred| match dark_preferred { .and_then(|dark_preferred| match dark_preferred {
true => Some("dark".into()), true => Some("dark".into()),

19
src/tests.rs
View File

@ -1,17 +1,22 @@
/*! Testing functionality */ /*! Testing functionality */
use ::data::Layout; use ::data::Layout;
use ::logging;
use xkbcommon::xkb; use xkbcommon::xkb;
use ::logging::WarningHandler;
pub struct CountAndPrint(u32); pub struct CountAndPrint(u32);
impl WarningHandler for CountAndPrint { impl logging::Handler for CountAndPrint {
fn handle(&mut self, warning: &str) { fn handle(&mut self, level: logging::Level, warning: &str) {
self.0 = self.0 + 1; use logging::Level::*;
println!("{}", warning); 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); let (layout, handler) = layout.build(handler);
if handler.0 > 0 { if handler.0 > 0 {
println!("{} mistakes in layout", handler.0) println!("{} problems while parsing layout", handler.0)
} }
let layout = layout.expect("layout broken"); let layout = layout.expect("layout broken");