From bd1c1787e8c0de5b6b582eefe53f684479c3edcd Mon Sep 17 00:00:00 2001 From: Ajeet D'Souza <98ajeet@gmail.com> Date: Fri, 9 Dec 2022 12:38:49 +0530 Subject: [PATCH] Move more commands to new DB impl --- contrib/completions/zoxide.ts | 4 ++ src/cmd/add.rs | 14 ++-- src/cmd/cmd.rs | 4 ++ src/cmd/edit.rs | 18 ++--- src/cmd/import.rs | 109 ++++++++++++++--------------- src/cmd/remove.rs | 9 +-- src/db/mod.rs | 125 ---------------------------------- src/store/mod.rs | 87 +++++++++++++++++++++-- 8 files changed, 156 insertions(+), 214 deletions(-) diff --git a/contrib/completions/zoxide.ts b/contrib/completions/zoxide.ts index b188b93..624d3c2 100644 --- a/contrib/completions/zoxide.ts +++ b/contrib/completions/zoxide.ts @@ -27,6 +27,7 @@ const completion: Fig.Spec = { subcommands: [ { name: "decrement", + hidden: true, options: [ { name: ["-h", "--help"], @@ -43,6 +44,7 @@ const completion: Fig.Spec = { }, { name: "delete", + hidden: true, options: [ { name: ["-h", "--help"], @@ -59,6 +61,7 @@ const completion: Fig.Spec = { }, { name: "increment", + hidden: true, options: [ { name: ["-h", "--help"], @@ -75,6 +78,7 @@ const completion: Fig.Spec = { }, { name: "reload", + hidden: true, options: [ { name: ["-h", "--help"], diff --git a/src/cmd/add.rs b/src/cmd/add.rs index 34ba0c0..48a448f 100644 --- a/src/cmd/add.rs +++ b/src/cmd/add.rs @@ -3,7 +3,7 @@ use std::path::Path; use anyhow::{bail, Result}; use crate::cmd::{Add, Run}; -use crate::db::DatabaseFile; +use crate::store::Store; use crate::{config, util}; impl Run for Add { @@ -12,13 +12,11 @@ impl Run for Add { // when writing to fzf / stdout. const EXCLUDE_CHARS: &[char] = &['\n', '\r']; - let data_dir = config::data_dir()?; let exclude_dirs = config::exclude_dirs()?; let max_age = config::maxage()?; let now = util::current_time()?; - let mut db = DatabaseFile::new(data_dir); - let mut db = db.open()?; + let mut db = Store::open()?; for path in &self.paths { let path = if config::resolve_symlinks() { util::canonicalize } else { util::resolve_path }(path)?; @@ -31,14 +29,12 @@ impl Run for Add { if !Path::new(path).is_dir() { bail!("not a directory: {path}"); } - db.add(path, now); + db.add_update(path, 1.0, now); } - if db.modified { + if db.dirty() { db.age(max_age); - db.save()?; } - - Ok(()) + db.save() } } diff --git a/src/cmd/cmd.rs b/src/cmd/cmd.rs index 9a17633..1f19a80 100644 --- a/src/cmd/cmd.rs +++ b/src/cmd/cmd.rs @@ -47,9 +47,13 @@ pub struct Edit { #[derive(Clone, Debug, Subcommand)] pub enum EditCommand { + #[clap(hide = true)] Decrement { path: String }, + #[clap(hide = true)] Delete { path: String }, + #[clap(hide = true)] Increment { path: String }, + #[clap(hide = true)] Reload, } diff --git a/src/cmd/edit.rs b/src/cmd/edit.rs index 7a1e54f..ca8714a 100644 --- a/src/cmd/edit.rs +++ b/src/cmd/edit.rs @@ -14,7 +14,7 @@ impl Run for Edit { match &self.cmd { Some(EditCommand::Decrement { path }) => { - db.increment(path, -1.0, now); + db.add(path, -1.0, now); db.save()?; print_dirs(db, now); } @@ -24,7 +24,7 @@ impl Run for Edit { print_dirs(db, now); } Some(EditCommand::Increment { path }) => { - db.increment(path, 1.0, now); + db.add(path, 1.0, now); db.save()?; print_dirs(db, now); } @@ -42,13 +42,13 @@ impl Run for Edit { "--no-sort", // Interface "--bind=\ - ctrl-r:reload(zoxide edit reload),\ - ctrl-w:reload(zoxide edit delete {2..}),\ - ctrl-a:reload(zoxide edit increment {2..}),\ - ctrl-d:reload(zoxide edit decrement {2..}),\ - ctrl-z:ignore,\ - double-click:ignore,\ - enter:abort", +ctrl-r:reload(zoxide edit reload),\ +ctrl-w:reload(zoxide edit delete {2..}),\ +ctrl-a:reload(zoxide edit increment {2..}),\ +ctrl-d:reload(zoxide edit decrement {2..}),\ +ctrl-z:ignore,\ +double-click:ignore,\ +enter:abort", "--cycle", "--keep-right", // Layout diff --git a/src/cmd/import.rs b/src/cmd/import.rs index b5a2fd2..8771d5f 100644 --- a/src/cmd/import.rs +++ b/src/cmd/import.rs @@ -3,24 +3,21 @@ use std::fs; use anyhow::{bail, Context, Result}; use crate::cmd::{Import, ImportFrom, Run}; -use crate::config; -use crate::db::{Database, DatabaseFile, Dir}; +use crate::store::Store; impl Run for Import { fn run(&self) -> Result<()> { let buffer = fs::read_to_string(&self.path) .with_context(|| format!("could not open database for importing: {}", &self.path.display()))?; - let data_dir = config::data_dir()?; - let mut db = DatabaseFile::new(data_dir); - let db = &mut db.open()?; - if !self.merge && !db.dirs.is_empty() { + let mut db = Store::open()?; + if !self.merge && !db.dirs().is_empty() { bail!("current database is not empty, specify --merge to continue anyway"); } match self.from { - ImportFrom::Autojump => from_autojump(db, &buffer), - ImportFrom::Z => from_z(db, &buffer), + ImportFrom::Autojump => import_autojump(&mut db, &buffer), + ImportFrom::Z => import_z(&mut db, &buffer), } .context("import error")?; @@ -28,7 +25,7 @@ impl Run for Import { } } -fn from_autojump<'a>(db: &mut Database<'a>, buffer: &'a str) -> Result<()> { +fn import_autojump(db: &mut Store, buffer: &str) -> Result<()> { for line in buffer.lines() { if line.is_empty() { continue; @@ -43,18 +40,16 @@ fn from_autojump<'a>(db: &mut Database<'a>, buffer: &'a str) -> Result<()> { let path = split.next().with_context(|| format!("invalid entry: {line}"))?; - db.dirs.push(Dir { path: path.into(), rank, last_accessed: 0 }); - db.modified = true; + db.add_unchecked(path, rank, 0); } - if db.modified { + if db.dirty() { db.dedup(); } - Ok(()) } -fn from_z<'a>(db: &mut Database<'a>, buffer: &'a str) -> Result<()> { +fn import_z(db: &mut Store, buffer: &str) -> Result<()> { for line in buffer.lines() { if line.is_empty() { continue; @@ -69,14 +64,12 @@ fn from_z<'a>(db: &mut Database<'a>, buffer: &'a str) -> Result<()> { let path = split.next().with_context(|| format!("invalid entry: {line}"))?; - db.dirs.push(Dir { path: path.into(), rank, last_accessed }); - db.modified = true; + db.add_unchecked(path, rank, last_accessed); } - if db.modified { + if db.dirty() { db.dedup(); } - Ok(()) } @@ -86,33 +79,33 @@ fn sigmoid(x: f64) -> f64 { #[cfg(test)] mod tests { - use super::sigmoid; - use crate::db::{Database, Dir}; + use super::*; + use crate::store::Dir; #[test] fn from_autojump() { - let buffer = r#" + let data_dir = tempfile::tempdir().unwrap(); + let mut db = Store::open_dir(data_dir.path()).unwrap(); + for (path, rank, last_accessed) in [ + ("/quux/quuz", 1.0, 100), + ("/corge/grault/garply", 6.0, 600), + ("/waldo/fred/plugh", 3.0, 300), + ("/xyzzy/thud", 8.0, 800), + ("/foo/bar", 9.0, 900), + ] { + db.add_unchecked(path, rank, last_accessed); + } + + let buffer = "\ 7.0 /baz 2.0 /foo/bar -5.0 /quux/quuz -"#; +5.0 /quux/quuz"; + import_autojump(&mut db, buffer).unwrap(); - let dirs = vec![ - Dir { path: "/quux/quuz".into(), rank: 1.0, last_accessed: 100 }, - Dir { path: "/corge/grault/garply".into(), rank: 6.0, last_accessed: 600 }, - Dir { path: "/waldo/fred/plugh".into(), rank: 3.0, last_accessed: 300 }, - Dir { path: "/xyzzy/thud".into(), rank: 8.0, last_accessed: 800 }, - Dir { path: "/foo/bar".into(), rank: 9.0, last_accessed: 900 }, - ]; - let data_dir = tempfile::tempdir().unwrap(); - let data_dir = &data_dir.path().to_path_buf(); - let mut db = Database { dirs: dirs.into(), modified: false, data_dir }; + db.sort_by_path(); + println!("got: {:?}", &db.dirs()); - super::from_autojump(&mut db, buffer).unwrap(); - db.dirs.sort_by(|dir1, dir2| dir1.path.cmp(&dir2.path)); - println!("got: {:?}", &db.dirs.as_slice()); - - let exp = &[ + let exp = [ Dir { path: "/baz".into(), rank: sigmoid(7.0), last_accessed: 0 }, Dir { path: "/corge/grault/garply".into(), rank: 6.0, last_accessed: 600 }, Dir { path: "/foo/bar".into(), rank: 9.0 + sigmoid(2.0), last_accessed: 900 }, @@ -122,7 +115,7 @@ mod tests { ]; println!("exp: {exp:?}"); - for (dir1, dir2) in db.dirs.iter().zip(exp) { + for (dir1, dir2) in db.dirs().iter().zip(exp) { assert_eq!(dir1.path, dir2.path); assert!((dir1.rank - dir2.rank).abs() < 0.01); assert_eq!(dir1.last_accessed, dir2.last_accessed); @@ -131,29 +124,29 @@ mod tests { #[test] fn from_z() { - let buffer = r#" + let data_dir = tempfile::tempdir().unwrap(); + let mut db = Store::open_dir(data_dir.path()).unwrap(); + for (path, rank, last_accessed) in [ + ("/quux/quuz", 1.0, 100), + ("/corge/grault/garply", 6.0, 600), + ("/waldo/fred/plugh", 3.0, 300), + ("/xyzzy/thud", 8.0, 800), + ("/foo/bar", 9.0, 900), + ] { + db.add_unchecked(path, rank, last_accessed); + } + + let buffer = "\ /baz|7|700 /quux/quuz|4|400 /foo/bar|2|200 -/quux/quuz|5|500 -"#; +/quux/quuz|5|500"; + import_z(&mut db, buffer).unwrap(); - let dirs = vec![ - Dir { path: "/quux/quuz".into(), rank: 1.0, last_accessed: 100 }, - Dir { path: "/corge/grault/garply".into(), rank: 6.0, last_accessed: 600 }, - Dir { path: "/waldo/fred/plugh".into(), rank: 3.0, last_accessed: 300 }, - Dir { path: "/xyzzy/thud".into(), rank: 8.0, last_accessed: 800 }, - Dir { path: "/foo/bar".into(), rank: 9.0, last_accessed: 900 }, - ]; - let data_dir = tempfile::tempdir().unwrap(); - let data_dir = &data_dir.path().to_path_buf(); - let mut db = Database { dirs: dirs.into(), modified: false, data_dir }; + db.sort_by_path(); + println!("got: {:?}", &db.dirs()); - super::from_z(&mut db, buffer).unwrap(); - db.dirs.sort_by(|dir1, dir2| dir1.path.cmp(&dir2.path)); - println!("got: {:?}", &db.dirs.as_slice()); - - let exp = &[ + let exp = [ Dir { path: "/baz".into(), rank: 7.0, last_accessed: 700 }, Dir { path: "/corge/grault/garply".into(), rank: 6.0, last_accessed: 600 }, Dir { path: "/foo/bar".into(), rank: 11.0, last_accessed: 900 }, @@ -163,7 +156,7 @@ mod tests { ]; println!("exp: {exp:?}"); - for (dir1, dir2) in db.dirs.iter().zip(exp) { + for (dir1, dir2) in db.dirs().iter().zip(exp) { assert_eq!(dir1.path, dir2.path); assert!((dir1.rank - dir2.rank).abs() < 0.01); assert_eq!(dir1.last_accessed, dir2.last_accessed); diff --git a/src/cmd/remove.rs b/src/cmd/remove.rs index abdb1e6..9a5f474 100644 --- a/src/cmd/remove.rs +++ b/src/cmd/remove.rs @@ -1,21 +1,18 @@ use anyhow::{bail, Result}; use crate::cmd::{Remove, Run}; -use crate::db::DatabaseFile; -use crate::{config, util}; +use crate::store::Store; +use crate::util; impl Run for Remove { fn run(&self) -> Result<()> { - let data_dir = config::data_dir()?; - let mut db = DatabaseFile::new(data_dir); - let mut db = db.open()?; + let mut db = Store::open()?; for path in &self.paths { if !db.remove(path) { let path_abs = util::resolve_path(path)?; let path_abs = util::path_to_str(&path_abs)?; if path_abs == path || !db.remove(path_abs) { - db.modified = false; bail!("path not found in database: {path}") } } diff --git a/src/db/mod.rs b/src/db/mod.rs index 41db369..aa3eae4 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -30,79 +30,10 @@ impl<'file> Database<'file> { Ok(()) } - /// Adds a new directory or increments its rank. Also updates its last accessed time. - pub fn add>(&mut self, path: S, now: Epoch) { - let path = path.as_ref(); - - match self.dirs.iter_mut().find(|dir| dir.path == path) { - Some(dir) => { - dir.last_accessed = now; - dir.rank += 1.0; - } - None => self.dirs.push(Dir { path: path.to_string().into(), last_accessed: now, rank: 1.0 }), - }; - - self.modified = true; - } - - pub fn dedup(&mut self) { - // Sort by path, so that equal paths are next to each other. - self.dirs.sort_by(|dir1, dir2| dir1.path.cmp(&dir2.path)); - - for idx in (1..self.dirs.len()).rev() { - // Check if curr_dir and next_dir have equal paths. - let curr_dir = &self.dirs[idx]; - let next_dir = &self.dirs[idx - 1]; - if next_dir.path != curr_dir.path { - continue; - } - - // Merge curr_dir's rank and last_accessed into next_dir. - let rank = curr_dir.rank; - let last_accessed = curr_dir.last_accessed; - let next_dir = &mut self.dirs[idx - 1]; - next_dir.last_accessed = next_dir.last_accessed.max(last_accessed); - next_dir.rank += rank; - - // Delete curr_dir. - self.dirs.swap_remove(idx); - self.modified = true; - } - } - // Streaming iterator for directories. pub fn stream(&mut self, now: Epoch) -> Stream<'_, 'file> { Stream::new(self, now) } - - /// Removes the directory with `path` from the store. This does not preserve ordering, but is - /// O(1). - pub fn remove>(&mut self, path: S) -> bool { - let path = path.as_ref(); - - if let Some(idx) = self.dirs.iter().position(|dir| dir.path == path) { - self.dirs.swap_remove(idx); - self.modified = true; - return true; - } - - false - } - - pub fn age(&mut self, max_age: Rank) { - let sum_age = self.dirs.iter().map(|dir| dir.rank).sum::(); - if sum_age > max_age { - let factor = 0.9 * max_age / sum_age; - for idx in (0..self.dirs.len()).rev() { - let dir = &mut self.dirs[idx]; - dir.rank *= factor; - if dir.rank < 1.0 { - self.dirs.swap_remove(idx); - } - } - self.modified = true; - } - } } pub struct DatabaseFile { @@ -142,59 +73,3 @@ fn db_path>(data_dir: P) -> PathBuf { const DB_FILENAME: &str = "db.zo"; data_dir.as_ref().join(DB_FILENAME) } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn add() { - let path = if cfg!(windows) { r"C:\foo\bar" } else { "/foo/bar" }; - let now = 946684800; - - let data_dir = tempfile::tempdir().unwrap(); - { - let mut db = DatabaseFile::new(data_dir.path()); - let mut db = db.open().unwrap(); - db.add(path, now); - db.add(path, now); - db.save().unwrap(); - } - { - let mut db = DatabaseFile::new(data_dir.path()); - let db = db.open().unwrap(); - assert_eq!(db.dirs.len(), 1); - - let dir = &db.dirs[0]; - assert_eq!(dir.path, path); - assert_eq!(dir.last_accessed, now); - } - } - - #[test] - fn remove() { - let path = if cfg!(windows) { r"C:\foo\bar" } else { "/foo/bar" }; - let now = 946684800; - - let data_dir = tempfile::tempdir().unwrap(); - { - let mut db = DatabaseFile::new(data_dir.path()); - let mut db = db.open().unwrap(); - db.add(path, now); - db.save().unwrap(); - } - { - let mut db = DatabaseFile::new(data_dir.path()); - let mut db = db.open().unwrap(); - assert!(db.remove(path)); - db.save().unwrap(); - } - { - let mut db = DatabaseFile::new(data_dir.path()); - let mut db = db.open().unwrap(); - assert!(db.dirs.is_empty()); - assert!(!db.remove(path)); - db.save().unwrap(); - } - } -} diff --git a/src/store/mod.rs b/src/store/mod.rs index da8b32b..0152950 100644 --- a/src/store/mod.rs +++ b/src/store/mod.rs @@ -1,5 +1,5 @@ use std::borrow::Cow; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::{fs, io}; use anyhow::{bail, Context, Result}; @@ -24,6 +24,10 @@ impl Store { pub fn open() -> Result { let data_dir = config::data_dir()?; + Self::open_dir(&data_dir) + } + + pub fn open_dir(data_dir: &Path) -> Result { let path = data_dir.join("db.zo"); match fs::read(&path) { @@ -31,9 +35,9 @@ impl Store { Err(e) if e.kind() == io::ErrorKind::NotFound => { // Create data directory, but don't create any file yet. The file will be created // later by [`Database::save`] if any data is modified. - fs::create_dir_all(&data_dir) + fs::create_dir_all(data_dir) .with_context(|| format!("unable to create data directory: {}", data_dir.display()))?; - Ok(Self::new(data_dir, Vec::new(), |_| Vec::new(), false)) + Ok(Self::new(path, Vec::new(), |_| Vec::new(), false)) } Err(e) => Err(e).with_context(|| format!("could not read from database: {}", path.display())), } @@ -41,11 +45,11 @@ impl Store { pub fn save(&mut self) -> Result<()> { // Only write to disk if the database is modified. - if !self.borrow_dirty() { + if !self.dirty() { return Ok(()); } - let bytes = Self::serialize(self.borrow_dirs())?; + let bytes = Self::serialize(self.dirs())?; util::write(self.borrow_path(), &bytes).context("could not write to database")?; self.with_dirty_mut(|dirty| *dirty = false); @@ -53,7 +57,7 @@ impl Store { } /// Increments the rank of a directory, or creates it if it does not exist. - pub fn increment(&mut self, path: impl AsRef + Into, by: Rank, now: Epoch) { + pub fn add(&mut self, path: impl AsRef + Into, 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 }), @@ -61,9 +65,17 @@ impl Store { self.with_dirty_mut(|dirty| *dirty = true); } + /// 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) { + self.with_dirs_mut(|dirs| dirs.push(Dir { path: path.into().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 increment_update(&mut self, path: impl AsRef + Into, by: Rank, now: Epoch) { + pub fn add_update(&mut self, path: impl AsRef + Into, 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); @@ -74,6 +86,8 @@ impl Store { 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) -> bool { let deleted = self.with_dirs_mut(|dirs| match dirs.iter().position(|dir| dir.path == path.as_ref()) { Some(idx) => { @@ -146,6 +160,10 @@ impl Store { self.with_dirty_mut(|dirty| *dirty = true); } + pub fn dirty(&self) -> bool { + *self.borrow_dirty() + } + pub fn dirs(&self) -> &[Dir] { self.borrow_dirs() } @@ -221,3 +239,58 @@ impl Dir<'_> { pub type Rank = f64; pub type Epoch = u64; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn add() { + let data_dir = tempfile::tempdir().unwrap(); + let path = if cfg!(windows) { r"C:\foo\bar" } else { "/foo/bar" }; + let now = 946684800; + + { + let mut db = Store::open_dir(data_dir.path()).unwrap(); + db.add(path, 1.0, now); + db.add(path, 1.0, now); + db.save().unwrap(); + } + + { + let db = Store::open_dir(data_dir.path()).unwrap(); + assert_eq!(db.dirs().len(), 1); + + let dir = &db.dirs()[0]; + assert_eq!(dir.path, path); + assert!((dir.rank - 2.0).abs() < 0.01); + assert_eq!(dir.last_accessed, now); + } + } + + #[test] + fn remove() { + let data_dir = tempfile::tempdir().unwrap(); + let path = if cfg!(windows) { r"C:\foo\bar" } else { "/foo/bar" }; + let now = 946684800; + + { + let mut db = Store::open_dir(data_dir.path()).unwrap(); + db.add(path, 1.0, now); + db.save().unwrap(); + } + + { + let mut db = Store::open_dir(data_dir.path()).unwrap(); + assert!(db.remove(path)); + db.save().unwrap(); + } + + { + let mut db = Store::open_dir(data_dir.path()).unwrap(); + assert!(db.dirs().is_empty()); + assert!(!db.remove(path)); + db.save().unwrap(); + } + } +}