Improve code quality and add tests

src/check.rs

@@ -30,7 +30,7 @@ pub fn check_dependencies() -> bool {
     let status = Command::new("notify-send")
         .arg("-v")
         .output();
-    if let Err(_) = status {
+    if status.is_err() {
         println!("Error: 'notify-send' command is needed for espanso to work correctly, please install it.");
         result = false;
     }
@@ -39,7 +39,7 @@ pub fn check_dependencies() -> bool {
     let status = Command::new("xclip")
         .arg("-version")
         .output();
-    if let Err(_) = status {
+    if status.is_err() {
         println!("Error: 'xclip' command is needed for espanso to work correctly, please install it.");
         result = false;
     }

src/config/mod.rs

@@ -34,7 +34,6 @@ use walkdir::WalkDir;
 
 pub(crate) mod runtime;
 
-// TODO: add documentation link
 const DEFAULT_CONFIG_FILE_CONTENT : &str = include_str!("../res/config.yml");
 
 const DEFAULT_CONFIG_FILE_NAME : &str = "default.yml";
@@ -162,7 +161,7 @@ impl Configs {
             let mut contents = String::new();
             let res = file.read_to_string(&mut contents);
 
-            if let Err(_) = res {
+            if res.is_err() {
                 return Err(ConfigLoadError::UnableToReadFile)
             }
 
@@ -187,7 +186,7 @@ impl Configs {
         });
         let parent_matches : Vec<Match> = self.matches.iter().filter(|&m| {
             !trigger_set.contains(&m.trigger)
-        }).map(|m| m.clone()).collect();
+        }).cloned().collect();
 
         merged_matches.extend(parent_matches);
         self.matches = merged_matches;
@@ -200,7 +199,7 @@ impl Configs {
         });
         let default_matches : Vec<Match> = default.matches.iter().filter(|&m| {
             !trigger_set.contains(&m.trigger)
-        }).map(|m| m.clone()).collect();
+        }).cloned().collect();
 
         self.matches.extend(default_matches);
     }
@@ -326,7 +325,7 @@ impl ConfigSet {
         // Create the espanso dir if id doesn't exist
         let res = create_dir_all(espanso_dir.as_path());
 
-        if let Ok(_) = res {
+        if res.is_ok() {
             let default_file = espanso_dir.join(DEFAULT_CONFIG_FILE_NAME);
 
             // If config file does not exist, create one from template
@@ -358,13 +357,12 @@ impl ConfigSet {
             return ConfigSet::load(espanso_dir.as_path())
         }
 
-        return Err(ConfigLoadError::UnableToCreateDefaultConfig)
+        Err(ConfigLoadError::UnableToCreateDefaultConfig)
     }
 
     pub fn get_default_config_dir() -> PathBuf {
         let home_dir = dirs::home_dir().expect("Unable to get home directory");
-        let espanso_dir = home_dir.join(".espanso");
+        home_dir.join(".espanso")
-        espanso_dir
     }
 
     pub fn get_default_packages_dir() -> PathBuf {

src/context/macos.rs

@@ -27,7 +27,7 @@ use std::fs;
 use log::{info, error};
 use std::process::exit;
 
-const STATUS_ICON_BINARY : &'static [u8] = include_bytes!("../res/mac/icon.png");
+const STATUS_ICON_BINARY : &[u8] = include_bytes!("../res/mac/icon.png");
 
 pub struct MacContext {
     pub send_channel: Sender<Event>
@@ -42,7 +42,7 @@ impl MacContext {
             if res == 0 {
                 error!("Accessibility must be enabled to make espanso work on MacOS.");
                 error!("Please allow espanso in the Security & Privacy panel, then restart espanso.");
-                error!("For more information: "); // TODO: add documentation link
+                error!("For more information: https://espanso.org/install/mac/");
                 exit(1);
             }
         }

src/context/windows.rs

@@ -26,8 +26,8 @@ use std::{fs};
 use widestring::U16CString;
 use log::{info};
 
-const BMP_BINARY : &'static [u8] = include_bytes!("../res/win/espanso.bmp");
+const BMP_BINARY : &[u8] = include_bytes!("../res/win/espanso.bmp");
-const ICO_BINARY : &'static [u8] = include_bytes!("../res/win/espanso.ico");
+const ICO_BINARY : &[u8] = include_bytes!("../res/win/espanso.ico");
 
 pub struct WindowsContext {
     send_channel: Sender<Event>,

src/event/manager.rs

@@ -55,7 +55,7 @@ impl <'a> EventManager for DefaultEventManager<'a> {
                         }
                     }
                 },
-                Err(_) => panic!("Broken event channel"),
+                Err(e) => panic!("Broken event channel {}", e),
             }
         }
     }

src/main.rs

@@ -63,7 +63,7 @@ mod clipboard;
 mod extension;
 mod sysdaemon;
 
-const VERSION: &'static str = env!("CARGO_PKG_VERSION");
+const VERSION: &str = env!("CARGO_PKG_VERSION");
 const LOG_FILE: &str = "espanso.log";
 
 fn main() {
@@ -168,52 +168,52 @@ fn main() {
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("dump") {
+    if matches.subcommand_matches("dump").is_some() {
         println!("{:#?}", config_set);
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("detect") {
+    if matches.subcommand_matches("detect").is_some() {
         detect_main();
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("daemon") {
+    if matches.subcommand_matches("daemon").is_some() {
         daemon_main(config_set);
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("register") {
+    if matches.subcommand_matches("register").is_some() {
         register_main(config_set);
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("unregister") {
+    if matches.subcommand_matches("unregister").is_some() {
         unregister_main(config_set);
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("log") {
+    if matches.subcommand_matches("log").is_some() {
         log_main();
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("start") {
+    if matches.subcommand_matches("start").is_some() {
         start_main(config_set);
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("status") {
+    if matches.subcommand_matches("status").is_some() {
         status_main();
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("stop") {
+    if matches.subcommand_matches("stop").is_some() {
         stop_main(config_set);
         return;
     }
 
-    if let Some(_) = matches.subcommand_matches("restart") {
+    if matches.subcommand_matches("restart").is_some() {
         restart_main(config_set);
         return;
     }
@@ -236,7 +236,7 @@ fn main() {
             list_package_main(config_set, matches);
             return;
         }
-        if let Some(_) = matches.subcommand_matches("refresh") {
+        if matches.subcommand_matches("refresh").is_some() {
             update_index_main(config_set);
             return;
         }
@@ -525,22 +525,22 @@ fn detect_main() {
 
 /// Send the given command to the espanso daemon
 fn cmd_main(config_set: ConfigSet, matches: &ArgMatches) {
-    let command = if let Some(_) = matches.subcommand_matches("exit") {
+    let command = if matches.subcommand_matches("exit").is_some() {
         Some(IPCCommand {
             id: String::from("exit"),
             payload: String::from(""),
         })
-    }else if let Some(_) = matches.subcommand_matches("toggle") {
+    }else if matches.subcommand_matches("toggle").is_some() {
         Some(IPCCommand {
             id: String::from("toggle"),
             payload: String::from(""),
         })
-    }else if let Some(_) = matches.subcommand_matches("enable") {
+    }else if matches.subcommand_matches("enable").is_some() {
         Some(IPCCommand {
             id: String::from("enable"),
             payload: String::from(""),
         })
-    }else if let Some(_) = matches.subcommand_matches("disable") {
+    }else if matches.subcommand_matches("disable").is_some() {
         Some(IPCCommand {
             id: String::from("disable"),
             payload: String::from(""),
@@ -748,7 +748,7 @@ fn acquire_lock() -> Option<File> {
 
     let res = file.try_lock_exclusive();
 
-    if let Ok(_) = res {
+    if res.is_ok() {
         return Some(file)
     }
 

src/matcher/scrolling.rs

@@ -126,7 +126,7 @@ impl <'a, R: MatchReceiver, M: ConfigManager<'a>> super::Matcher for ScrollingMa
         if m == config.toggle_key {
             let mut toggle_press_time = self.toggle_press_time.borrow_mut();
             if let Ok(elapsed) = toggle_press_time.elapsed() {
-                if elapsed.as_millis() < config.toggle_interval as u128 {
+                if elapsed.as_millis() < u128::from(config.toggle_interval) {
                     self.toggle();
 
                     let is_enabled = self.is_enabled.borrow();

src/package/default.rs

@@ -117,16 +117,14 @@ impl DefaultPackageManager {
                     }else{
                         started = true;
                     }
-                }else{
+                }else if started {
-                    if started {
+                    let caps = FIELD_REGEX.captures(&line);
-                        let caps = FIELD_REGEX.captures(&line);
+                    if let Some(caps) = caps {
-                        if let Some(caps) = caps {
+                        let property = caps.get(1);
-                            let property = caps.get(1);
+                        let value = caps.get(2);
-                            let value = caps.get(2);
+                        if property.is_some() && value.is_some() {
-                            if property.is_some() && value.is_some() {
+                            fields.insert(property.unwrap().as_str().to_owned(),
-                                fields.insert(property.unwrap().as_str().to_owned(),
+                                          value.unwrap().as_str().to_owned());
-                                              value.unwrap().as_str().to_owned());
-                            }
                         }
                     }
                 }
@@ -161,7 +159,7 @@ impl DefaultPackageManager {
             return local_index.last_update
         }
 
-        return 0;
+        0
     }
 
     fn list_local_packages_names(&self) -> Vec<String> {
@@ -260,14 +258,14 @@ impl super::PackageManager for DefaultPackageManager {
         let readme_path = temp_package_dir.join("README.md");
 
         let package = Self::parse_package_from_readme(&readme_path);
-        if !package.is_some() {
+        if package.is_none() {
-            return Ok(InstallResult::UnableToParsePackageInfo);  // TODO: test
+            return Ok(InstallResult::UnableToParsePackageInfo);
         }
         let package = package.unwrap();
 
         let source_dir = temp_package_dir.join(package.version);
         if !source_dir.exists() {
-            return Ok(InstallResult::MissingPackageVersion);  // TODO: test
+            return Ok(InstallResult::MissingPackageVersion);
         }
 
         let target_dir = &self.package_dir.join(name);
@@ -317,7 +315,7 @@ mod tests {
     use std::path::Path;
     use crate::package::PackageManager;
     use std::fs::{create_dir, create_dir_all};
-    use crate::package::InstallResult::{Installed, NotFoundInRepo};
+    use crate::package::InstallResult::*;
     use std::io::Write;
 
     const OUTDATED_INDEX_CONTENT : &str = include_str!("../res/test/outdated_index.json");
@@ -496,6 +494,36 @@ mod tests {
         assert_eq!(temp.package_manager.install_package("not-existing").unwrap(), NotFoundInRepo);
     }
 
+    #[test]
+    fn test_install_package_missing_version() {
+        let mut temp = create_temp_package_manager(|_, data_dir| {
+            let index_file = data_dir.join(DEFAULT_PACKAGE_INDEX_FILE);
+            std::fs::write(index_file, INSTALL_PACKAGE_INDEX);
+        });
+
+        assert_eq!(temp.package_manager.install_package("dummy-package2").unwrap(), MissingPackageVersion);
+    }
+
+    #[test]
+    fn test_install_package_missing_readme_unable_to_parse_package_info() {
+        let mut temp = create_temp_package_manager(|_, data_dir| {
+            let index_file = data_dir.join(DEFAULT_PACKAGE_INDEX_FILE);
+            std::fs::write(index_file, INSTALL_PACKAGE_INDEX);
+        });
+
+        assert_eq!(temp.package_manager.install_package("dummy-package3").unwrap(), UnableToParsePackageInfo);
+    }
+
+    #[test]
+    fn test_install_package_bad_readme_unable_to_parse_package_info() {
+        let mut temp = create_temp_package_manager(|_, data_dir| {
+            let index_file = data_dir.join(DEFAULT_PACKAGE_INDEX_FILE);
+            std::fs::write(index_file, INSTALL_PACKAGE_INDEX);
+        });
+
+        assert_eq!(temp.package_manager.install_package("dummy-package4").unwrap(), UnableToParsePackageInfo);
+    }
+
     #[test]
     fn test_list_local_packages() {
         let mut temp = create_temp_package_manager(|_, data_dir| {

src/res/test/install_package_index.json

@@ -12,6 +12,36 @@
 
         },
 
+        {
+            "name": "dummy-package2",
+            "title": "Dummy Package",
+            "version": "9.9.9",
+            "repo": "https://github.com/federico-terzi/espanso-hub-core",
+            "desc": "Dummy package",
+            "author": "Federico Terzi"
+
+        },
+
+        {
+            "name": "dummy-package3",
+            "title": "Dummy Package",
+            "version": "0.1.0",
+            "repo": "https://github.com/federico-terzi/espanso-hub-core",
+            "desc": "Dummy package",
+            "author": "Federico Terzi"
+
+        },
+
+        {
+            "name": "dummy-package4",
+            "title": "Dummy Package",
+            "version": "0.1.0",
+            "repo": "https://github.com/federico-terzi/espanso-hub-core",
+            "desc": "Dummy package",
+            "author": "Federico Terzi"
+
+        },
+
 
         {
             "name": "italian-accents",

src/ui/linux.rs

@@ -22,7 +22,7 @@ use super::MenuItem;
 use log::{error, info};
 use std::path::PathBuf;
 
-const LINUX_ICON_CONTENT : &'static [u8] = include_bytes!("../res/linux/icon.png");
+const LINUX_ICON_CONTENT : &[u8] = include_bytes!("../res/linux/icon.png");
 
 pub struct LinuxUIManager {
     icon_path: PathBuf,