From eaf6ef590086e0ebdc70c8e771f1ecf2f556d76c Mon Sep 17 00:00:00 2001 From: Ajeet D'Souza <98ajeet@gmail.com> Date: Mon, 25 May 2020 00:39:27 +0530 Subject: [PATCH] Minor refactor --- CHANGELOG.md | 4 ++++ src/config.rs | 6 ++++-- src/db.rs | 44 ++++++++++++++++------------------------ src/fzf.rs | 2 +- src/subcommand/add.rs | 32 +++++++++++++---------------- src/subcommand/import.rs | 25 +++++++++++------------ src/subcommand/query.rs | 2 -- src/subcommand/remove.rs | 39 ++++++++++++++++------------------- src/util.rs | 10 ++++++--- 9 files changed, 78 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 684894a..2212703 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/config.rs b/src/config.rs index e85ce0c..a8bc920 100644 --- a/src/config.rs +++ b/src/config.rs @@ -35,12 +35,14 @@ pub fn zo_exclude_dirs() -> Vec { pub fn zo_maxage() -> Result { 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::().with_context(|| { format!("unable to parse _ZO_MAXAGE as integer: {}", maxage_str) })?; Ok(maxage as Rank) - }, + } None => Ok(1000.0), } } diff --git a/src/db.rs b/src/db.rs index 50865e1..4d95671 100644 --- a/src/db.rs +++ b/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(|| { - format!("could not read from database: {}", file_path.display()) - }) + return Err(e) + .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!( - "could not deserialize database version: {}", - file_path.display(), - ) + format!("could not deserialize database version: {}", path.display()) })?; let dirs = match version { - Self::CURRENT_VERSION => deserializer.deserialize(buffer_dirs).with_context(|| { - format!("could not deserialize database: {}", file_path.display()) - })?, + Self::CURRENT_VERSION => deserializer + .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 - .last() - .and_then(|query_last| Path::new(query_last).file_name()) - { - if let Some(dir_name) = Path::new(&path_lower).file_name() { - // - // 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(); + let get_filenames = || { + let query_name = Path::new(query.last()?).file_name()?.to_str()?; + let dir_name = Path::new(&path_lower).file_name()?.to_str()?; + Some((query_name, dir_name)) + }; - if !dir_name_str.contains(query_name_str) { - return false; - } + if let Some((query_name, dir_name)) = get_filenames() { + if !dir_name.contains(query_name) { + return false; } } diff --git a/src/fzf.rs b/src/fzf.rs index 211ab1a..629b182 100644 --- a/src/fzf.rs +++ b/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 = diff --git a/src/subcommand/add.rs b/src/subcommand/add.rs index b5e0403..894f533 100644 --- a/src/subcommand/add.rs +++ b/src/subcommand/add.rs @@ -1,6 +1,6 @@ use crate::config; -use crate::db::{Dir, Rank}; -use crate::util::{get_current_time, get_db, path_to_str}; +use crate::db::{Db, Dir, Rank}; +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>(path: P) -> Result<()> { let path = path.as_ref(); - let path = dunce::canonicalize(path) - .with_context(|| format!("could not resolve directory: {}", path.display()))?; + let path = canonicalize(&path)?; - let exclude_dirs = config::zo_exclude_dirs(); - if exclude_dirs - .iter() - .any(|excluded_path| excluded_path == &path) - { + if config::zo_exclude_dirs().contains(&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>(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::(); if sum_age > maxage { @@ -76,8 +76,4 @@ fn add>(path: P) -> Result<()> { } } } - - db.modified = true; - - Ok(()) } diff --git a/src/subcommand/import.rs b/src/subcommand/import.rs index dc574a2..fafb5aa 100644 --- a/src/subcommand/import.rs +++ b/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>(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>(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>(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()?; - Some((path_str, epoch_str, rank_str)) + let path = split_line.next()?; + Some((path, epoch_str, rank_str)) })() - .context("invalid entry")?; + .with_context(|| format!("invalid entry: {}", line))?; let epoch = epoch_str .parse::() @@ -67,19 +68,17 @@ fn import_line(db: &mut Db, line: &str) -> Result<()> { .parse::() .with_context(|| format!("invalid rank: {}", rank_str))?; - let path = dunce::canonicalize(path_str) - .with_context(|| format!("could not resolve path: {}", path_str))?; - - let path_str = path_to_str(&path)?; + let path = canonicalize(&path)?; + let path = 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. - if let Some(dir) = db.dirs.iter_mut().find(|dir| dir.path == path_str) { + // 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) { 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, }); diff --git a/src/subcommand/query.rs b/src/subcommand/query.rs index bf4cb5c..a4ad365 100644 --- a/src/subcommand/query.rs +++ b/src/subcommand/query.rs @@ -51,8 +51,6 @@ fn query(keywords: &[String]) -> Result> { 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) { diff --git a/src/subcommand/remove.rs b/src/subcommand/remove.rs index 2a71dcd..530ce8b 100644 --- a/src/subcommand/remove.rs +++ b/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() { - remove(&path) - } else { - clap::Error::with_description( - &format!( - "remove requires 1 value in non-interactive mode, but {} were provided", - self.query.len() - ), - clap::ErrorKind::WrongNumberOfValues, - ) - .exit(); - } + clap::Error::with_description( + &format!( + "remove requires 1 value in non-interactive mode, but {} were provided", + self.query.len() + ), + 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) - .with_context(|| format!("could not resolve path: {}", path_str))?; - let path_abs_str = path_to_str(&path_abs)?; + let path = canonicalize(&path)?; + let path = path_to_str(&path)?; - 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<()> { diff --git a/src/util.rs b/src/util.rs index fe9eb65..5c30e83 100644 --- a/src/util.rs +++ b/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 { @@ -17,12 +17,16 @@ pub fn get_current_time() -> Result { .context("system clock set to invalid time")? .as_secs(); - Ok(current_time as Epoch) + Ok(current_time as _) +} + +pub fn canonicalize>(path: &P) -> Result { + let path = path.as_ref(); + dunce::canonicalize(path).with_context(|| format!("could not resolve path: {}", path.display())) } pub fn path_to_str>(path: &P) -> Result<&str> { let path = path.as_ref(); - path.to_str() .with_context(|| format!("invalid utf-8 sequence in path: {}", path.display())) }