enable deleting history items with keybinding

src/edit_mode/keybindings.rs

@@ -101,6 +101,7 @@ pub fn add_common_control_bindings(kb: &mut Keybindings) {
     kb.add_binding(KM::CONTROL, KC::Char('d'), ReedlineEvent::CtrlD);
     kb.add_binding(KM::CONTROL, KC::Char('l'), ReedlineEvent::ClearScreen);
     kb.add_binding(KM::CONTROL, KC::Char('r'), ReedlineEvent::SearchHistory);
+    kb.add_binding(KM::SHIFT, KC::Delete, ReedlineEvent::DeleteHistoryItem);
     kb.add_binding(KM::CONTROL, KC::Char('o'), ReedlineEvent::OpenEditor);
 }
 /// Add the arrow navigation and its `Ctrl` variants

src/engine.rs

@@ -764,6 +764,15 @@ impl Reedline {
                 // A handled Event causes a repaint
                 Ok(EventStatus::Handled)
             }
+            ReedlineEvent::DeleteHistoryItem => {
+                match self
+                    .history_cursor
+                    .delete_item_at_cursor(self.history.as_mut())
+                {
+                    Some(Ok(())) => Ok(EventStatus::Handled),
+                    Some(Err(_)) | None => Ok(EventStatus::Inapplicable),
+                }
+            }
             ReedlineEvent::PreviousHistory | ReedlineEvent::Up | ReedlineEvent::SearchHistory => {
                 self.history_cursor
                     .back(self.history.as_ref())
@@ -1097,6 +1106,26 @@ impl Reedline {
                 self.enter_history_search();
                 Ok(EventStatus::Handled)
             }
+            ReedlineEvent::DeleteHistoryItem => {
+                if self.input_mode != InputMode::HistoryTraversal {
+                    self.run_edit_commands(&[EditCommand::Clear]);
+                    return Ok(EventStatus::Handled);
+                }
+                match self
+                    .history_cursor
+                    .delete_item_at_cursor(self.history.as_mut())
+                {
+                    Some(Ok(())) => {
+                        self.update_buffer_from_history();
+                        // Move to end of first line, see `Self::previous_history()`.
+                        self.editor.move_to_start(UndoBehavior::HistoryNavigation);
+                        self.editor
+                            .move_to_line_end(UndoBehavior::HistoryNavigation);
+                        Ok(EventStatus::Handled)
+                    }
+                    Some(Err(_)) | None => Ok(EventStatus::Inapplicable),
+                }
+            }
             ReedlineEvent::Multiple(events) => {
                 let mut latest_signal = EventStatus::Inapplicable;
                 for event in events {
@@ -1159,6 +1188,7 @@ impl Reedline {
             .back(self.history.as_ref())
             .expect("todo: error handling");
         self.update_buffer_from_history();
+        // Move to end of *first* line, so that pressing up again goes directly to previous item.
         self.editor.move_to_start(UndoBehavior::HistoryNavigation);
         self.editor
             .move_to_line_end(UndoBehavior::HistoryNavigation);

src/enums.rs

@@ -470,6 +470,9 @@ pub enum ReedlineEvent {
     /// Search the history for a string
     SearchHistory,
 
+    /// Delete currently selected history item
+    DeleteHistoryItem,
+
     /// In vi mode multiple reedline events can be chained while parsing the
     /// command or movement characters
     Multiple(Vec<ReedlineEvent>),
@@ -539,6 +542,7 @@ impl Display for ReedlineEvent {
             ReedlineEvent::Left => write!(f, "Left"),
             ReedlineEvent::NextHistory => write!(f, "NextHistory"),
             ReedlineEvent::SearchHistory => write!(f, "SearchHistory"),
+            ReedlineEvent::DeleteHistoryItem => write!(f, "DeleteHistoryItem"),
             ReedlineEvent::Multiple(_) => write!(f, "Multiple[ {{ ReedLineEvents, }} ]"),
             ReedlineEvent::UntilFound(_) => write!(f, "UntilFound [ {{ ReedLineEvents, }} ]"),
             ReedlineEvent::Menu(_) => write!(f, "Menu Name: <string>"),

src/history/cursor.rs

@@ -91,6 +91,33 @@ impl HistoryCursor {
         self.current.as_ref().map(|e| e.command_line.to_string())
     }
 
+    /// Delete the item (if present) at the cursor from the history, and go back or forward one item
+    pub fn delete_item_at_cursor(&mut self, history: &mut dyn History) -> Option<Result<()>> {
+        let current = self.current.as_ref()?;
+        if let Some(id) = current.id {
+            match history.delete(id) {
+                Ok(()) => {
+                    // Cursor must be moved *after* deletion, as deleting may
+                    // alter which entry a `HistoryItemId` points to.
+                    if (self.back(history).is_err()
+                        || self.current.is_none()
+                        || self.current.as_ref().unwrap().id == Some(id))
+                        && self.forward(history).is_err()
+                    {
+                        self.current = None;
+                    }
+                    Some(Ok(()))
+                }
+                Err(e) => Some(Err(e)),
+            }
+        } else {
+            if self.back(history).is_err() {
+                self.current = None;
+            }
+            Some(Ok(()))
+        }
+    }
+
     /// Poll the current [`HistoryNavigationQuery`] mode
     pub fn get_navigation(&self) -> HistoryNavigationQuery {
         self.query.clone()

src/history/file_backed.rs

@@ -192,19 +192,99 @@ impl History for FileBackedHistory {
         Ok(())
     }
 
-    fn delete(&mut self, _h: super::HistoryItemId) -> Result<()> {
-        Err(ReedlineError(
-            ReedlineErrorVariants::HistoryFeatureUnsupported {
-                history: "FileBackedHistory",
-                feature: "removing entries",
-            },
-        ))
+    fn delete(&mut self, h: super::HistoryItemId) -> Result<()> {
+        let id = usize::try_from(h.0).map_err(|_| {
+            ReedlineError(ReedlineErrorVariants::OtherHistoryError(
+                "Invalid ID (not usize)",
+            ))
+        })?;
+
+        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..);
@@ -223,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)
                 }
             };
 
@@ -269,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 {

src/history/item.rs

@@ -52,7 +52,7 @@ impl From<HistorySessionId> for i64 {
 /// This trait represents additional arbitrary context to be added to a history (optional, see [`HistoryItem`])
 pub trait HistoryItemExtraInfo: Serialize + DeserializeOwned + Default + Send {}
 
-#[derive(Default, Debug, PartialEq, Eq)]
+#[derive(Clone, Copy, Default, Debug, PartialEq, Eq)]
 /// something that is serialized as null and deserialized by ignoring everything
 pub struct IgnoreAllExtraInfo;