From dd44bdc57ea2b90744fe47daca4bdce317781990 Mon Sep 17 00:00:00 2001 From: Riccardo Attilio Galli Date: Sun, 5 Nov 2023 09:58:43 -0800 Subject: [PATCH 1/6] Alert the user that the custom theme was not used and why --- src/color.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/color.rs b/src/color.rs index 8cb93c1ba..ed75fac59 100644 --- a/src/color.rs +++ b/src/color.rs @@ -176,7 +176,10 @@ impl Colors { ThemeOption::Default | ThemeOption::NoLscolors => Some(Theme::default().color), ThemeOption::Custom => Some( Theme::from_path::(Path::new("colors").to_str().unwrap()) - .unwrap_or_default(), + .unwrap_or_else(|e| { + print_output!("Warning: cannot load custom theme. {}.\n\n", e); + ColorTheme::default() + }), ), ThemeOption::CustomLegacy(ref file) => { print_output!( From 922c796f4a5d7d40c42baa1dc6f96339d9a713f5 Mon Sep 17 00:00:00 2001 From: Riccardo Attilio Galli Date: Sun, 5 Nov 2023 10:00:30 -0800 Subject: [PATCH 2/6] Increase details of invalid theme format error --- src/theme.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/theme.rs b/src/theme.rs index f18b33106..329e1e30a 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -27,11 +27,11 @@ pub struct Theme { #[derive(Error, Debug)] pub enum Error { - #[error("Theme file not existed")] + #[error("Cannot read theme file. {0}")] NotExisted(#[from] io::Error), - #[error("Theme file format invalid")] + #[error("Theme file format invalid. {0}")] InvalidFormat(#[from] serde_yaml::Error), - #[error("Theme file path invalid {0}")] + #[error("Theme file path invalid. {0}")] InvalidPath(String), #[error("Unknown Theme error")] Unknown(), From 6c9f6e80f97ce60da5f2eafda1fae0b17ac95292 Mon Sep 17 00:00:00 2001 From: Riccardo Attilio Galli Date: Sun, 5 Nov 2023 10:00:52 -0800 Subject: [PATCH 3/6] Exit early if a theme file format is invalid --- src/theme.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/theme.rs b/src/theme.rs index 329e1e30a..88043ed47 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -68,6 +68,7 @@ impl Theme { Ok(t) => return Ok(t), Err(e) => { err = Error::from(e); + break; } }, Err(e) => err = Error::from(e), From b76c3c280e6c770e27cc8fd73489132b5270fc31 Mon Sep 17 00:00:00 2001 From: Riccardo Attilio Galli Date: Sun, 5 Nov 2023 12:03:58 -0800 Subject: [PATCH 4/6] Add tests to theme.rs --- src/theme.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/theme.rs b/src/theme.rs index 88043ed47..1d3a8e9c0 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -89,3 +89,71 @@ impl Theme { serde_yaml::from_str::(yaml) } } + +#[cfg(test)] +mod tests { + + use super::Error; + use super::Theme; + + #[test] + fn test_can_deserialize_yaml() { + use std::collections::BTreeMap; + let mut map: BTreeMap = BTreeMap::new(); + map.insert("user".to_string(), "1".to_string()); + map.insert("group".to_string(), "2".to_string()); + assert_eq!( + map, + Theme::with_yaml( + r#"--- + user: 1 + group: 2 + "# + ) + .unwrap() + ); + } + + #[test] + fn test_ioerror() { + use super::ColorTheme; + + let dir = assert_fs::TempDir::new().unwrap(); + let theme = dir.path().join("does-not-exist.yaml"); + + let res = Theme::from_path::(theme.to_str().unwrap()); + assert!(res.is_err()); + let the_error = res.unwrap_err(); + assert!(matches!(&the_error, Error::NotExisted(_))); + if let Error::NotExisted(some_err) = &the_error { + assert_eq!(some_err.kind(), std::io::ErrorKind::NotFound); + } + + // There are many reasons why we could get an IoError, not just "file not found". + // Here we test that we actually get informations about the underlying io error. + assert_eq!( + "Cannot read theme file. No such file or directory (os error 2)".to_string(), + the_error.to_string() + ); + } + + #[test] + fn test_invalid_format() { + use super::ColorTheme; + use std::fs::File; + use std::io::Write; + + let dir = assert_fs::TempDir::new().unwrap(); + let theme = dir.path().join("does-not-exist.yaml"); + let mut file = File::create(&theme).unwrap(); + // Write a purposefully bogus file + writeln!(file, "{}", "bogus-field: 1").unwrap(); + + let res = Theme::from_path::(theme.to_str().unwrap()); + assert!(res.is_err()); + // Just check the first part of serde_yaml output so that we don't break the test just adding new fields. + assert!(res.unwrap_err().to_string().starts_with( + "Theme file format invalid. unknown field `bogus-field`, expected one of" + )); + } +} From 17058d9681739ebe87a2353cb6bb1c51672f139d Mon Sep 17 00:00:00 2001 From: Riccardo Attilio Galli Date: Wed, 22 Nov 2023 20:09:46 -0800 Subject: [PATCH 5/6] Update unrelated code to satisfy newer clippy --- src/theme.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/theme.rs b/src/theme.rs index 1d3a8e9c0..dffcc5c51 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -147,7 +147,7 @@ mod tests { let theme = dir.path().join("does-not-exist.yaml"); let mut file = File::create(&theme).unwrap(); // Write a purposefully bogus file - writeln!(file, "{}", "bogus-field: 1").unwrap(); + writeln!(file, "bogus-field: 1").unwrap(); let res = Theme::from_path::(theme.to_str().unwrap()); assert!(res.is_err()); From cb9930b2e74fb73a22d455ce168f108c765f4958 Mon Sep 17 00:00:00 2001 From: Riccardo Attilio Galli Date: Wed, 22 Nov 2023 20:16:58 -0800 Subject: [PATCH 6/6] Check file not found output with other OSes in mind --- src/theme.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/theme.rs b/src/theme.rs index dffcc5c51..a08443ad6 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -131,10 +131,8 @@ mod tests { // There are many reasons why we could get an IoError, not just "file not found". // Here we test that we actually get informations about the underlying io error. - assert_eq!( - "Cannot read theme file. No such file or directory (os error 2)".to_string(), - the_error.to_string() - ); + assert!(the_error.to_string().starts_with("Cannot read theme file")); + assert!(the_error.to_string().ends_with("(os error 2)")); } #[test]