Address review comments

Cargo.lock

@@ -254,6 +254,16 @@ version = "1.0.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "453440c271cf5577fd2a40e4942540cb7d0d2f85e27c8d07dd0023c925a67541"
 
+[[package]]
+name = "edit"
+version = "0.1.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c562aa71f7bc691fde4c6bf5f93ae5a5298b617c2eb44c76c87832299a17fbb4"
+dependencies = [
+ "tempfile",
+ "which",
+]
+
 [[package]]
 name = "either"
 version = "1.7.0"
@@ -881,6 +891,7 @@ dependencies = [
  "dialoguer",
  "dirs",
  "dunce",
+ "edit",
  "fastrand",
  "glob",
  "nix",

Cargo.toml

@@ -26,10 +26,10 @@ clap = { version = "3.1.0", features = ["derive"] }
 dialoguer = "0.10.2"
 dirs = "4.0.0"
 dunce = "1.0.1"
+edit = "0.1.4"
 fastrand = "1.7.0"
 glob = "0.3.0"
 serde = { version = "1.0.116", features = ["derive"] }
-tempfile = "3.1.0"
 
 [target.'cfg(unix)'.dependencies]
 nix = { version = "0.24.1", default-features = false, features = [
@@ -49,6 +49,8 @@ clap_complete_fig = "3.1.0"
 assert_cmd = "2.0.0"
 rstest = { version = "0.15.0", default-features = false }
 rstest_reuse = "0.4.0"
+tempfile = "3.1.0"
+
 
 [features]
 default = []

src/cmd/add.rs

@@ -31,7 +31,7 @@ impl Run for Add {
             if !Path::new(path).is_dir() {
                 bail!("not a directory: {path}");
             }
-            db.add(path, now);
+            db.add(path, now, 1.0);
         }
 
         if db.modified {

src/cmd/edit.rs

@@ -1,14 +1,13 @@
 use crate::cmd::{Edit, Run};
-use crate::db::{db_path, Database, DatabaseFile, Epoch, Rank};
+use crate::db::{Database, DatabaseFile, Epoch, Rank};
-use crate::util::{rename, resolve_path};
+use crate::util::resolve_path;
 use crate::{config, util};
 use anyhow::Result;
 use std::fmt::Write as FmtWrite;
 use std::path::PathBuf;
 
-use core::mem;
+use dialoguer::Input;
-use dialoguer::{Editor, Input};
+use edit::edit;
-use tempfile::tempdir;
 
 const HEADER: &str = "\
 # Blank lines and lines prepended with '#' are ignored; Line order is insignificant
@@ -23,18 +22,16 @@ enum ValidationResult {
 
 impl Run for Edit {
     fn run(&self) -> Result<()> {
-        let temp_dir = tempdir()?;
-        let temp_dir_path = temp_dir.path();
         while let Some(db_edits) = get_db_edits()? {
-            let mut db_file = DatabaseFile::new(temp_dir_path);
+            let data_dir = config::data_dir()?;
+            let mut db_file = DatabaseFile::new(data_dir);
             let mut db = db_file.open()?;
-            let result = validate_db(&mut db, db_edits);
+            db.clear();
+            let problems = get_problems(&mut db, db_edits);
+            let result = handle_problems(problems);
             match result {
                 ValidationResult::Success => {
                     db.save()?;
-                    mem::drop(db);
-                    mem::drop(db_file);
-                    rename(db_path(temp_dir_path), db_path(config::data_dir()?))?;
                     return Ok(());
                 }
                 ValidationResult::Exit => break,
@@ -55,7 +52,13 @@ fn get_db_edits() -> Result<Option<String>> {
     while let Some(dir) = stream.next() {
         writeln!(&mut to_edit, "{},{},{}", dir.last_accessed, dir.rank, dir.path)?;
     }
-    Ok(Editor::new().edit(&to_edit)?)
+    let edit_result = edit(&to_edit)?;
+    if edit_result == to_edit {
+        // The file was not changed at all so we don't want to attempt to overwrite the original
+        Ok(None)
+    } else {
+        Ok(Some(edit_result))
+    }
 }
 
 fn validate_db(db: &mut Database, db_edits: String) -> ValidationResult {
@@ -133,7 +136,7 @@ fn validate_db(db: &mut Database, db_edits: String) -> ValidationResult {
         };
 
         if let (Some(path), Some(last_accessed), Some(rank)) = (path, last_accessed, rank) {
-            db.add_raw(&path, last_accessed, rank);
+            db.add(&path, last_accessed, rank);
         }
     }
     let has_warnings = !warnings.is_empty();

src/db/mod.rs

@@ -31,11 +31,7 @@ impl<'file> Database<'file> {
     }
 
     /// Adds a new directory or increments its rank. Also updates its last accessed time.
-    pub fn add<S: AsRef<str>>(&mut self, path: S, now: Epoch) {
+    pub fn add<S: AsRef<str>>(&mut self, path: S, last_accessed: Epoch, rank: Rank) {
-        self.add_raw(path, now, 1.0)
-    }
-
-    pub fn add_raw<S: AsRef<str>>(&mut self, path: S, last_accessed: Epoch, rank: Rank) {
         let path = path.as_ref();
 
         match self.dirs.iter_mut().find(|dir| dir.path == path) {
@@ -95,6 +91,11 @@ impl<'file> Database<'file> {
         false
     }
 
+    /// Removes all entries from the store.
+    pub fn clear(&mut self) {
+        self.dirs = DirList::new();
+    }
+
     pub fn age(&mut self, max_age: Rank) {
         let sum_age = self.dirs.iter().map(|dir| dir.rank).sum::<Rank>();
         if sum_age > max_age {
@@ -162,8 +163,8 @@ mod tests {
         {
             let mut db = DatabaseFile::new(data_dir.path());
             let mut db = db.open().unwrap();
-            db.add(path, now);
+            db.add(path, now, 1.0);
-            db.add(path, now);
+            db.add(path, now, 1.0);
             db.save().unwrap();
         }
         {
@@ -186,7 +187,7 @@ mod tests {
         {
             let mut db = DatabaseFile::new(data_dir.path());
             let mut db = db.open().unwrap();
-            db.add(path, now);
+            db.add(path, now, 1.0);
             db.save().unwrap();
         }
         {

src/util.rs

@@ -160,7 +160,7 @@ fn tmpfile<P: AsRef<Path>>(dir: P) -> Result<(File, PathBuf)> {
 }
 
 /// Similar to [`fs::rename`], but retries on Windows.
-pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> Result<()> {
+fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> Result<()> {
     const MAX_ATTEMPTS: usize = 5;
     let from = from.as_ref();
     let to = to.as_ref();