From 9aad030152c962a0620809d781bca64a138baac7 Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Wed, 31 Jan 2024 14:06:11 +0100 Subject: [PATCH 1/4] Add `History::delete` for `FileBackedHistory` - Add `History::delete` for `FileBackedHistory` - Adapt `history` to print id rather than just consecutive numbers - Add `history delete ` to demo See also https://github.com/nushell/nushell/pull/11629 --- examples/demo.rs | 14 +++++++++++++- src/engine.rs | 5 +++-- src/history/file_backed.rs | 21 ++++++++++++++------- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/examples/demo.rs b/examples/demo.rs index ad1f808a..74e03c12 100644 --- a/examples/demo.rs +++ b/examples/demo.rs @@ -16,7 +16,7 @@ use { #[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))] use reedline::FileBackedHistory; -use reedline::{CursorConfig, MenuBuilder}; +use reedline::{CursorConfig, HistoryItemId, MenuBuilder}; fn main() -> reedline::Result<()> { println!("Ctrl-D to quit"); @@ -175,6 +175,18 @@ fn main() -> reedline::Result<()> { line_editor.print_history_session()?; continue; } + // Delete history entry of a certain id + if buffer.trim().starts_with("history delete-item") { + let parts: Vec<&str> = buffer.split_whitespace().collect(); + if parts.len() == 3 { + if let Ok(id) = parts[2].parse::() { + line_editor.history_mut().delete(HistoryItemId::new(id))?; + continue; + } + } + println!("Invalid command. Use: history delete "); + continue; + } // Get this history session identifier if buffer.trim() == "history sessionid" { line_editor.print_history_session_id()?; diff --git a/src/engine.rs b/src/engine.rs index e7bcceb7..dd2a12fb 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -536,8 +536,9 @@ impl Reedline { .search(SearchQuery::everything(SearchDirection::Forward, None)) .expect("todo: error handling"); - for (i, entry) in history.iter().enumerate() { - self.print_line(&format!("{}\t{}", i, entry.command_line))?; + for entry in history.iter() { + let Some(id) = entry.id else { continue }; + self.print_line(&format!("{}\t{}", id, entry.command_line))?; } Ok(()) } diff --git a/src/history/file_backed.rs b/src/history/file_backed.rs index ffff840b..125fd4c3 100644 --- a/src/history/file_backed.rs +++ b/src/history/file_backed.rs @@ -205,13 +205,20 @@ 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 = h.0 as usize; + let num_entries = self.entries.len(); + // Check if the id is valid + if id >= num_entries { + return Err(ReedlineError(ReedlineErrorVariants::OtherHistoryError( + "Given id is out of range.", + ))); + } + + // Remove the item with the specified id + self.entries.remove(id); + + Ok(()) } /// Writes unwritten history contents to disk. From 3673afeb4e7cf93107b067fd2e407d43c2cc6c9b Mon Sep 17 00:00:00 2001 From: Julian Hofer Date: Mon, 5 Feb 2024 08:46:26 +0100 Subject: [PATCH 2/4] Attempt to overwrite file when necessary --- src/history/file_backed.rs | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/history/file_backed.rs b/src/history/file_backed.rs index 125fd4c3..01d10652 100644 --- a/src/history/file_backed.rs +++ b/src/history/file_backed.rs @@ -31,6 +31,7 @@ pub struct FileBackedHistory { file: Option, len_on_disk: usize, // Keep track what was previously written to disk session: Option, + overwrite: bool, // Will overwrite file on next sync, not just new entries } impl Default for FileBackedHistory { @@ -217,6 +218,7 @@ impl History for FileBackedHistory { // Remove the item with the specified id self.entries.remove(id); + self.overwrite = true; Ok(()) } @@ -226,9 +228,6 @@ impl History for FileBackedHistory { /// If file would exceed `capacity` truncates the oldest entries. fn sync(&mut self) -> std::io::Result<()> { if let Some(fname) = &self.file { - // The unwritten entries - let own_entries = self.entries.range(self.len_on_disk..); - if let Some(base_dir) = fname.parent() { std::fs::create_dir_all(base_dir)?; } @@ -238,14 +237,39 @@ impl History for FileBackedHistory { .create(true) .write(true) .read(true) + .truncate(self.overwrite) .open(fname)?, ); let mut writer_guard = f_lock.write()?; + + if self.overwrite { + self.overwrite = false; + let mut writer = BufWriter::new(writer_guard.deref_mut()); + for line in &self.entries { + writer.write_all(encode_entry(line).as_bytes())?; + writer.write_all("\n".as_bytes())?; + } + writer.flush()?; + return Ok(()); + } + + // The unwritten entries + let own_entries = self.entries.range(self.len_on_disk..); + let (mut foreign_entries, truncate) = { let reader = BufReader::new(writer_guard.deref()); let mut from_file = reader .lines() .map(|o| o.map(|i| decode_entry(&i))) + .filter(|e| { + if let Ok(entry) = e { + if entry.starts_with(' ') { + self.overwrite = true; + return false; + } + } + true + }) .collect::>>()?; if from_file.len() + own_entries.len() > self.capacity { ( @@ -313,6 +337,7 @@ impl FileBackedHistory { file: None, len_on_disk: 0, session: None, + overwrite: false, }) } From 192713dd2d60cee3128a427e15b907517a5fb454 Mon Sep 17 00:00:00 2001 From: Hofer-Julian Date: Mon, 5 Feb 2024 11:24:43 +0100 Subject: [PATCH 3/4] Rename `history delete-item` to `history remove-item` --- examples/demo.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/demo.rs b/examples/demo.rs index 74e03c12..867d58b1 100644 --- a/examples/demo.rs +++ b/examples/demo.rs @@ -176,7 +176,7 @@ fn main() -> reedline::Result<()> { continue; } // Delete history entry of a certain id - if buffer.trim().starts_with("history delete-item") { + if buffer.trim().starts_with("history remove-item") { let parts: Vec<&str> = buffer.split_whitespace().collect(); if parts.len() == 3 { if let Ok(id) = parts[2].parse::() { @@ -184,7 +184,7 @@ fn main() -> reedline::Result<()> { continue; } } - println!("Invalid command. Use: history delete "); + println!("Invalid command. Use: history remove-item "); continue; } // Get this history session identifier From 0207cf279514c19c75c8e4b0d85bdcfe9a1b4255 Mon Sep 17 00:00:00 2001 From: Hofer-Julian Date: Mon, 5 Feb 2024 11:33:25 +0100 Subject: [PATCH 4/4] Improve docs for `History::sync` --- src/history/file_backed.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/history/file_backed.rs b/src/history/file_backed.rs index 01d10652..239e4371 100644 --- a/src/history/file_backed.rs +++ b/src/history/file_backed.rs @@ -223,9 +223,11 @@ impl History for FileBackedHistory { Ok(()) } - /// Writes unwritten history contents to disk. + /// Syncs current state with disk. /// + /// Normally, that means reading entries from the file and appending new entries to it. /// If file would exceed `capacity` truncates the oldest entries. + /// If necessary, the whole file is overwritten. fn sync(&mut self) -> std::io::Result<()> { if let Some(fname) = &self.file { if let Some(base_dir) = fname.parent() {