From 73596971bf570b7f15dd36db0ebc54fccbac8f02 Mon Sep 17 00:00:00 2001 From: Swarnim Arun Date: Fri, 12 May 2023 02:34:52 +0530 Subject: [PATCH] fix: path input consistency & minor code cleanup this should fix and improve cross platform experience of the application without breaking compatibility with anything, but needs more testing so if anyone wants to try it out they are welcome --- src/cmd/cmd.rs | 6 +++--- src/db/dir.rs | 5 +++-- src/db/mod.rs | 16 ++++++++-------- src/db/stream.rs | 19 ++++++++++++++----- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/cmd/cmd.rs b/src/cmd/cmd.rs index 5a78a83e..a1e74fde 100644 --- a/src/cmd/cmd.rs +++ b/src/cmd/cmd.rs @@ -64,11 +64,11 @@ pub struct Edit { #[derive(Clone, Debug, Subcommand)] pub enum EditCommand { #[clap(hide = true)] - Decrement { path: String }, + Decrement { path: PathBuf }, #[clap(hide = true)] - Delete { path: String }, + Delete { path: PathBuf }, #[clap(hide = true)] - Increment { path: String }, + Increment { path: PathBuf }, #[clap(hide = true)] Reload, } diff --git a/src/db/dir.rs b/src/db/dir.rs index 5d6d62cb..48753617 100644 --- a/src/db/dir.rs +++ b/src/db/dir.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; use std::fmt::{self, Display, Formatter}; +use std::path::Path; use serde::{Deserialize, Serialize}; @@ -8,7 +9,7 @@ use crate::util::{DAY, HOUR, WEEK}; #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Dir<'a> { #[serde(borrow)] - pub path: Cow<'a, str>, + pub path: Cow<'a, Path>, pub rank: Rank, pub last_accessed: Epoch, } @@ -61,7 +62,7 @@ impl Display for DirDisplay<'_> { let score = self.dir.score(now).clamp(0.0, 9999.0); write!(f, "{score:>6.1}{}", self.separator)?; } - write!(f, "{}", self.dir.path) + write!(f, "{}", self.dir.path.display()) } } diff --git a/src/db/mod.rs b/src/db/mod.rs index 8eb9fc6a..275fb675 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -64,11 +64,11 @@ impl Database { } /// Increments the rank of a directory, or creates it if it does not exist. - pub fn add(&mut self, path: impl AsRef + Into, by: Rank, now: Epoch) { + pub fn add(&mut self, path: impl AsRef, by: Rank, now: Epoch) { self.with_dirs_mut(|dirs| match dirs.iter_mut().find(|dir| dir.path == path.as_ref()) { Some(dir) => dir.rank = (dir.rank + by).max(0.0), None => { - dirs.push(Dir { path: path.into().into(), rank: by.max(0.0), last_accessed: now }) + dirs.push(Dir { path: path.as_ref().to_owned().into(), rank: by.max(0.0), last_accessed: now }) } }); self.with_dirty_mut(|dirty| *dirty = true); @@ -77,23 +77,23 @@ impl Database { /// Creates a new directory. This will create a duplicate entry if this /// directory is always in the database, it is expected that the user either /// does a check before calling this, or calls `dedup()` afterward. - pub fn add_unchecked(&mut self, path: impl AsRef + Into, rank: Rank, now: Epoch) { + pub fn add_unchecked(&mut self, path: impl AsRef, rank: Rank, now: Epoch) { self.with_dirs_mut(|dirs| { - dirs.push(Dir { path: path.into().into(), rank, last_accessed: now }) + dirs.push(Dir { path: path.as_ref().to_owned().into(), rank, last_accessed: now }) }); self.with_dirty_mut(|dirty| *dirty = true); } /// Increments the rank and updates the last_accessed of a directory, or /// creates it if it does not exist. - pub fn add_update(&mut self, path: impl AsRef + Into, by: Rank, now: Epoch) { + pub fn add_update(&mut self, path: impl AsRef, by: Rank, now: Epoch) { self.with_dirs_mut(|dirs| match dirs.iter_mut().find(|dir| dir.path == path.as_ref()) { Some(dir) => { dir.rank = (dir.rank + by).max(0.0); dir.last_accessed = now; } None => { - dirs.push(Dir { path: path.into().into(), rank: by.max(0.0), last_accessed: now }) + dirs.push(Dir { path: path.as_ref().to_owned().into(), rank: by.max(0.0), last_accessed: now }) } }); self.with_dirty_mut(|dirty| *dirty = true); @@ -101,7 +101,7 @@ impl Database { /// Removes the directory with `path` from the store. This does not preserve /// ordering, but is O(1). - pub fn remove(&mut self, path: impl AsRef) -> bool { + pub fn remove(&mut self, path: impl AsRef) -> bool { match self.dirs().iter().position(|dir| dir.path == path.as_ref()) { Some(idx) => { self.swap_remove(idx); @@ -256,7 +256,7 @@ mod tests { assert_eq!(db.dirs().len(), 1); let dir = &db.dirs()[0]; - assert_eq!(dir.path, path); + assert_eq!(dir.path, Path::new(path)); assert!((dir.rank - 2.0).abs() < 0.01); assert_eq!(dir.last_accessed, now); } diff --git a/src/db/stream.rs b/src/db/stream.rs index 44b59d97..bf9e45dd 100644 --- a/src/db/stream.rs +++ b/src/db/stream.rs @@ -1,5 +1,6 @@ use std::iter::Rev; use std::ops::Range; +use std::path::Path; use std::{fs, path}; use crate::db::{Database, Dir, Epoch}; @@ -71,7 +72,13 @@ impl<'a> Stream<'a> { continue; } - if Some(dir.path.as_ref()) == self.exclude_path.as_deref() { + if self + .exclude_path + .as_deref() + .map(|a| dir.path.to_str().map(|x| x == a)) + .flatten() + .unwrap_or_default() + { self.did_exclude = true; continue; } @@ -87,7 +94,7 @@ impl<'a> Stream<'a> { self.did_exclude } - fn matches_exists(&self, path: &str) -> bool { + fn matches_exists(&self, path: &Path) -> bool { if !self.check_exists { return true; } @@ -95,12 +102,14 @@ impl<'a> Stream<'a> { resolver(path).map(|m| m.is_dir()).unwrap_or_default() } - fn matches_keywords(&self, path: &str) -> bool { + fn matches_keywords(&self, path: &Path) -> bool { let (keywords_last, keywords) = match self.keywords.split_last() { Some(split) => split, None => return true, }; - + let Some(path) = path.to_str() else { + return false; + }; let path = util::to_lowercase(path); let mut path = path.as_str(); match path.rfind(keywords_last) { @@ -155,6 +164,6 @@ mod tests { fn query(#[case] keywords: &[&str], #[case] path: &str, #[case] is_match: bool) { let db = &mut Database::new(PathBuf::new(), Vec::new(), |_| Vec::new(), false); let stream = Stream::new(db, 0).with_keywords(keywords); - assert_eq!(is_match, stream.matches_keywords(path)); + assert_eq!(is_match, stream.matches_keywords(Path::new(path))); } }