Minor refactor

CHANGELOG.md

@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Removed
 
+- Support for non-UTF8 paths.
+
+### Removed
+
 - Backward compatibility with `v0.2.x` databases.
 
 ## [0.4.0] - 2020-05-03

src/config.rs

@@ -35,12 +35,14 @@ pub fn zo_exclude_dirs() -> Vec<PathBuf> {
 pub fn zo_maxage() -> Result<Rank> {
     match env::var_os("_ZO_MAXAGE") {
         Some(maxage_osstr) => {
-            let maxage_str = maxage_osstr.to_str().context("invalid utf-8 sequence in _ZO_MAXAGE")?;
+            let maxage_str = maxage_osstr
+                .to_str()
+                .context("invalid utf-8 sequence in _ZO_MAXAGE")?;
             let maxage = maxage_str.parse::<u64>().with_context(|| {
                 format!("unable to parse _ZO_MAXAGE as integer: {}", maxage_str)
             })?;
             Ok(maxage as Rank)
-        },
+        }
         None => Ok(1000.0),
     }
 }

src/db.rs

@@ -23,9 +23,9 @@ impl Db {
         fs::create_dir_all(&data_dir)
             .with_context(|| format!("unable to create data directory: {}", data_dir.display()))?;
 
-        let file_path = Self::get_path(&data_dir);
+        let path = Self::get_path(&data_dir);
 
-        let buffer = match fs::read(&file_path) {
+        let buffer = match fs::read(&path) {
             Ok(buffer) => buffer,
             Err(e) if e.kind() == io::ErrorKind::NotFound => {
                 return Ok(Db {
@@ -35,9 +35,8 @@ impl Db {
                 })
             }
             Err(e) => {
-                return Err(e).with_context(|| {
+                return Err(e)
-                    format!("could not read from database: {}", file_path.display())
+                    .with_context(|| format!("could not read from database: {}", path.display()))
-                })
             }
         };
 
@@ -54,7 +53,7 @@ impl Db {
             as _;
 
         if buffer.len() < version_size {
-            bail!("database is corrupted: {}", file_path.display());
+            bail!("database is corrupted: {}", path.display());
         }
 
         let (buffer_version, buffer_dirs) = buffer.split_at(version_size);
@@ -63,21 +62,18 @@ impl Db {
         deserializer.limit(Self::MAX_SIZE);
 
         let version = deserializer.deserialize(buffer_version).with_context(|| {
-            format!(
+            format!("could not deserialize database version: {}", path.display())
-                "could not deserialize database version: {}",
-                file_path.display(),
-            )
         })?;
 
         let dirs = match version {
-            Self::CURRENT_VERSION => deserializer.deserialize(buffer_dirs).with_context(|| {
+            Self::CURRENT_VERSION => deserializer
-                format!("could not deserialize database: {}", file_path.display())
+                .deserialize(buffer_dirs)
-            })?,
+                .with_context(|| format!("could not deserialize database: {}", path.display()))?,
             DbVersion(version_num) => bail!(
                 "zoxide {} does not support schema v{}: {}",
                 env!("ZOXIDE_VERSION"),
                 version_num,
-                file_path.display(),
+                path.display(),
             ),
         };
 
@@ -189,19 +185,15 @@ impl Dir {
     pub fn is_match(&self, query: &[String]) -> bool {
         let path_lower = self.path.to_lowercase();
 
-        if let Some(query_name) = query
+        let get_filenames = || {
-            .last()
+            let query_name = Path::new(query.last()?).file_name()?.to_str()?;
-            .and_then(|query_last| Path::new(query_last).file_name())
+            let dir_name = Path::new(&path_lower).file_name()?.to_str()?;
-        {
+            Some((query_name, dir_name))
-            if let Some(dir_name) = Path::new(&path_lower).file_name() {
+        };
-                // <https://github.com/rust-lang/rust/issues/49802>
-                // unwrap is safe here because we've already handled invalid UTF-8
-                let dir_name_str = dir_name.to_str().unwrap();
-                let query_name_str = query_name.to_str().unwrap();
 
-                if !dir_name_str.contains(query_name_str) {
+        if let Some((query_name, dir_name)) = get_filenames() {
-                    return false;
+            if !dir_name.contains(query_name) {
-                }
+                return false;
             }
         }
 

src/fzf.rs

@@ -62,7 +62,7 @@ impl Fzf {
             Some(0) => {
                 let path_bytes = output
                     .stdout
-                    .get(12..output.stdout.len() - 1)
+                    .get(12..output.stdout.len().saturating_sub(1))
                     .context("fzf returned invalid output")?;
 
                 let path_str =

src/subcommand/add.rs

@@ -1,6 +1,6 @@
 use crate::config;
-use crate::db::{Dir, Rank};
+use crate::db::{Db, Dir, Rank};
-use crate::util::{get_current_time, get_db, path_to_str};
+use crate::util::{canonicalize, get_current_time, get_db, path_to_str};
 
 use anyhow::{Context, Result};
 use structopt::StructOpt;
@@ -31,27 +31,20 @@ impl Add {
 
 fn add<P: AsRef<Path>>(path: P) -> Result<()> {
     let path = path.as_ref();
-    let path = dunce::canonicalize(path)
+    let path = canonicalize(&path)?;
-        .with_context(|| format!("could not resolve directory: {}", path.display()))?;
 
-    let exclude_dirs = config::zo_exclude_dirs();
+    if config::zo_exclude_dirs().contains(&path) {
-    if exclude_dirs
-        .iter()
-        .any(|excluded_path| excluded_path == &path)
-    {
         return Ok(());
     }
 
-    let path_str = path_to_str(&path)?;
-
     let mut db = get_db()?;
     let now = get_current_time()?;
-
+    let path = path_to_str(&path)?;
     let maxage = config::zo_maxage()?;
 
-    match db.dirs.iter_mut().find(|dir| dir.path == path_str) {
+    match db.dirs.iter_mut().find(|dir| dir.path == path) {
         None => db.dirs.push(Dir {
-            path: path_str.to_string(),
+            path: path.to_string(),
             last_accessed: now,
             rank: 1.0,
         }),
@@ -61,6 +54,13 @@ fn add<P: AsRef<Path>>(path: P) -> Result<()> {
         }
     };
 
+    age(&mut db, maxage);
+    db.modified = true;
+
+    Ok(())
+}
+
+fn age(db: &mut Db, maxage: Rank) {
     let sum_age = db.dirs.iter().map(|dir| dir.rank).sum::<Rank>();
 
     if sum_age > maxage {
@@ -76,8 +76,4 @@ fn add<P: AsRef<Path>>(path: P) -> Result<()> {
             }
         }
     }
-
-    db.modified = true;
-
-    Ok(())
 }

src/subcommand/import.rs

@@ -1,5 +1,5 @@
 use crate::db::{Db, Dir};
-use crate::util::{get_db, path_to_str};
+use crate::util::{canonicalize, get_db, path_to_str};
 
 use anyhow::{bail, Context, Result};
 use structopt::StructOpt;
@@ -23,6 +23,7 @@ impl Import {
 }
 
 fn import<P: AsRef<Path>>(path: P, merge: bool) -> Result<()> {
+    let path = path.as_ref();
     let mut db = get_db()?;
 
     if !db.dirs.is_empty() && !merge {
@@ -33,7 +34,7 @@ fn import<P: AsRef<Path>>(path: P, merge: bool) -> Result<()> {
     }
 
     let buffer = fs::read_to_string(&path)
-        .with_context(|| format!("could not read z database: {}", path.as_ref().display()))?;
+        .with_context(|| format!("could not read z database: {}", path.display()))?;
 
     for (idx, line) in buffer.lines().enumerate() {
         if let Err(e) = import_line(&mut db, line) {
@@ -51,13 +52,13 @@ fn import<P: AsRef<Path>>(path: P, merge: bool) -> Result<()> {
 fn import_line(db: &mut Db, line: &str) -> Result<()> {
     let mut split_line = line.rsplitn(3, '|');
 
-    let (path_str, epoch_str, rank_str) = (|| {
+    let (path, epoch_str, rank_str) = (|| {
         let epoch_str = split_line.next()?;
         let rank_str = split_line.next()?;
-        let path_str = split_line.next()?;
+        let path = split_line.next()?;
-        Some((path_str, epoch_str, rank_str))
+        Some((path, epoch_str, rank_str))
     })()
-    .context("invalid entry")?;
+    .with_context(|| format!("invalid entry: {}", line))?;
 
     let epoch = epoch_str
         .parse::<i64>()
@@ -67,19 +68,17 @@ fn import_line(db: &mut Db, line: &str) -> Result<()> {
         .parse::<f64>()
         .with_context(|| format!("invalid rank: {}", rank_str))?;
 
-    let path = dunce::canonicalize(path_str)
+    let path = canonicalize(&path)?;
-        .with_context(|| format!("could not resolve path: {}", path_str))?;
+    let path = path_to_str(&path)?;
-
-    let path_str = path_to_str(&path)?;
 
     // If the path exists in the database, add the ranks and set the epoch to
-    // the largest of the parsed epoch and the already present epoch.
+    // the more recent of the parsed epoch and the already present epoch.
-    if let Some(dir) = db.dirs.iter_mut().find(|dir| dir.path == path_str) {
+    if let Some(dir) = db.dirs.iter_mut().find(|dir| dir.path == path) {
         dir.rank += rank;
         dir.last_accessed = epoch.max(dir.last_accessed);
     } else {
         db.dirs.push(Dir {
-            path: path_str.to_string(),
+            path: path.to_string(),
             rank,
             last_accessed: epoch,
         });

src/subcommand/query.rs

@@ -51,8 +51,6 @@ fn query(keywords: &[String]) -> Result<Option<String>> {
     db.dirs
         .sort_unstable_by_key(|dir| FloatOrd(dir.get_frecency(now)));
 
-    // Iterating in reverse order ensures that the directory indices do not
-    // change as we remove them.
     for idx in (0..db.dirs.len()).rev() {
         let dir = &db.dirs[idx];
         if !dir.is_match(&keywords) {

src/subcommand/remove.rs

@@ -1,7 +1,7 @@
 use crate::fzf::Fzf;
-use crate::util::{get_current_time, get_db, path_to_str};
+use crate::util::{canonicalize, get_current_time, get_db, path_to_str};
 
-use anyhow::{bail, Context, Result};
+use anyhow::{bail, Result};
 use structopt::StructOpt;
 
 #[derive(Debug, StructOpt)]
@@ -16,43 +16,40 @@ impl Remove {
     pub fn run(&self) -> Result<()> {
         if self.interactive {
             remove_interactive(&self.query)
+        } else if let [path] = self.query.as_slice() {
+            remove(&path)
         } else {
-            if let &[path] = &self.query.as_slice() {
+            clap::Error::with_description(
-                remove(&path)
+                &format!(
-            } else {
+                    "remove requires 1 value in non-interactive mode, but {} were provided",
-                clap::Error::with_description(
+                    self.query.len()
-                    &format!(
+                ),
-                        "remove requires 1 value in non-interactive mode, but {} were provided",
+                clap::ErrorKind::WrongNumberOfValues,
-                        self.query.len()
+            )
-                    ),
+            .exit();
-                    clap::ErrorKind::WrongNumberOfValues,
-                )
-                .exit();
-            }
         }
     }
 }
 
-fn remove(path_str: &str) -> Result<()> {
+fn remove(path: &str) -> Result<()> {
     let mut db = get_db()?;
 
-    if let Some(idx) = db.dirs.iter().position(|dir| &dir.path == path_str) {
+    if let Some(idx) = db.dirs.iter().position(|dir| dir.path == path) {
         db.dirs.swap_remove(idx);
         db.modified = true;
         return Ok(());
     }
 
-    let path_abs = dunce::canonicalize(path_str)
+    let path = canonicalize(&path)?;
-        .with_context(|| format!("could not resolve path: {}", path_str))?;
+    let path = path_to_str(&path)?;
-    let path_abs_str = path_to_str(&path_abs)?;
 
-    if let Some(idx) = db.dirs.iter().position(|dir| dir.path == path_abs_str) {
+    if let Some(idx) = db.dirs.iter().position(|dir| dir.path == path) {
         db.dirs.swap_remove(idx);
         db.modified = true;
         return Ok(());
     }
 
-    bail!("could not find path in database: {}", path_str)
+    bail!("could not find path in database: {}", path)
 }
 
 fn remove_interactive(keywords: &[String]) -> Result<()> {

src/util.rs

@@ -3,7 +3,7 @@ use crate::db::{Db, Epoch};
 
 use anyhow::{Context, Result};
 
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::time::SystemTime;
 
 pub fn get_db() -> Result<Db> {
@@ -17,12 +17,16 @@ pub fn get_current_time() -> Result<Epoch> {
         .context("system clock set to invalid time")?
         .as_secs();
 
-    Ok(current_time as Epoch)
+    Ok(current_time as _)
+}
+
+pub fn canonicalize<P: AsRef<Path>>(path: &P) -> Result<PathBuf> {
+    let path = path.as_ref();
+    dunce::canonicalize(path).with_context(|| format!("could not resolve path: {}", path.display()))
 }
 
 pub fn path_to_str<P: AsRef<Path>>(path: &P) -> Result<&str> {
     let path = path.as_ref();
-
     path.to_str()
         .with_context(|| format!("invalid utf-8 sequence in path: {}", path.display()))
 }