Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: path consistency & some code improvements #569

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/cmd/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
5 changes: 3 additions & 2 deletions src/db/dir.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::borrow::Cow;
use std::fmt::{self, Display, Formatter};
use std::path::Path;

use serde::{Deserialize, Serialize};

Expand All @@ -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,
}
Expand Down Expand Up @@ -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())
}
}

Expand Down
16 changes: 8 additions & 8 deletions src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<str> + Into<String>, by: Rank, now: Epoch) {
pub fn add(&mut self, path: impl AsRef<Path>, 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);
Expand All @@ -77,31 +77,31 @@ 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<str> + Into<String>, rank: Rank, now: Epoch) {
pub fn add_unchecked(&mut self, path: impl AsRef<Path>, 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<str> + Into<String>, by: Rank, now: Epoch) {
pub fn add_update(&mut self, path: impl AsRef<Path>, 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);
}

/// 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<str>) -> bool {
pub fn remove(&mut self, path: impl AsRef<Path>) -> bool {
match self.dirs().iter().position(|dir| dir.path == path.as_ref()) {
Some(idx) => {
self.swap_remove(idx);
Expand Down Expand Up @@ -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);
}
Expand Down
19 changes: 14 additions & 5 deletions src/db/stream.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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;
}
Expand All @@ -87,20 +94,22 @@ 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;
}
let resolver = if self.resolve_symlinks { fs::symlink_metadata } else { fs::metadata };
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) {
Expand Down Expand Up @@ -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)));
}
}