From f9b66c36dda118624c6ce007c332c7d2da309f27 Mon Sep 17 00:00:00 2001 From: Olaf Liebe Date: Wed, 20 Nov 2024 12:46:47 +0100 Subject: [PATCH 1/4] Added some tests before refactoring. --- src/history.rs | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/src/history.rs b/src/history.rs index afa8670..c27bd3d 100644 --- a/src/history.rs +++ b/src/history.rs @@ -78,3 +78,71 @@ impl History { } } } + +#[cfg(test)] +#[tokio::test] +async fn test_history() { + let mut history = History::default(); + + history.sender.unbounded_send("foo".into()).unwrap(); + history.update().await; + history.sender.unbounded_send("bar".into()).unwrap(); + history.update().await; + history.sender.unbounded_send("baz".into()).unwrap(); + history.update().await; + + for _ in 0..2 { + // Previous will navigate nowhere. + assert_eq!(None, history.search_previous("")); + + // Going back in history. + assert_eq!(Some("baz"), history.search_next("")); + assert_eq!(Some("bar"), history.search_next("")); + assert_eq!(Some("foo"), history.search_next("")); + + // Last entry should just repeat. + assert_eq!(Some("foo"), history.search_next("")); + + // Going forward. + assert_eq!(Some("bar"), history.search_previous("")); + assert_eq!(Some("baz"), history.search_previous("")); + + // Alternate. + assert_eq!(Some("bar"), history.search_next("")); + assert_eq!(Some("baz"), history.search_previous("")); + + // Back to the beginning. Should return "" once. + assert_eq!(Some(""), history.search_previous("")); + assert_eq!(None, history.search_previous("")); + + // Going back again. + assert_eq!(Some("baz"), history.search_next("")); + assert_eq!(Some("bar"), history.search_next("")); + + // Resetting the position. + history.reset_position(); + } +} + +#[cfg(test)] +#[tokio::test] +async fn test_history_limit() { + let mut history = History { + max_size: 3, + ..Default::default() + }; + + history.sender.unbounded_send("foo".into()).unwrap(); + history.update().await; + history.sender.unbounded_send("bar".into()).unwrap(); + history.update().await; + history.sender.unbounded_send("baz".into()).unwrap(); + history.update().await; + history.sender.unbounded_send("qux".into()).unwrap(); // Should remove "foo". + history.update().await; + + assert_eq!(Some("qux"), history.search_next("")); + assert_eq!(Some("baz"), history.search_next("")); + assert_eq!(Some("bar"), history.search_next("")); + assert_eq!(Some("bar"), history.search_next("")); +} From 2e5da09a7619c4ca9396f01ad8b96af9518d5380 Mon Sep 17 00:00:00 2001 From: Olaf Liebe Date: Wed, 20 Nov 2024 12:58:15 +0100 Subject: [PATCH 2/4] Removed mpsc channel for history. --- Cargo.lock | 1 - Cargo.toml | 1 - src/history.rs | 59 +++++++++++++++++--------------------------------- src/lib.rs | 11 +++------- 4 files changed, 23 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f40885f..effc5b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -691,7 +691,6 @@ version = "0.4.3" dependencies = [ "async-std", "crossterm", - "futures-channel", "futures-util", "log", "pin-project", diff --git a/Cargo.toml b/Cargo.toml index 299ac10..fe5cead 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,6 @@ edition = "2021" [dependencies] crossterm = { version = "0.28.1", features = ["event-stream"] } -futures-channel = "0.3" futures-util = { version = "0.3", features = ["io"] } pin-project = "1.0" thingbuf = "0.1" diff --git a/src/history.rs b/src/history.rs index c27bd3d..3e84a5c 100644 --- a/src/history.rs +++ b/src/history.rs @@ -1,24 +1,15 @@ use std::collections::VecDeque; -use futures_channel::mpsc::{self, UnboundedReceiver, UnboundedSender}; -use futures_util::StreamExt; - pub struct History { pub entries: VecDeque, pub max_size: usize, - pub sender: UnboundedSender, - receiver: UnboundedReceiver, - current_position: Option, } impl Default for History { fn default() -> Self { - let (sender, receiver) = mpsc::unbounded(); Self { entries: Default::default(), max_size: 1000, - sender, - receiver, current_position: Default::default(), } } @@ -26,22 +17,19 @@ impl Default for History { impl History { // Update history entries - pub async fn update(&mut self) { - // Receive a new line - if let Some(line) = self.receiver.next().await { - // Reset offset to newest entry - self.current_position = None; - // Don't add entry if last entry was same, or line was empty. - if self.entries.front() == Some(&line) || line.is_empty() { - return; - } - // Add entry to front of history - self.entries.push_front(line); - // Check if already have enough entries - if self.entries.len() > self.max_size { - // Remove oldest entry - self.entries.pop_back(); - } + pub fn add_entry(&mut self, line: String) { + // Reset offset to newest entry + self.current_position = None; + // Don't add entry if last entry was same, or line was empty. + if self.entries.front() == Some(&line) || line.is_empty() { + return; + } + // Add entry to front of history + self.entries.push_front(line); + // Check if already have enough entries + if self.entries.len() > self.max_size { + // Remove oldest entry + self.entries.pop_back(); } } @@ -84,12 +72,9 @@ impl History { async fn test_history() { let mut history = History::default(); - history.sender.unbounded_send("foo".into()).unwrap(); - history.update().await; - history.sender.unbounded_send("bar".into()).unwrap(); - history.update().await; - history.sender.unbounded_send("baz".into()).unwrap(); - history.update().await; + history.add_entry("foo".into()); + history.add_entry("bar".into()); + history.add_entry("baz".into()); for _ in 0..2 { // Previous will navigate nowhere. @@ -132,14 +117,10 @@ async fn test_history_limit() { ..Default::default() }; - history.sender.unbounded_send("foo".into()).unwrap(); - history.update().await; - history.sender.unbounded_send("bar".into()).unwrap(); - history.update().await; - history.sender.unbounded_send("baz".into()).unwrap(); - history.update().await; - history.sender.unbounded_send("qux".into()).unwrap(); // Should remove "foo". - history.update().await; + history.add_entry("foo".into()); + history.add_entry("bar".into()); + history.add_entry("baz".into()); + history.add_entry("qux".into()); // Should remove "foo". assert_eq!(Some("qux"), history.search_next("")); assert_eq!(Some("baz"), history.search_next("")); diff --git a/src/lib.rs b/src/lib.rs index ec51963..f3cc7ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,7 +54,6 @@ use crossterm::{ terminal::{self, disable_raw_mode, Clear}, QueueableCommand, }; -use futures_channel::mpsc; use futures_util::{pin_mut, ready, select, AsyncWrite, FutureExt, StreamExt}; use thingbuf::mpsc::{errors::TrySendError, Receiver, Sender}; use thiserror::Error; @@ -189,10 +188,7 @@ pub struct Readline { raw_term: Stdout, event_stream: EventStream, // Stream of events line_receiver: Receiver>, - line: LineState, // Current line - - history_sender: mpsc::UnboundedSender, } impl Readline { @@ -203,14 +199,12 @@ impl Readline { terminal::enable_raw_mode()?; let line = LineState::new(prompt, terminal::size()?); - let history_sender = line.history.sender.clone(); let mut readline = Readline { raw_term: stdout(), event_stream: EventStream::new(), line_receiver, line, - history_sender, }; readline.line.render(&mut readline.raw_term)?; readline.raw_term.queue(terminal::EnableLineWrap)?; @@ -297,14 +291,15 @@ impl Readline { }, None => return Err(ReadlineError::Closed), }, - _ = self.line.history.update().fuse() => {} } } } /// Add a line to the input history pub fn add_history_entry(&mut self, entry: String) -> Option<()> { - self.history_sender.unbounded_send(entry).ok() + self.line.history.add_entry(entry); + // Return value to keep compatibility with previous API. + Some(()) } } From f91baca39773cd8c8b01459fba6df78ec4bc2131 Mon Sep 17 00:00:00 2001 From: Olaf Liebe Date: Wed, 20 Nov 2024 13:02:25 +0100 Subject: [PATCH 3/4] Flipped order of entries in VecDeque (more natural for import/export). --- src/history.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/history.rs b/src/history.rs index 3e84a5c..0e10e7a 100644 --- a/src/history.rs +++ b/src/history.rs @@ -1,6 +1,7 @@ use std::collections::VecDeque; pub struct History { + // Note: old entries in front, new ones at the back. pub entries: VecDeque, pub max_size: usize, current_position: Option, @@ -21,15 +22,15 @@ impl History { // Reset offset to newest entry self.current_position = None; // Don't add entry if last entry was same, or line was empty. - if self.entries.front() == Some(&line) || line.is_empty() { + if self.entries.back() == Some(&line) || line.is_empty() { return; } - // Add entry to front of history - self.entries.push_front(line); + // Add entry to back of history + self.entries.push_back(line); // Check if already have enough entries if self.entries.len() > self.max_size { // Remove oldest entry - self.entries.pop_back(); + self.entries.pop_front(); } } @@ -41,25 +42,26 @@ impl History { // Find next history that matches a given string from an index pub fn search_next(&mut self, _current: &str) -> Option<&str> { if let Some(index) = &mut self.current_position { - if *index < self.entries.len() - 1 { - *index += 1; + if *index > 0 { + *index -= 1; } Some(&self.entries[*index]) - } else if !self.entries.is_empty() { - self.current_position = Some(0); - Some(&self.entries[0]) + } else if let Some(last) = self.entries.back() { + self.current_position = Some(self.entries.len() - 1); + Some(last) } else { None } } + // Find previous history item that matches a given string from an index pub fn search_previous(&mut self, _current: &str) -> Option<&str> { if let Some(index) = &mut self.current_position { - if *index == 0 { + if *index == self.entries.len() - 1 { self.current_position = None; return Some(""); } - *index -= 1; + *index += 1; Some(&self.entries[*index]) } else { None From ace85def5addecc4a3c6bfb74e382bca108bcffc Mon Sep 17 00:00:00 2001 From: Olaf Liebe Date: Wed, 20 Nov 2024 13:51:32 +0100 Subject: [PATCH 4/4] Extended history API (get/set/clear/load/save). --- src/history.rs | 98 ++++++++++++++++++++++++++++++++++++++++++++++---- src/lib.rs | 49 +++++++++++++++++++++---- 2 files changed, 135 insertions(+), 12 deletions(-) diff --git a/src/history.rs b/src/history.rs index 0e10e7a..87c33ab 100644 --- a/src/history.rs +++ b/src/history.rs @@ -2,8 +2,8 @@ use std::collections::VecDeque; pub struct History { // Note: old entries in front, new ones at the back. - pub entries: VecDeque, - pub max_size: usize, + entries: VecDeque, + max_size: usize, current_position: Option, } impl Default for History { @@ -34,6 +34,36 @@ impl History { } } + // Changes the history size. + pub fn set_max_size(&mut self, max_size: usize) { + self.max_size = max_size; + + while self.entries.len() > max_size { + // Remove oldest entry + self.entries.pop_front(); + } + + // Make sure we don't end up in an invalid position. + self.reset_position(); + } + + // Returns the current history entries. + pub fn get_entries(&self) -> &VecDeque { + &self.entries + } + + // Replaces the current history entries. + pub fn set_entries(&mut self, entries: impl IntoIterator) { + self.entries.clear(); + + // Using `add_entry` will respect `max_size` and remove duplicate lines etc. + for entry in entries.into_iter() { + self.add_entry(entry); + } + + self.reset_position(); + } + // Sets the history position back to the start. pub fn reset_position(&mut self) { self.current_position = None; @@ -70,8 +100,8 @@ impl History { } #[cfg(test)] -#[tokio::test] -async fn test_history() { +#[test] +fn test_history() { let mut history = History::default(); history.add_entry("foo".into()); @@ -112,8 +142,8 @@ async fn test_history() { } #[cfg(test)] -#[tokio::test] -async fn test_history_limit() { +#[test] +fn test_history_limit() { let mut history = History { max_size: 3, ..Default::default() @@ -128,4 +158,60 @@ async fn test_history_limit() { assert_eq!(Some("baz"), history.search_next("")); assert_eq!(Some("bar"), history.search_next("")); assert_eq!(Some("bar"), history.search_next("")); + + history.set_max_size(2); + + assert_eq!(Some("qux"), history.search_next("")); + assert_eq!(Some("baz"), history.search_next("")); + assert_eq!(Some("baz"), history.search_next("")); +} + +#[cfg(test)] +#[test] +fn test_history_reset_on_add() { + let mut history = History::default(); + + history.add_entry("foo".into()); + history.add_entry("bar".into()); + history.add_entry("baz".into()); + + assert_eq!(None, history.search_previous("")); + assert_eq!(Some("baz"), history.search_next("")); + assert_eq!(Some("bar"), history.search_next("")); + + // This should reset the history position. + history.add_entry("qux".into()); + + assert_eq!(None, history.search_previous("")); + assert_eq!(Some("qux"), history.search_next("")); + assert_eq!(Some("baz"), history.search_next("")); + assert_eq!(Some("bar"), history.search_next("")); + assert_eq!(Some("foo"), history.search_next("")); +} + +#[cfg(test)] +#[test] +fn test_history_export() { + let mut history = History { + max_size: 3, + ..Default::default() + }; + + assert_eq!(history.get_entries(), &VecDeque::new()); + + history.add_entry("foo".into()); + history.add_entry("bar".into()); + history.add_entry("baz".into()); + + assert_eq!(history.get_entries(), &["foo", "bar", "baz"]); + + history.add_entry("qux".into()); + + assert_eq!(history.get_entries(), &["bar", "baz", "qux"]); + + history.set_entries(["a".to_string(), "b".to_string(), "b".to_string()]); + + assert_eq!(Some("b"), history.search_next("")); + assert_eq!(Some("a"), history.search_next("")); + assert_eq!(Some("a"), history.search_next("")); } diff --git a/src/lib.rs b/src/lib.rs index f3cc7ce..d82b070 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,10 +43,7 @@ //! - Ctrl-C: Send an `Interrupt` event use std::{ - io::{self, stdout, Stdout, Write}, - ops::DerefMut, - pin::Pin, - task::{Context, Poll}, + collections::VecDeque, fs::File, io::{self, stdout, BufRead, BufReader, BufWriter, Stdout, Write}, ops::DerefMut, path::Path, pin::Pin, task::{Context, Poll} }; use crossterm::{ @@ -234,8 +231,7 @@ impl Readline { /// Set maximum history length. The default length is 1000. pub fn set_max_history(&mut self, max_size: usize) { - self.line.history.max_size = max_size; - self.line.history.entries.truncate(max_size); + self.line.history.set_max_size(max_size); } /// Set whether the input line should remain on the screen after @@ -301,6 +297,47 @@ impl Readline { // Return value to keep compatibility with previous API. Some(()) } + + /// Returns the entries of the history in the order they were added in. + pub fn get_history_entries(&self) -> &VecDeque { + self.line.history.get_entries() + } + + /// Replaces the current history. + pub fn set_history_entries(&mut self, entries: impl IntoIterator) { + self.line.history.set_entries(entries); + } + + /// Clears the current history. + pub fn clear_history(&mut self) { + self.set_history_entries([]); + } + + /// Saves the history as a plain text file. + pub fn save_history(&self, path: impl AsRef) -> std::io::Result<()> { + let file = File::create(path)?; + let mut writer = BufWriter::new(file); + + for line in self.get_history_entries() { + writeln!(writer, "{line}")?; + } + + Ok(()) + } + + /// Loads the history from a plain text file. + pub fn load_history(&mut self, path: impl AsRef) -> std::io::Result<()> { + let file = File::open(path)?; + let reader = BufReader::new(file); + + self.clear_history(); + + for line in reader.lines() { + self.add_history_entry(line?); + } + + Ok(()) + } } impl Drop for Readline {