From b6d25da7c24d1471578dc28cd6589a9a81e50968 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Sat, 28 Sep 2019 21:14:14 +0000 Subject: [PATCH 1/3] layout: Fallback to builtin before switching layouts When the user-provided layout was broken or missing, the loading would proceed with the fallback layout. It tries to load the builtin one first now. --- src/data.rs | 72 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 29 deletions(-) diff --git a/src/data.rs b/src/data.rs index 0fb1f280..44231643 100644 --- a/src/data.rs +++ b/src/data.rs @@ -94,36 +94,60 @@ impl fmt::Display for DataSource { /// If the layout exists, but is broken, fallback is activated. fn load_layout( name: &str -) -> (Result<::layout::Layout, LoadError>, DataSource) { +) -> ( + Result<::layout::Layout, LoadError>, // last attempted + DataSource, // last attempt source + Option<(LoadError, DataSource)>, // first attempt source +) { let path = env::var_os("SQUEEKBOARD_KEYBOARDSDIR") .map(PathBuf::from) .or_else(|| xdg::data_path("squeekboard/keyboards")) .map(|path| path.join(name).with_extension("yaml")); - let (layout, source) = match path { - Some(path) => {( + let layout = match path { + Some(path) => Some(( Layout::from_yaml_stream(path.clone()) - .map_err(|e| LoadError::BadData(e)), - DataSource::File(path) - )}, - None => {( - load_layout_from_resource(name), - DataSource::Resource(name.into()) - )}, + .map_err(LoadError::BadData), + DataSource::File(path), + )), + None => None, // No env var, not an error + }; + + let (failed_attempt, layout) = match layout { + Some((Ok(layout), path)) => (None, Some((layout, path))), + Some((Err(e), path)) => (Some((e, path)), None), + None => (None, None), }; + let (layout, source) = match layout { + Some((layout, path)) => (Ok(layout), path), + None => ( + load_layout_from_resource(name), + DataSource::Resource(name.into()), + ), + }; + + // FIXME: attempt at each step of fallback let layout = layout.and_then( |layout| layout.build().map_err(LoadError::BadKeyMap) ); - (layout, source) + (layout, source, failed_attempt) } fn load_layout_with_fallback( name: &str ) -> ::layout::Layout { - let (layout, source) = load_layout(name); - let (layout, source) = match (layout, source) { + let (layout, source, attempt) = load_layout(name); + + if let Some((e, source)) = attempt { + eprintln!( + "Failed to load layout from {}: {}, trying builtin", + source, e + ); + }; + + let (layout, source, attempt) = match (layout, source) { (Err(e), source) => { eprintln!( "Failed to load layout from {}: {}, using fallback", @@ -131,24 +155,14 @@ fn load_layout_with_fallback( ); load_layout(FALLBACK_LAYOUT_NAME) }, - res => res, + (res, source) => (res, source, None), }; - let (layout, source) = match (layout, source) { - (Err(e), source) => { - eprintln!( - "Failed to load fallback layout from {}: {}, using hardcoded", - source, e - ); - ( - load_layout_from_resource(FALLBACK_LAYOUT_NAME) - .and_then( - |layout| layout.build().map_err(LoadError::BadKeyMap) - ), - DataSource::Resource(FALLBACK_LAYOUT_NAME.into()), - ) - }, - res => res, + if let Some((e, source)) = attempt { + eprintln!( + "Failed to load layout from {}: {}, trying builtin", + source, e + ); }; match (layout, source) { From e33f591a1fb9d17ce329885694ee7c31813c94ea Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Sat, 28 Sep 2019 21:37:51 +0000 Subject: [PATCH 2/3] layouts: Test fallback order --- src/data.rs | 50 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/src/data.rs b/src/data.rs index 44231643..0643d6e0 100644 --- a/src/data.rs +++ b/src/data.rs @@ -40,7 +40,7 @@ pub mod c { .expect("Bad layout name") .expect("Empty layout name"); - let layout = load_layout_with_fallback(name); + let layout = build_layout_with_fallback(name); Box::into_raw(Box::new(layout)) } } @@ -76,6 +76,7 @@ pub fn load_layout_from_resource( .map_err(|e| LoadError::BadResource(e)) } +#[derive(Debug, PartialEq)] enum DataSource { File(PathBuf), Resource(String), @@ -93,16 +94,14 @@ impl fmt::Display for DataSource { /// Tries to load the layout from the first place where it's present. /// If the layout exists, but is broken, fallback is activated. fn load_layout( - name: &str + name: &str, + keyboards_path: Option, ) -> ( - Result<::layout::Layout, LoadError>, // last attempted + Result, // last attempted DataSource, // last attempt source Option<(LoadError, DataSource)>, // first attempt source ) { - let path = env::var_os("SQUEEKBOARD_KEYBOARDSDIR") - .map(PathBuf::from) - .or_else(|| xdg::data_path("squeekboard/keyboards")) - .map(|path| path.join(name).with_extension("yaml")); + let path = keyboards_path.map(|path| path.join(name).with_extension("yaml")); let layout = match path { Some(path) => Some(( @@ -126,6 +125,19 @@ fn load_layout( DataSource::Resource(name.into()), ), }; + + (layout, source, failed_attempt) +} + +fn build_layout( + name: &str, + keyboards_path: Option, +) -> ( + Result<::layout::Layout, LoadError>, // last attempted + DataSource, // last attempt source + Option<(LoadError, DataSource)>, // first attempt source +) { + let (layout, source, failed_attempt) = load_layout(name, keyboards_path); // FIXME: attempt at each step of fallback let layout = layout.and_then( @@ -135,10 +147,14 @@ fn load_layout( (layout, source, failed_attempt) } -fn load_layout_with_fallback( +fn build_layout_with_fallback( name: &str ) -> ::layout::Layout { - let (layout, source, attempt) = load_layout(name); + let path = env::var_os("SQUEEKBOARD_KEYBOARDSDIR") + .map(PathBuf::from) + .or_else(|| xdg::data_path("squeekboard/keyboards")); + + let (layout, source, attempt) = build_layout(name, path.clone()); if let Some((e, source)) = attempt { eprintln!( @@ -153,7 +169,7 @@ fn load_layout_with_fallback( "Failed to load layout from {}: {}, using fallback", source, e ); - load_layout(FALLBACK_LAYOUT_NAME) + build_layout(FALLBACK_LAYOUT_NAME, path) }, (res, source) => (res, source, None), }; @@ -621,6 +637,20 @@ mod tests { ); } + /// First fallback should be to builtin, not to FALLBACK_LAYOUT_NAME + #[test] + fn fallbacks_order() { + let (layout, source, _failure) = load_layout( + "nb", + Some(PathBuf::from("tests")) + ); + + assert_eq!( + source, + load_layout("nb", None).1 + ); + } + #[test] fn unicode_keysym() { let keysym = xkb::keysym_from_name( From 1b424bd663feb3bccfa2f813632ffcfe251e9630 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Sun, 29 Sep 2019 07:54:32 +0000 Subject: [PATCH 3/3] layout: Attempt to build xdg keymap at every load --- src/data.rs | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/src/data.rs b/src/data.rs index 0643d6e0..5014715f 100644 --- a/src/data.rs +++ b/src/data.rs @@ -40,7 +40,7 @@ pub mod c { .expect("Bad layout name") .expect("Empty layout name"); - let layout = build_layout_with_fallback(name); + let layout = load_layout_with_fallback(name); Box::into_raw(Box::new(layout)) } } @@ -97,7 +97,7 @@ fn load_layout( name: &str, keyboards_path: Option, ) -> ( - Result, // last attempted + Result<::layout::Layout, LoadError>, // last attempted DataSource, // last attempt source Option<(LoadError, DataSource)>, // first attempt source ) { @@ -106,7 +106,10 @@ fn load_layout( let layout = match path { Some(path) => Some(( Layout::from_yaml_stream(path.clone()) - .map_err(LoadError::BadData), + .map_err(LoadError::BadData) + .and_then(|layout| + layout.build().map_err(LoadError::BadKeyMap) + ), DataSource::File(path), )), None => None, // No env var, not an error @@ -121,7 +124,10 @@ fn load_layout( let (layout, source) = match layout { Some((layout, path)) => (Ok(layout), path), None => ( - load_layout_from_resource(name), + load_layout_from_resource(name) + .and_then(|layout| + layout.build().map_err(LoadError::BadKeyMap) + ), DataSource::Resource(name.into()), ), }; @@ -129,32 +135,14 @@ fn load_layout( (layout, source, failed_attempt) } -fn build_layout( - name: &str, - keyboards_path: Option, -) -> ( - Result<::layout::Layout, LoadError>, // last attempted - DataSource, // last attempt source - Option<(LoadError, DataSource)>, // first attempt source -) { - let (layout, source, failed_attempt) = load_layout(name, keyboards_path); - - // FIXME: attempt at each step of fallback - let layout = layout.and_then( - |layout| layout.build().map_err(LoadError::BadKeyMap) - ); - - (layout, source, failed_attempt) -} - -fn build_layout_with_fallback( +fn load_layout_with_fallback( name: &str ) -> ::layout::Layout { let path = env::var_os("SQUEEKBOARD_KEYBOARDSDIR") .map(PathBuf::from) .or_else(|| xdg::data_path("squeekboard/keyboards")); - let (layout, source, attempt) = build_layout(name, path.clone()); + let (layout, source, attempt) = load_layout(name, path.clone()); if let Some((e, source)) = attempt { eprintln!( @@ -169,7 +157,7 @@ fn build_layout_with_fallback( "Failed to load layout from {}: {}, using fallback", source, e ); - build_layout(FALLBACK_LAYOUT_NAME, path) + load_layout(FALLBACK_LAYOUT_NAME, path) }, (res, source) => (res, source, None), };