delete from file on file-backed history

Author: samlich

src/history/file_backed.rs

@@ -198,26 +198,93 @@ impl History for FileBackedHistory {
                 "Invalid ID (not usize)",
             ))
         })?;
-        if self.len_on_disk <= id {
-            self.entries.remove(id);
-            Ok(())
-        } else {
-            // Since no ID is written to disk, it's not possible to delete them.
-            // E.g. consider another instance having deleted entries, after this instance
-            // loaded the file.
-            Err(ReedlineError(
-                ReedlineErrorVariants::HistoryFeatureUnsupported {
-                    history: "FileBackedHistory",
-                    feature: "removing entries",
-                },
-            ))
+
+        let delete = self.entries[id].clone();
+        let mut i = 0;
+        while i < self.entries.len() {
+            if self.entries[i] == delete {
+                self.entries.remove(i);
+                if i < self.len_on_disk {
+                    self.len_on_disk -= 1;
+                }
+            } else {
+                i += 1;
+            }
         }
+
+        self.sync_and_delete_item(Some(&delete))
+            .map_err(|e| ReedlineError(ReedlineErrorVariants::IOError(e)))
     }
 
     /// Writes unwritten history contents to disk.
     ///
     /// If file would exceed `capacity` truncates the oldest entries.
     fn sync(&mut self) -> std::io::Result<()> {
+        self.sync_and_delete_item(None)
+    }
+
+    fn session(&self) -> Option<HistorySessionId> {
+        self.session
+    }
+}
+
+impl FileBackedHistory {
+    /// Creates a new in-memory history that remembers `n <= capacity` elements
+    ///
+    /// # Panics
+    ///
+    /// If `capacity == usize::MAX`
+    pub fn new(capacity: usize) -> Self {
+        if capacity == usize::MAX {
+            panic!("History capacity too large to be addressed safely");
+        }
+        FileBackedHistory {
+            capacity,
+            entries: VecDeque::new(),
+            file: None,
+            len_on_disk: 0,
+            session: None,
+        }
+    }
+
+    /// Creates a new history with an associated history file.
+    ///
+    /// History file format: commands separated by new lines.
+    /// If file exists file will be read otherwise empty file will be created.
+    ///
+    ///
+    /// **Side effects:** creates all nested directories to the file
+    ///
+    pub fn with_file(capacity: usize, file: PathBuf) -> std::io::Result<Self> {
+        let mut hist = Self::new(capacity);
+        if let Some(base_dir) = file.parent() {
+            std::fs::create_dir_all(base_dir)?;
+        }
+        hist.file = Some(file);
+        hist.sync()?;
+        Ok(hist)
+    }
+
+    // this history doesn't store any info except command line
+    fn construct_entry(id: Option<HistoryItemId>, command_line: String) -> HistoryItem {
+        HistoryItem {
+            id,
+            start_timestamp: None,
+            command_line,
+            session_id: None,
+            hostname: None,
+            cwd: None,
+            duration: None,
+            exit_status: None,
+            more_info: None,
+        }
+    }
+
+    /// Writes unwritten history contents to disk, and optionally deletes
+    /// all occurences of the provided item.
+    ///
+    /// If file would exceed `capacity` truncates the oldest entries.
+    fn sync_and_delete_item(&mut self, delete: Option<&str>) -> std::io::Result<()> {
         if let Some(fname) = &self.file {
             // The unwritten entries
             let own_entries = self.entries.range(self.len_on_disk..);
@@ -236,17 +303,26 @@ impl History for FileBackedHistory {
             let mut writer_guard = f_lock.write()?;
             let (mut foreign_entries, truncate) = {
                 let reader = BufReader::new(writer_guard.deref());
+                let mut deletions = false;
                 let mut from_file = reader
                     .lines()
                     .map(|o| o.map(|i| decode_entry(&i)))
+                    .filter(|x| {
+                        if x.as_deref().ok() == delete {
+                            deletions = true;
+                            false
+                        } else {
+                            true
+                        }
+                    })
                     .collect::<std::io::Result<VecDeque<_>>>()?;
                 if from_file.len() + own_entries.len() > self.capacity {
                     (
                         from_file.split_off(from_file.len() - (self.capacity - own_entries.len())),
                         true,
                     )
                 } else {
-                    (from_file, false)
+                    (from_file, deletions)
                 }
             };
 
@@ -282,63 +358,6 @@ impl History for FileBackedHistory {
         }
         Ok(())
     }
-
-    fn session(&self) -> Option<HistorySessionId> {
-        self.session
-    }
-}
-
-impl FileBackedHistory {
-    /// Creates a new in-memory history that remembers `n <= capacity` elements
-    ///
-    /// # Panics
-    ///
-    /// If `capacity == usize::MAX`
-    pub fn new(capacity: usize) -> Self {
-        if capacity == usize::MAX {
-            panic!("History capacity too large to be addressed safely");
-        }
-        FileBackedHistory {
-            capacity,
-            entries: VecDeque::new(),
-            file: None,
-            len_on_disk: 0,
-            session: None,
-        }
-    }
-
-    /// Creates a new history with an associated history file.
-    ///
-    /// History file format: commands separated by new lines.
-    /// If file exists file will be read otherwise empty file will be created.
-    ///
-    ///
-    /// **Side effects:** creates all nested directories to the file
-    ///
-    pub fn with_file(capacity: usize, file: PathBuf) -> std::io::Result<Self> {
-        let mut hist = Self::new(capacity);
-        if let Some(base_dir) = file.parent() {
-            std::fs::create_dir_all(base_dir)?;
-        }
-        hist.file = Some(file);
-        hist.sync()?;
-        Ok(hist)
-    }
-
-    // this history doesn't store any info except command line
-    fn construct_entry(id: Option<HistoryItemId>, command_line: String) -> HistoryItem {
-        HistoryItem {
-            id,
-            start_timestamp: None,
-            command_line,
-            session_id: None,
-            hostname: None,
-            cwd: None,
-            duration: None,
-            exit_status: None,
-            more_info: None,
-        }
-    }
 }
 
 impl Drop for FileBackedHistory {