Merge pull request #1023 from emma31-dev/test_changes

Minor refractors to improve DX

Cargo.lock

@@ -320,9 +320,9 @@ checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe"
 
 [[package]]
 name = "libc"
-version = "0.2.169"
+version = "0.2.184"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a"
+checksum = "48f5d2a454e16a5ea0f4ced81bd44e4cfc7bd3a507b61887c99fd3538b28e4af"
 
 [[package]]
 name = "linux-raw-sys"

Cargo.toml

@@ -24,7 +24,7 @@ regex = { version = "1.7.3", default-features = false, features = [
    "unicode-perl",
 ] }
 clap = { version = "4.2.1", features = ["derive", "cargo"] }
-crossterm = "0.28.0"
+crossterm = "0.28.1"
 lazy_static = "1.4.0"
 etcetera = "0.10.0"
 walkdir = "2.3.3"

src/bin/main.rs

@@ -6,7 +6,7 @@ use thiserror::Error;
 #[derive(Error, Debug)]
 #[error(
     "\rHey, listen! navi encountered a problem.
-Do you think this is a bug? File an issue at https://github.com/denisidoro/navi."
+Do you think this is a bug? File an issue at https://github.com/denisidoro/navi/issues."
 )]
 pub struct FileAnIssue {
     #[source]

src/common/clipboard.rs

@@ -1,5 +1,5 @@
-use crate::common::shell::{self, ShellSpawnError, EOF};
-use crate::prelude::*;
+use crate::common::shell::{out, ShellSpawnError, EOF};
+use anyhow::Result;
 
 pub fn copy(text: String) -> Result<()> {
     let cmd = r#"
@@ -19,15 +19,16 @@ _copy() {
    fi
 }"#;
 
-    shell::out()
+    out()
         .arg(
             format!(
-                r#"{cmd} 
-        read -r -d '' x <<'{EOF}'
+                r#"
+{cmd} 
+read -r -d '' x <<'{EOF}'
 {text}
 {EOF}
 
-echo -n "$x" | _copy"#,
+echo -n "$x" | _copy"#
             )
             .as_str(),
         )

src/common/fs.rs

@@ -1,8 +1,9 @@
 use crate::prelude::*;
 use remove_dir_all::remove_dir_all;
+use std::env::current_exe;
 use std::ffi::OsStr;
-use std::fs::{self, create_dir_all, File};
-use std::io;
+use std::fs::{create_dir_all, read_link, File};
+use std::io::BufReader;
 use thiserror::Error;
 
 pub trait ToStringExt {
@@ -33,30 +34,29 @@ pub struct UnreadableDir {
     source: anyhow::Error,
 }
 
-pub fn open(filename: &Path) -> Result<File> {
-    File::open(filename).with_context(|| {
-        let x = filename.to_string();
-        format!("Failed to open file {}", &x)
+pub fn open<P: AsRef<Path>>(filename: P) -> Result<File> {
+    File::open(filename.as_ref()).with_context(|| {
+        let x = filename.as_ref().to_string();
+        format!("Failed to open file {}", x)
     })
 }
 
-pub fn read_lines(filename: &Path) -> Result<impl Iterator<Item = Result<String>>> {
-    let file = open(filename)?;
-    Ok(io::BufReader::new(file)
-        .lines()
-        .map(|line| line.map_err(Error::from)))
+pub fn read_lines<P: AsRef<Path>>(filename: P) -> Result<impl Iterator<Item = Result<String>>> {
+    let file = open(filename.as_ref())?;
+    Ok(BufReader::new(file).lines().map(|line| line.map_err(Error::from)))
 }
 
-pub fn pathbuf_to_string(pathbuf: &Path) -> Result<String> {
+fn pathbuf_to_string<P: AsRef<Path>>(pathbuf: P) -> Result<String> {
     Ok(pathbuf
+        .as_ref()
         .as_os_str()
         .to_str()
-        .ok_or_else(|| InvalidPath(pathbuf.to_path_buf()))
+        .ok_or_else(|| InvalidPath(pathbuf.as_ref().to_path_buf()))
         .map(str::to_string)?)
 }
 
 fn follow_symlink(pathbuf: PathBuf) -> Result<PathBuf> {
-    fs::read_link(pathbuf.clone())
+    read_link(pathbuf.clone())
         .map(|o| {
             let o_str = o
                 .as_os_str()
@@ -77,7 +77,7 @@ fn follow_symlink(pathbuf: PathBuf) -> Result<PathBuf> {
 }
 
 fn exe_pathbuf() -> Result<PathBuf> {
-    let pathbuf = std::env::current_exe().context("Unable to acquire executable's path")?;
+    let pathbuf = current_exe().context("Unable to acquire executable's path")?;
 
     #[cfg(target_family = "windows")]
     let pathbuf = dunce::canonicalize(pathbuf)?;
@@ -108,8 +108,8 @@ pub fn exe_string() -> String {
     exe_abs_string().unwrap_or_else(|_| "navi".to_string())
 }
 
-pub fn create_dir(path: &Path) -> Result<()> {
-    create_dir_all(path).with_context(|| {
+pub fn create_dir<P: AsRef<Path>>(path: P) -> Result<()> {
+    create_dir_all(path.as_ref()).with_context(|| {
         format!(
             "Failed to create directory `{}`",
             pathbuf_to_string(path).expect("Unable to parse {path}")
@@ -117,8 +117,8 @@ pub fn create_dir(path: &Path) -> Result<()> {
     })
 }
 
-pub fn remove_dir(path: &Path) -> Result<()> {
-    remove_dir_all(path).with_context(|| {
+pub fn remove_dir<P: AsRef<Path>>(path: P) -> Result<()> {
+    remove_dir_all(path.as_ref()).with_context(|| {
         format!(
             "Failed to remove directory `{}`",
             pathbuf_to_string(path).expect("Unable to parse {path}")

src/common/git.rs

@@ -15,8 +15,21 @@ pub fn shallow_clone(uri: &str, target: &str) -> Result<()> {
 pub fn meta(uri: &str) -> (String, String, String) {
     let actual_uri = if uri.contains("://") || uri.contains('@') {
         uri.to_string()
+    } else if let Some((domain, route)) = uri.split_once('/') {
+        if domain.contains(".") {
+            format!("https://{domain}/{route}")
+        } else {
+            // Users can pass name starting wirh a slash
+            let first_char = uri.chars().next();
+
+            if first_char == Some('/') {
+                format!("https://github.com{uri}")
+            } else {
+                format!("https://github.com/{uri}")
+            }
+        }
     } else {
-        format!("https://github.com/{uri}")
+        panic!("Invalid link")
     };
 
     let uri_to_split = actual_uri.replace(':', "/");
@@ -54,4 +67,36 @@ mod tests {
         assert_eq!(user, "user".to_string());
         assert_eq!(repo, "repo".to_string());
     }
+
+    #[test]
+    fn test_meta_github_repo_name() {
+        let (actual_uri, user, repo) = meta("user/repo");
+        assert_eq!(actual_uri, "https://github.com/user/repo".to_string());
+        assert_eq!(user, "user".to_string());
+        assert_eq!(repo, "repo".to_string());
+    }
+
+    #[test]
+    fn test_meta_github_repo_name_with_slash() {
+        let (actual_uri, user, repo) = meta("/user/repo");
+        assert_eq!(actual_uri, "https://github.com/user/repo".to_string());
+        assert_eq!(user, "user".to_string());
+        assert_eq!(repo, "repo".to_string());
+    }
+
+    #[test]
+    fn test_meta_github_clean_link() {
+        let (actual_uri, user, repo) = meta("github.com/user/repo");
+        assert_eq!(actual_uri, "https://github.com/user/repo".to_string());
+        assert_eq!(user, "user".to_string());
+        assert_eq!(repo, "repo".to_string());
+    }
+
+    #[test]
+    fn test_meta_random_git_provider_http() {
+        let (actual_uri, user, repo) = meta("https://sr.ht/user/repo");
+        assert_eq!(actual_uri, "https://sr.ht/user/repo".to_string());
+        assert_eq!(user, "user".to_string());
+        assert_eq!(repo, "repo".to_string());
+    }
 }

src/common/hash.rs

@@ -1,30 +1,7 @@
-use std::hash::{Hash, Hasher};
-
-const MAGIC_INIT: u64 = 0x811C_9DC5;
+use std::hash::{DefaultHasher, Hash, Hasher};
 
 pub fn fnv<T: Hash>(x: &T) -> u64 {
-    let mut hasher = FnvHasher::new();
+    let mut hasher = DefaultHasher::new();
     x.hash(&mut hasher);
     hasher.finish()
 }
-
-struct FnvHasher(u64);
-
-impl FnvHasher {
-    fn new() -> Self {
-        FnvHasher(MAGIC_INIT)
-    }
-}
-
-impl Hasher for FnvHasher {
-    fn finish(&self) -> u64 {
-        self.0
-    }
-
-    fn write(&mut self, bytes: &[u8]) {
-        for byte in bytes.iter() {
-            self.0 ^= u64::from(*byte);
-            self.0 = self.0.wrapping_mul(0x0100_0000_01b3);
-        }
-    }
-}

src/common/shell.rs

@@ -1,4 +1,4 @@
-use crate::prelude::*;
+use crate::config::CONFIG;
 use clap::ValueEnum;
 use std::process::Command;
 use thiserror::Error;
@@ -24,9 +24,10 @@ pub struct ShellSpawnError {
 }
 
 impl ShellSpawnError {
-    pub fn new<SourceError>(command: impl Into<String>, source: SourceError) -> Self
+    pub fn new<SourceError, T>(command: T, source: SourceError) -> Self
     where
         SourceError: std::error::Error + Sync + Send + 'static,
+        T: Into<String>,
     {
         ShellSpawnError {
             command: command.into(),

src/common/terminal.rs

@@ -1,5 +1,4 @@
 use crate::prelude::*;
-use crossterm::style;
 use crossterm::terminal;
 
 use std::process::Command;
@@ -45,21 +44,15 @@ pub fn width() -> u16 {
     }
 }
 
-pub fn parse_ansi(ansi: &str) -> Option<style::Color> {
-    style::Color::parse_ansi(&format!("5;{ansi}"))
-}
+#[cfg(test)]
+mod tests {
+    use super::*;
 
-#[derive(Debug, Clone)]
-pub struct Color(#[allow(unused)] pub style::Color); // suppress warning: field `0` is never read.
+    #[test]
+    fn test_width() {
+        let result = width();
+        let is_ok = if result == 0 { false } else { true };
 
-impl FromStr for Color {
-    type Err = &'static str;
-
-    fn from_str(ansi: &str) -> Result<Self, Self::Err> {
-        if let Some(c) = parse_ansi(ansi) {
-            Ok(Color(c))
-        } else {
-            Err("Invalid color")
-        }
+        assert!(is_ok);
     }
 }

src/filesystem.rs

@@ -98,12 +98,12 @@ pub fn cheat_paths(path: Option<String>) -> Result<String> {
 ///
 /// We are currently handling two cases: When the platform is `macOS` and when the platform isn't (including `Windows` and `Linux/Unix` platforms)
 fn get_data_dir_by_platform() -> Result<PathBuf> {
-    if cfg!(target_os = "macos") {
-        let base_dirs = etcetera::base_strategy::Apple::new()?;
+    if cfg!(target_os = "windows") {
+        let base_dirs = etcetera::choose_base_strategy()?;
 
         Ok(base_dirs.data_dir())
     } else {
-        let base_dirs = etcetera::choose_base_strategy()?;
+        let base_dirs = etcetera::base_strategy::Xdg::new()?;
 
         Ok(base_dirs.data_dir())
     }
@@ -113,12 +113,12 @@ fn get_data_dir_by_platform() -> Result<PathBuf> {
 ///
 /// We are currently handling two cases: When the platform is `macOS` and when the platform isn't (including `Windows` and `Linux/Unix` platforms)
 fn get_config_dir_by_platform() -> Result<PathBuf> {
-    if cfg!(target_os = "macos") {
-        let base_dirs = etcetera::base_strategy::Apple::new()?;
+    if cfg!(target_os = "windows") {
+        let base_dirs = etcetera::choose_base_strategy()?;
 
         Ok(base_dirs.config_dir())
     } else {
-        let base_dirs = etcetera::choose_base_strategy()?;
+        let base_dirs = etcetera::base_strategy::Xdg::new()?;
 
         Ok(base_dirs.config_dir())
     }