Improve history API

Cargo.toml

@@ -23,6 +23,10 @@ crossterm = { version = "0.27.0", features = ["serde"] }
 fd-lock = "3.0.3"
 itertools = "0.10.3"
 nu-ansi-term = "0.49.0"
+rand = { version = "0.8.5", default-features = false, features = [
+    "small_rng",
+    "getrandom",
+] }
 rusqlite = { version = "0.29.0", optional = true }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = { version = "1.0.79", optional = true }

examples/cwd_aware_hinter.rs

@@ -6,14 +6,21 @@
 // pressing "a" hints to abc.
 // Up/Down or Ctrl p/n, to select next/previous match
 
-use std::io;
+use std::{
+    io,
+    sync::atomic::{AtomicI64, Ordering},
+};
+
+use reedline::HistoryItemId;
+
+static COUNTER: AtomicI64 = AtomicI64::new(0);
 
 fn create_item(cwd: &str, cmd: &str, exit_status: i64) -> reedline::HistoryItem {
     use std::time::Duration;
 
     use reedline::HistoryItem;
     HistoryItem {
-        id: None,
+        id: HistoryItemId(COUNTER.fetch_add(1, Ordering::SeqCst)),
         start_timestamp: None,
         command_line: cmd.to_string(),
         session_id: None,
@@ -32,13 +39,13 @@ fn create_filled_example_history(home_dir: &str, orig_dir: &str) -> Box<dyn reed
     #[cfg(any(feature = "sqlite", feature = "sqlite-dynlib"))]
     let mut history = Box::new(reedline::SqliteBackedHistory::in_memory().unwrap());
 
-    history.save(create_item(orig_dir, "dummy", 0)).unwrap(); // add dummy item so ids start with 1
-    history.save(create_item(orig_dir, "ls /usr", 0)).unwrap();
-    history.save(create_item(orig_dir, "pwd", 0)).unwrap();
+    history.save(&create_item(orig_dir, "dummy", 0)).unwrap(); // add dummy item so ids start with 1
+    history.save(&create_item(orig_dir, "ls /usr", 0)).unwrap();
+    history.save(&create_item(orig_dir, "pwd", 0)).unwrap();
 
-    history.save(create_item(home_dir, "cat foo", 0)).unwrap();
-    history.save(create_item(home_dir, "ls bar", 0)).unwrap();
-    history.save(create_item(home_dir, "rm baz", 0)).unwrap();
+    history.save(&create_item(home_dir, "cat foo", 0)).unwrap();
+    history.save(&create_item(home_dir, "ls bar", 0)).unwrap();
+    history.save(&create_item(home_dir, "rm baz", 0)).unwrap();
 
     history
 }

src/engine.rs

@@ -1730,32 +1730,41 @@
     fn submit_buffer(&mut self, prompt: &dyn Prompt) -> io::Result<EventStatus> {
         let buffer = self.editor.get_buffer().to_string();
         self.hide_hints = true;
+
         // Additional repaint to show the content without hints etc.
         if let Some(transient_prompt) = self.transient_prompt.take() {
             self.repaint(transient_prompt.as_ref())?;
             self.transient_prompt = Some(transient_prompt);
         } else {
             self.repaint(prompt)?;
         }
-        if !buffer.is_empty() {
-            let mut entry = HistoryItem::from_command_line(&buffer);
-            entry.session_id = self.get_history_session_id();
 
-            if self
+        if !buffer.is_empty() {
+            let filtered = self
                 .history_exclusion_prefix
                 .as_ref()
-                .map(|prefix| buffer.starts_with(prefix))
-                .unwrap_or(false)
-            {
-                entry.id = Some(Self::FILTERED_ITEM_ID);
-                self.history_last_run_id = entry.id;
-                self.history_excluded_item = Some(entry);
+                .map_or(false, |prefix| buffer.starts_with(prefix));
+
+            let entry_id = if filtered {
+                Self::FILTERED_ITEM_ID
+            } else {
+                self.history.generate_id()
+            };
+
+            let mut entry = HistoryItem::from_command_line(&buffer, entry_id);
+
+            entry.session_id = self.get_history_session_id();
+
+            if filtered {
+                self.history.replace(&entry).expect("todo: error handling");
             } else {
-                entry = self.history.save(entry).expect("todo: error handling");
-                self.history_last_run_id = entry.id;
-                self.history_excluded_item = None;
+                self.history.save(&entry).expect("todo: error handling");
             }
+
+            self.history_last_run_id = Some(entry_id);
+            self.history_excluded_item = if filtered { Some(entry) } else { None };
         }
+
         self.run_edit_commands(&[EditCommand::Clear]);
         self.editor.reset_undo_stack();
 

src/history/base.rs

@@ -186,10 +186,15 @@ impl SearchQuery {
 /// Represents a history file or database
 /// Data could be stored e.g. in a plain text file, in a `JSONL` file, in a `SQLite` database
 pub trait History: Send {
-    /// save a history item to the database
-    /// if given id is None, a new id is created and set in the return value
-    /// if given id is Some, the existing entry is updated
-    fn save(&mut self, h: HistoryItem) -> Result<HistoryItem>;
+    /// generate a unique identifier for a new [`HistoryItem`]
+    fn generate_id(&mut self) -> HistoryItemId;
+
+    /// save a new history item to the database
+    fn save(&mut self, h: &HistoryItem) -> Result<()>;
+
+    /// replace an existing history item in the database
+    fn replace(&mut self, h: &HistoryItem) -> Result<()>;
+
     /// load a history item by its id
     fn load(&self, id: HistoryItemId) -> Result<HistoryItem>;
 
@@ -227,11 +232,18 @@ mod test {
     #[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))]
     const IS_FILE_BASED: bool = true;
 
+    use std::{
+        sync::atomic::{AtomicI64, Ordering},
+        time::Duration,
+    };
+
     use crate::HistorySessionId;
 
+    static COUNTER: AtomicI64 = AtomicI64::new(0);
+
     fn create_item(session: i64, cwd: &str, cmd: &str, exit_status: i64) -> HistoryItem {
         HistoryItem {
-            id: None,
+            id: HistoryItemId(COUNTER.fetch_add(1, Ordering::SeqCst)),
             start_timestamp: None,
             command_line: cmd.to_string(),
             session_id: Some(HistorySessionId::new(session)),
@@ -242,7 +254,6 @@ mod test {
             more_info: None,
         }
     }
-    use std::time::Duration;
 
     use super::*;
     fn create_filled_example_history() -> Result<Box<dyn History>> {
@@ -251,20 +262,20 @@ mod test {
         #[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))]
         let mut history = crate::FileBackedHistory::default();
         #[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))]
-        history.save(create_item(1, "/", "dummy", 0))?; // add dummy item so ids start with 1
-        history.save(create_item(1, "/home/me", "cd ~/Downloads", 0))?; // 1
-        history.save(create_item(1, "/home/me/Downloads", "unzp foo.zip", 1))?; // 2
-        history.save(create_item(1, "/home/me/Downloads", "unzip foo.zip", 0))?; // 3
-        history.save(create_item(1, "/home/me/Downloads", "cd foo", 0))?; // 4
-        history.save(create_item(1, "/home/me/Downloads/foo", "ls", 0))?; // 5
-        history.save(create_item(1, "/home/me/Downloads/foo", "ls -alh", 0))?; // 6
-        history.save(create_item(1, "/home/me/Downloads/foo", "cat x.txt", 0))?; // 7
-
-        history.save(create_item(1, "/home/me", "cd /etc/nginx", 0))?; // 8
-        history.save(create_item(1, "/etc/nginx", "ls -l", 0))?; // 9
-        history.save(create_item(1, "/etc/nginx", "vim nginx.conf", 0))?; // 10
-        history.save(create_item(1, "/etc/nginx", "vim htpasswd", 0))?; // 11
-        history.save(create_item(1, "/etc/nginx", "cat nginx.conf", 0))?; // 12
+        history.save(&create_item(1, "/", "dummy", 0))?; // add dummy item so ids start with 1
+        history.save(&create_item(1, "/home/me", "cd ~/Downloads", 0))?; // 1
+        history.save(&create_item(1, "/home/me/Downloads", "unzp foo.zip", 1))?; // 2
+        history.save(&create_item(1, "/home/me/Downloads", "unzip foo.zip", 0))?; // 3
+        history.save(&create_item(1, "/home/me/Downloads", "cd foo", 0))?; // 4
+        history.save(&create_item(1, "/home/me/Downloads/foo", "ls", 0))?; // 5
+        history.save(&create_item(1, "/home/me/Downloads/foo", "ls -alh", 0))?; // 6
+        history.save(&create_item(1, "/home/me/Downloads/foo", "cat x.txt", 0))?; // 7
+
+        history.save(&create_item(1, "/home/me", "cd /etc/nginx", 0))?; // 8
+        history.save(&create_item(1, "/etc/nginx", "ls -l", 0))?; // 9
+        history.save(&create_item(1, "/etc/nginx", "vim nginx.conf", 0))?; // 10
+        history.save(&create_item(1, "/etc/nginx", "vim htpasswd", 0))?; // 11
+        history.save(&create_item(1, "/etc/nginx", "cat nginx.conf", 0))?; // 12
         Ok(Box::new(history))
     }
 
@@ -409,8 +420,8 @@ mod test {
 
         // create history, add a few entries
         let mut history = open_history();
-        history.save(create_item(1, "/home/me", "cd ~/Downloads", 0))?; // 1
-        history.save(create_item(1, "/home/me/Downloads", "unzp foo.zip", 1))?; // 2
+        history.save(&create_item(1, "/home/me", "cd ~/Downloads", 0))?; // 1
+        history.save(&create_item(1, "/home/me/Downloads", "unzp foo.zip", 1))?; // 2
         assert_eq!(history.count_all()?, 2);
         drop(history);