From 2d6fe65ef00c495127c3c82aee9e48438dcf11af Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Sun, 28 Jan 2024 10:34:52 -0500 Subject: [PATCH 1/8] Rework external printer --- examples/external_printer.rs | 25 +++++--- src/engine.rs | 52 +++++----------- src/external_printer.rs | 116 +++++++++++++++++++---------------- src/lib.rs | 9 +-- 4 files changed, 98 insertions(+), 104 deletions(-) diff --git a/examples/external_printer.rs b/examples/external_printer.rs index 633a1238..f52d4406 100644 --- a/examples/external_printer.rs +++ b/examples/external_printer.rs @@ -3,42 +3,47 @@ // cargo run --example external_printer --features=external_printer use { - reedline::ExternalPrinter, - reedline::{DefaultPrompt, Reedline, Signal}, + reedline::{DefaultPrompt, ExternalPrinterChannel, Reedline, Signal}, std::thread, std::thread::sleep, std::time::Duration, }; fn main() { - let printer = ExternalPrinter::default(); - // make a clone to use it in a different thread - let p_clone = printer.clone(); - // get the Sender to have full sending control - let p_sender = printer.sender(); + // Create a channel for the external printer. + let channel = ExternalPrinterChannel::default(); + + // Get a clone of the channel sender to use in a separate thread. + let printer = channel.sender(); + + // Note that the senders/printers can also be cloned. + // let printer_clone = printer.clone(); // external printer that prints a message every second thread::spawn(move || { let mut i = 1; loop { sleep(Duration::from_secs(1)); - assert!(p_clone + assert!(printer .print(format!("Message {i} delivered.\nWith two lines!")) .is_ok()); i += 1; } }); + let printer = channel.sender(); + // external printer that prints a bunch of messages after 3 seconds thread::spawn(move || { sleep(Duration::from_secs(3)); for _ in 0..10 { sleep(Duration::from_millis(1)); - assert!(p_sender.send("Fast Hello !".to_string()).is_ok()); + assert!(printer.print("Hello!").is_ok()); } }); - let mut line_editor = Reedline::create().with_external_printer(printer); + // create a `Reedline` struct and assign the channel for the external printer + let mut line_editor = Reedline::create().with_external_printer(channel); let prompt = DefaultPrompt::default(); loop { diff --git a/src/engine.rs b/src/engine.rs index e7bcceb7..d5544369 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -3,18 +3,14 @@ use std::path::PathBuf; use itertools::Itertools; use nu_ansi_term::{Color, Style}; +#[cfg(feature = "external_printer")] +use crate::ExternalPrinterChannel; use crate::{enums::ReedlineRawEvent, CursorConfig}; #[cfg(feature = "bashisms")] use crate::{ history::SearchFilter, menu_functions::{parse_selection_char, ParseAction}, }; -#[cfg(feature = "external_printer")] -use { - crate::external_printer::ExternalPrinter, - crossbeam::channel::TryRecvError, - std::io::{Error, ErrorKind}, -}; use { crate::{ completion::{Completer, DefaultCompleter}, @@ -154,7 +150,7 @@ pub struct Reedline { kitty_protocol: KittyProtocolGuard, #[cfg(feature = "external_printer")] - external_printer: Option>, + external_printer_channel: Option, } struct BufferEditor { @@ -225,7 +221,7 @@ impl Reedline { bracketed_paste: BracketedPasteGuard::default(), kitty_protocol: KittyProtocolGuard::default(), #[cfg(feature = "external_printer")] - external_printer: None, + external_printer_channel: None, } } @@ -679,11 +675,10 @@ impl Reedline { let mut paste_enter_state = false; #[cfg(feature = "external_printer")] - if let Some(ref external_printer) = self.external_printer { + if let Some(channel) = &self.external_printer_channel { // get messages from printer as crlf separated "lines" - let messages = Self::external_messages(external_printer)?; + let messages = channel.messages(); if !messages.is_empty() { - // print the message(s) self.painter.print_external_message( messages, self.editor.line_buffer(), @@ -1730,38 +1725,23 @@ impl Reedline { ) } - /// Adds an external printer + /// Returns a reference to the external printer's channel, or none if one was not set /// /// ## Required feature: /// `external_printer` #[cfg(feature = "external_printer")] - pub fn with_external_printer(mut self, printer: ExternalPrinter) -> Self { - self.external_printer = Some(printer); - self + pub fn external_printer(&self) -> Option<&ExternalPrinterChannel> { + self.external_printer_channel.as_ref() } + /// Adds an external printer + /// + /// ## Required feature: + /// `external_printer` #[cfg(feature = "external_printer")] - fn external_messages(external_printer: &ExternalPrinter) -> Result> { - let mut messages = Vec::new(); - loop { - let result = external_printer.receiver().try_recv(); - match result { - Ok(line) => { - let lines = line.lines().map(String::from).collect::>(); - messages.extend(lines); - } - Err(TryRecvError::Empty) => { - break; - } - Err(TryRecvError::Disconnected) => { - return Err(Error::new( - ErrorKind::NotConnected, - TryRecvError::Disconnected, - )); - } - } - } - Ok(messages) + pub fn with_external_printer(mut self, channel: ExternalPrinterChannel) -> Self { + self.external_printer_channel = Some(channel); + self } fn submit_buffer(&mut self, prompt: &dyn Prompt) -> io::Result { diff --git a/src/external_printer.rs b/src/external_printer.rs index f4c71e7c..a10a86b9 100644 --- a/src/external_printer.rs +++ b/src/external_printer.rs @@ -1,72 +1,80 @@ -//! To print messages while editing a line -//! -//! See example: -//! -//! ``` shell -//! cargo run --example external_printer --features=external_printer -//! ``` -#[cfg(feature = "external_printer")] -use { - crossbeam::channel::{bounded, Receiver, SendError, Sender}, - std::fmt::Display, -}; +use std::fmt::Display; -#[cfg(feature = "external_printer")] -pub const EXTERNAL_PRINTER_DEFAULT_CAPACITY: usize = 20; +use crossbeam::channel::{bounded, Receiver, SendError, Sender, TryRecvError}; -/// An ExternalPrinter allows to print messages of text while editing a line. -/// The message is printed as a new line, the line-edit will continue below the -/// output. +/// An external printer allows one to print messages of text while editing a line. +/// The message is printed as a new line, and the line-edit will continue below the output. /// /// ## Required feature: /// `external_printer` -#[cfg(feature = "external_printer")] -#[derive(Debug, Clone)] -pub struct ExternalPrinter -where - T: Display, -{ - sender: Sender, - receiver: Receiver, +#[derive(Debug)] +pub struct ExternalPrinterChannel { + sender: Sender, + receiver: Receiver, } -#[cfg(feature = "external_printer")] -impl ExternalPrinter -where - T: Display, -{ - /// Creates an ExternalPrinter to store lines with a max_cap - pub fn new(max_cap: usize) -> Self { - let (sender, receiver) = bounded::(max_cap); - Self { sender, receiver } - } - /// Gets a Sender to use the printer externally by sending lines to it - pub fn sender(&self) -> Sender { - self.sender.clone() +impl ExternalPrinterChannel { + pub const DEFAULT_CAPACITY: usize = 20; + + pub fn new() -> Self { + Self::default() } - /// Receiver to get messages if any - pub fn receiver(&self) -> &Receiver { - &self.receiver + + pub fn with_capacity(capacity: usize) -> Self { + let (sender, receiver) = bounded(capacity); + Self { sender, receiver } } - /// Convenience method if the whole Printer is cloned, blocks if max_cap is reached. - /// - pub fn print(&self, line: T) -> Result<(), SendError> { - self.sender.send(line) + pub fn sender(&self) -> ExternalPrinter { + ExternalPrinter(self.sender.clone()) } - /// Convenience method to get a line if any, doesn´t block. - pub fn get_line(&self) -> Option { - self.receiver.try_recv().ok() + pub(crate) fn messages(&self) -> Vec { + let mut messages = Vec::new(); + loop { + match self.receiver.try_recv() { + Ok(string) => { + messages.extend(string.lines().map(String::from)); + } + Err(TryRecvError::Empty) => { + break; + } + Err(TryRecvError::Disconnected) => { + debug_assert!(false); // there is always one sender in `self`. + break; + } + } + } + messages } } -#[cfg(feature = "external_printer")] -impl Default for ExternalPrinter -where - T: Display, -{ +impl Default for ExternalPrinterChannel { fn default() -> Self { - Self::new(EXTERNAL_PRINTER_DEFAULT_CAPACITY) + Self::with_capacity(Self::DEFAULT_CAPACITY) + } +} + +/// An external printer allows one to print messages of text while editing a line. +/// The message is printed as a new line, and the line-edit will continue below the output. +/// +/// ## Required feature: +/// `external_printer` +#[derive(Debug, Clone)] +pub struct ExternalPrinter(Sender); + +impl ExternalPrinter { + /// Queues a string message to be printed + /// + /// This function blocks if the underlying channel is full. + pub fn print(&self, string: impl Into) -> Result<(), String> { + self.0.send(string.into()).map_err(|SendError(s)| s) + } + + /// Queues a value to be printed via its [`Display`] implementation + /// + /// This function blocks if the underlying channel is full. + pub fn display(&self, value: &T) -> Result<(), String> { + self.print(value.to_string()) } } diff --git a/src/lib.rs b/src/lib.rs index 4d14261b..3a0d9dd5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -284,9 +284,12 @@ pub use menu::{ mod terminal_extensions; pub use terminal_extensions::kitty_protocol_available; -mod utils; - +#[cfg(feature = "external_printer")] mod external_printer; +#[cfg(feature = "external_printer")] +pub use external_printer::{ExternalPrinter, ExternalPrinterChannel}; + +mod utils; pub use utils::{ get_reedline_default_keybindings, get_reedline_edit_commands, get_reedline_keybinding_modifiers, get_reedline_keycodes, get_reedline_prompt_edit_modes, @@ -298,5 +301,3 @@ pub use crossterm::{ event::{KeyCode, KeyModifiers}, style::Color, }; -#[cfg(feature = "external_printer")] -pub use external_printer::ExternalPrinter; From 570546b7e432e2c6a531f0939ae81837546964f7 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Sun, 28 Jan 2024 10:37:43 -0500 Subject: [PATCH 2/8] Fix event polling --- src/engine.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index d5544369..aae47239 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -689,7 +689,11 @@ impl Reedline { } let mut latest_resize = None; - loop { + + // There could be multiple events queued up! + // pasting text, resizes, blocking this thread (e.g. during debugging) + // We should be able to handle all of them as quickly as possible without causing unnecessary output steps. + while event::poll(Duration::from_millis(POLL_WAIT))? { match event::read()? { Event::Resize(x, y) => { latest_resize = Some((x, y)); @@ -718,13 +722,6 @@ impl Reedline { } } } - - // There could be multiple events queued up! - // pasting text, resizes, blocking this thread (e.g. during debugging) - // We should be able to handle all of them as quickly as possible without causing unnecessary output steps. - if !event::poll(Duration::from_millis(POLL_WAIT))? { - break; - } } if let Some((x, y)) = latest_resize { From 0d8089ea00e1c7657aac6c51782096687c43c7fb Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Sun, 28 Jan 2024 17:14:08 -0500 Subject: [PATCH 3/8] Add doc comments and one test --- src/external_printer.rs | 49 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/src/external_printer.rs b/src/external_printer.rs index a10a86b9..d1c121d7 100644 --- a/src/external_printer.rs +++ b/src/external_printer.rs @@ -2,6 +2,10 @@ use std::fmt::Display; use crossbeam::channel::{bounded, Receiver, SendError, Sender, TryRecvError}; +/// The channel to which external messages can be sent +/// +/// Use the [`sender`](Self::sender) to create [`ExternalPrinter`]s for use in other threads. +/// /// An external printer allows one to print messages of text while editing a line. /// The message is printed as a new line, and the line-edit will continue below the output. /// @@ -14,22 +18,33 @@ pub struct ExternalPrinterChannel { } impl ExternalPrinterChannel { + /// The default maximum number of lines that can be queued up for printing + /// + /// If the channel is full, calls to [`ExternalPrinter::print`] will block + /// and wait for the channel to have spare capacity again. pub const DEFAULT_CAPACITY: usize = 20; + /// Create a new `ExternalPrinterChannel` with default capacity pub fn new() -> Self { Self::default() } + /// Create a new `ExternalPrinterChannel` with the given capacity + /// + /// The capacity determines the maximum number of lines that can be queued up for printing + /// before subsequent calls to [`ExternalPrinter::print`] will block. pub fn with_capacity(capacity: usize) -> Self { let (sender, receiver) = bounded(capacity); Self { sender, receiver } } + /// Returns a new [`ExternalPrinter`] which can be used in other threads to queue messages to print pub fn sender(&self) -> ExternalPrinter { ExternalPrinter(self.sender.clone()) } pub(crate) fn messages(&self) -> Vec { + // TODO: refactor to use `self.receiver.try_iter()` let mut messages = Vec::new(); loop { match self.receiver.try_recv() { @@ -55,6 +70,11 @@ impl Default for ExternalPrinterChannel { } } +/// An [`ExternalPrinter`] queue messages for printing by sending them to an [`ExternalPrinterChannel`] +/// +/// [`ExternalPrinter`] are created through [`ExternalPrinterChannel::sender`] +/// or by cloning an existing [`ExternalPrinter`]. +/// /// An external printer allows one to print messages of text while editing a line. /// The message is printed as a new line, and the line-edit will continue below the output. /// @@ -66,15 +86,38 @@ pub struct ExternalPrinter(Sender); impl ExternalPrinter { /// Queues a string message to be printed /// - /// This function blocks if the underlying channel is full. + /// This will block if the corresponding [`ExternalPrinterChannel`] is full. + /// Also, if the channel has been dropped, + /// then the `String` message that would have been sent is returned as an `Err`. + /// + /// To print any type that implements [`Display`] use [`display`](Self::display). pub fn print(&self, string: impl Into) -> Result<(), String> { self.0.send(string.into()).map_err(|SendError(s)| s) } - /// Queues a value to be printed via its [`Display`] implementation + /// Queues a value to be printed, using the result of its [`Display`] implementation as the message /// - /// This function blocks if the underlying channel is full. + /// This will block if the corresponding [`ExternalPrinterChannel`] is full. + /// Also, if the channel has been dropped, + /// then the `String` message that would have been sent is returned as an `Err`. + /// + /// If `T` is a string-like type, use [`print`](Self::display) instead, + /// since it can be more efficient. pub fn display(&self, value: &T) -> Result<(), String> { self.print(value.to_string()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn thread_safe() { + fn send_sync(_: &T) {} + + let channel = ExternalPrinterChannel::new(); + send_sync(&channel); + send_sync(&channel.sender()) + } +} From fda063b8030bd8eeca56fea8926d152cdec5f292 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Mon, 29 Jan 2024 11:31:26 -0500 Subject: [PATCH 4/8] Refactor using `Receiver::try_iter` --- src/engine.rs | 5 ++--- src/external_printer.rs | 22 +++------------------- src/painting/painter.rs | 35 ++++++++++++++++++++--------------- 3 files changed, 25 insertions(+), 37 deletions(-) diff --git a/src/engine.rs b/src/engine.rs index aae47239..35f6add6 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -677,10 +677,9 @@ impl Reedline { #[cfg(feature = "external_printer")] if let Some(channel) = &self.external_printer_channel { // get messages from printer as crlf separated "lines" - let messages = channel.messages(); - if !messages.is_empty() { + if !channel.receiver().is_empty() { self.painter.print_external_message( - messages, + channel, self.editor.line_buffer(), prompt, )?; diff --git a/src/external_printer.rs b/src/external_printer.rs index d1c121d7..16b0fc58 100644 --- a/src/external_printer.rs +++ b/src/external_printer.rs @@ -1,6 +1,6 @@ use std::fmt::Display; -use crossbeam::channel::{bounded, Receiver, SendError, Sender, TryRecvError}; +use crossbeam::channel::{bounded, Receiver, SendError, Sender}; /// The channel to which external messages can be sent /// @@ -43,24 +43,8 @@ impl ExternalPrinterChannel { ExternalPrinter(self.sender.clone()) } - pub(crate) fn messages(&self) -> Vec { - // TODO: refactor to use `self.receiver.try_iter()` - let mut messages = Vec::new(); - loop { - match self.receiver.try_recv() { - Ok(string) => { - messages.extend(string.lines().map(String::from)); - } - Err(TryRecvError::Empty) => { - break; - } - Err(TryRecvError::Disconnected) => { - debug_assert!(false); // there is always one sender in `self`. - break; - } - } - } - messages + pub(crate) fn receiver(&self) -> &Receiver { + &self.receiver } } diff --git a/src/painting/painter.rs b/src/painting/painter.rs index 4986a961..e3d0ce7d 100644 --- a/src/painting/painter.rs +++ b/src/painting/painter.rs @@ -16,7 +16,10 @@ use { std::io::{Result, Write}, }; #[cfg(feature = "external_printer")] -use {crate::LineBuffer, crossterm::cursor::MoveUp}; +use { + crate::{ExternalPrinterChannel, LineBuffer}, + crossterm::cursor::MoveUp, +}; // Returns a string that skips N number of lines with the next offset of lines // An offset of 0 would return only one line after skipping the required lines @@ -495,7 +498,7 @@ impl Painter { #[cfg(feature = "external_printer")] pub(crate) fn print_external_message( &mut self, - messages: Vec, + channel: &ExternalPrinterChannel, line_buffer: &LineBuffer, prompt: &dyn Prompt, ) -> Result<()> { @@ -523,19 +526,21 @@ impl Painter { self.stdout.queue(MoveUp(buffer_num_lines - 1))?; } let erase_line = format!("\r{}\r", " ".repeat(self.screen_width().into())); - for line in messages { - self.stdout.queue(Print(&erase_line))?; - // Note: we don't use `print_line` here because we don't want to - // flush right now. The subsequent repaint of the prompt will cause - // immediate flush anyways. And if we flush here, every external - // print causes visible flicker. - self.stdout.queue(Print(line))?.queue(Print("\r\n"))?; - let new_start = self.prompt_start_row.saturating_add(1); - let height = self.screen_height(); - if new_start >= height { - self.prompt_start_row = height - 1; - } else { - self.prompt_start_row = new_start; + for messsage in channel.receiver().try_iter() { + for line in messsage.lines() { + self.stdout.queue(Print(&erase_line))?; + // Note: we don't use `print_line` here because we don't want to + // flush right now. The subsequent repaint of the prompt will cause + // immediate flush anyways. And if we flush here, every external + // print causes visible flicker. + self.stdout.queue(Print(line))?.queue(Print("\r\n"))?; + let new_start = self.prompt_start_row.saturating_add(1); + let height = self.screen_height(); + if new_start >= height { + self.prompt_start_row = height - 1; + } else { + self.prompt_start_row = new_start; + } } } Ok(()) From 2baf0ffc0381503d6e89144866e2f46e21757ab9 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Mon, 29 Jan 2024 16:43:20 -0500 Subject: [PATCH 5/8] Allow sending raw bytes --- src/external_printer.rs | 43 +++++++++++++++++++++++++++++++++++------ src/painting/painter.rs | 28 ++++++++++++++++++++------- 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/external_printer.rs b/src/external_printer.rs index 16b0fc58..988f6be3 100644 --- a/src/external_printer.rs +++ b/src/external_printer.rs @@ -13,8 +13,8 @@ use crossbeam::channel::{bounded, Receiver, SendError, Sender}; /// `external_printer` #[derive(Debug)] pub struct ExternalPrinterChannel { - sender: Sender, - receiver: Receiver, + sender: Sender>, + receiver: Receiver>, } impl ExternalPrinterChannel { @@ -43,7 +43,7 @@ impl ExternalPrinterChannel { ExternalPrinter(self.sender.clone()) } - pub(crate) fn receiver(&self) -> &Receiver { + pub(crate) fn receiver(&self) -> &Receiver> { &self.receiver } } @@ -65,9 +65,21 @@ impl Default for ExternalPrinterChannel { /// ## Required feature: /// `external_printer` #[derive(Debug, Clone)] -pub struct ExternalPrinter(Sender); +pub struct ExternalPrinter(Sender>); impl ExternalPrinter { + /// Queues a `Vec` of bytes to be written + /// + /// This will block if the corresponding [`ExternalPrinterChannel`] is full. + /// Also, if the channel has been dropped, + /// then the `Vec` of bytes that would have been sent are returned as an `Err`. + /// + /// Sending raw bytes allows non-UTF-8 byte sequences to be printed. + /// To send a UTF-8 `String`, prefer [`print`](Self::print) instead. + pub fn write(&self, bytes: Vec) -> Result<(), Vec> { + self.0.send(bytes).map_err(SendError::into_inner) + } + /// Queues a string message to be printed /// /// This will block if the corresponding [`ExternalPrinterChannel`] is full. @@ -76,7 +88,9 @@ impl ExternalPrinter { /// /// To print any type that implements [`Display`] use [`display`](Self::display). pub fn print(&self, string: impl Into) -> Result<(), String> { - self.0.send(string.into()).map_err(|SendError(s)| s) + self.0 + .send(string.into().into_bytes()) + .map_err(|SendError(bytes)| String::from_utf8(bytes).expect("valid utf-8")) } /// Queues a value to be printed, using the result of its [`Display`] implementation as the message @@ -85,7 +99,7 @@ impl ExternalPrinter { /// Also, if the channel has been dropped, /// then the `String` message that would have been sent is returned as an `Err`. /// - /// If `T` is a string-like type, use [`print`](Self::display) instead, + /// If `T` is a string-like type, use [`print`](Self::print) instead, /// since it can be more efficient. pub fn display(&self, value: &T) -> Result<(), String> { self.print(value.to_string()) @@ -104,4 +118,21 @@ mod tests { send_sync(&channel); send_sync(&channel.sender()) } + + #[test] + fn receives_message() { + let channel = ExternalPrinterChannel::new(); + let sender = channel.sender(); + assert!(sender.print("some text").is_ok()); + assert_eq!(channel.receiver().recv(), Ok("some text".into())) + } + + #[test] + fn print_error_does_not_panic() { + let channel = ExternalPrinterChannel::new(); + let sender = channel.sender(); + drop(channel); + let res = sender.print("some text"); + assert_eq!(res, Err("some text".into())) + } } diff --git a/src/painting/painter.rs b/src/painting/painter.rs index e3d0ce7d..1f88666c 100644 --- a/src/painting/painter.rs +++ b/src/painting/painter.rs @@ -525,15 +525,29 @@ impl Painter { if buffer_num_lines > 1 { self.stdout.queue(MoveUp(buffer_num_lines - 1))?; } + let erase_line = format!("\r{}\r", " ".repeat(self.screen_width().into())); - for messsage in channel.receiver().try_iter() { - for line in messsage.lines() { + for message in channel.receiver().try_iter() { + for line in message.split_inclusive(|&b| b == b'\n') { + let line = line + .strip_suffix(&[b'\n']) + .map(|line| line.strip_suffix(&[b'\r']).unwrap_or(line)) + .unwrap_or(line); + self.stdout.queue(Print(&erase_line))?; - // Note: we don't use `print_line` here because we don't want to - // flush right now. The subsequent repaint of the prompt will cause - // immediate flush anyways. And if we flush here, every external - // print causes visible flicker. - self.stdout.queue(Print(line))?.queue(Print("\r\n"))?; + + // Note: we don't flush here. + // The subsequent repaint of the prompt will cause immediate flush anyways. + // And if we flush here, every external print causes visible flicker. + // + // crossterm's `Print` command return true for `is_ansi_code_supported`. + // This means crossterm will use `Print`'s implementation of `Command::write_ansi` + // without doing any special handling for ANSI sequences. + // So, it's ok for us to do something similar by calling `write_all` directly + // without any special handling. + self.stdout.write_all(line)?; // self.stdout.queue(Print(line))?; + self.stdout.queue(Print("\r\n"))?; + let new_start = self.prompt_start_row.saturating_add(1); let height = self.screen_height(); if new_start >= height { From 1a548ca51e599a27a2408ceb895980d2f261c8f9 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Tue, 30 Jan 2024 17:59:08 -0500 Subject: [PATCH 6/8] Remove old comment --- src/engine.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/engine.rs b/src/engine.rs index 35f6add6..c29822dd 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -676,7 +676,6 @@ impl Reedline { #[cfg(feature = "external_printer")] if let Some(channel) = &self.external_printer_channel { - // get messages from printer as crlf separated "lines" if !channel.receiver().is_empty() { self.painter.print_external_message( channel, From f5b7290a6f8e70ed49a7392df92c55479df558d6 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Wed, 31 Jan 2024 18:04:18 -0500 Subject: [PATCH 7/8] Fix empty lines being skipped --- src/painting/painter.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/painting/painter.rs b/src/painting/painter.rs index 1f88666c..3bde5842 100644 --- a/src/painting/painter.rs +++ b/src/painting/painter.rs @@ -527,7 +527,11 @@ impl Painter { } let erase_line = format!("\r{}\r", " ".repeat(self.screen_width().into())); - for message in channel.receiver().try_iter() { + for mut message in channel.receiver().try_iter() { + // add a new line for next message + // messages that already end in '\n' will have a blank line between it and the next message. + message.push(b'\n'); + for line in message.split_inclusive(|&b| b == b'\n') { let line = line .strip_suffix(&[b'\n']) @@ -540,7 +544,7 @@ impl Painter { // The subsequent repaint of the prompt will cause immediate flush anyways. // And if we flush here, every external print causes visible flicker. // - // crossterm's `Print` command return true for `is_ansi_code_supported`. + // crossterm's `Print` command returns true for `is_ansi_code_supported`. // This means crossterm will use `Print`'s implementation of `Command::write_ansi` // without doing any special handling for ANSI sequences. // So, it's ok for us to do something similar by calling `write_all` directly From 5ddee21eee371be7e066d9487403203511e1369a Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Thu, 1 Feb 2024 14:26:37 -0500 Subject: [PATCH 8/8] Use `std::sync::mpsc` instead of `crossbeam-channel` In Rust 1.67, `crossbeam-channel` was merged into `std::sync::mpsc`. --- Cargo.lock | 57 ------------------ Cargo.toml | 3 +- examples/external_printer.rs | 25 ++++---- src/engine.rs | 33 +++++----- src/external_printer.rs | 114 +++++++---------------------------- src/lib.rs | 2 +- src/painting/painter.rs | 22 ++++--- 7 files changed, 66 insertions(+), 190 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e21b60c..63620001 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -146,62 +146,6 @@ version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06ea2b9bc92be3c2baa9334a323ebca2d6f074ff852cd1d7b11064035cd3868f" -[[package]] -name = "crossbeam" -version = "0.8.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1137cd7e7fc0fb5d3c5a8678be38ec56e819125d8d7907411fe24ccb943faca8" -dependencies = [ - "crossbeam-channel", - "crossbeam-deque", - "crossbeam-epoch", - "crossbeam-queue", - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-channel" -version = "0.5.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "176dc175b78f56c0f321911d9c8eb2b77a78a4860b9c19db83835fea1a46649b" -dependencies = [ - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-deque" -version = "0.8.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "613f8cc01fe9cf1a3eb3d7f488fd2fa8388403e97039e2f73692932e291a770d" -dependencies = [ - "crossbeam-epoch", - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-epoch" -version = "0.9.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b82ac4a3c2ca9c3460964f020e1402edd5753411d7737aa39c3714ad1b5420e" -dependencies = [ - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-queue" -version = "0.3.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "df0346b5d5e76ac2fe4e327c5fd1118d6be7c51dfb18f9b7922923f287471e35" -dependencies = [ - "crossbeam-utils", -] - -[[package]] -name = "crossbeam-utils" -version = "0.8.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "248e3bacc7dc6baa3b21e405ee045c3047101a49145e7e9eca583ab4c2ca5345" - [[package]] name = "crossterm" version = "0.27.0" @@ -721,7 +665,6 @@ version = "0.28.0" dependencies = [ "arboard", "chrono", - "crossbeam", "crossterm", "fd-lock", "gethostname 0.4.3", diff --git a/Cargo.toml b/Cargo.toml index 1b392cad..28a009da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,6 @@ chrono = { version = "0.4.19", default-features = false, features = [ "clock", "serde", ] } -crossbeam = { version = "0.8.2", optional = true } crossterm = { version = "0.27.0", features = ["serde"] } fd-lock = "3.0.3" itertools = "0.12.0" @@ -41,7 +40,7 @@ tempfile = "3.3.0" [features] bashisms = [] -external_printer = ["crossbeam"] +external_printer = [] sqlite = ["rusqlite/bundled", "serde_json"] sqlite-dynlib = ["rusqlite", "serde_json"] system_clipboard = ["arboard"] diff --git a/examples/external_printer.rs b/examples/external_printer.rs index f52d4406..bbac7843 100644 --- a/examples/external_printer.rs +++ b/examples/external_printer.rs @@ -3,47 +3,46 @@ // cargo run --example external_printer --features=external_printer use { - reedline::{DefaultPrompt, ExternalPrinterChannel, Reedline, Signal}, + reedline::{DefaultPrompt, ExternalPrinter, Reedline, Signal}, std::thread, std::thread::sleep, std::time::Duration, }; fn main() { - // Create a channel for the external printer. - let channel = ExternalPrinterChannel::default(); + let printer = ExternalPrinter::new(); - // Get a clone of the channel sender to use in a separate thread. - let printer = channel.sender(); + // Get a clone of the sender to use in a separate thread. + let sender = printer.sender(); - // Note that the senders/printers can also be cloned. - // let printer_clone = printer.clone(); + // Note that the senders can also be cloned. + // let sender_clone = sender.clone(); // external printer that prints a message every second thread::spawn(move || { let mut i = 1; loop { sleep(Duration::from_secs(1)); - assert!(printer - .print(format!("Message {i} delivered.\nWith two lines!")) + assert!(sender + .send(format!("Message {i} delivered.\nWith two lines!").into()) .is_ok()); i += 1; } }); - let printer = channel.sender(); + let sender = printer.sender(); // external printer that prints a bunch of messages after 3 seconds thread::spawn(move || { sleep(Duration::from_secs(3)); for _ in 0..10 { sleep(Duration::from_millis(1)); - assert!(printer.print("Hello!").is_ok()); + assert!(sender.send("Hello!".into()).is_ok()); } }); - // create a `Reedline` struct and assign the channel for the external printer - let mut line_editor = Reedline::create().with_external_printer(channel); + // create a `Reedline` struct and assign the external printer + let mut line_editor = Reedline::create().with_external_printer(printer); let prompt = DefaultPrompt::default(); loop { diff --git a/src/engine.rs b/src/engine.rs index c29822dd..cebd9932 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -4,7 +4,7 @@ use itertools::Itertools; use nu_ansi_term::{Color, Style}; #[cfg(feature = "external_printer")] -use crate::ExternalPrinterChannel; +use crate::ExternalPrinter; use crate::{enums::ReedlineRawEvent, CursorConfig}; #[cfg(feature = "bashisms")] use crate::{ @@ -150,7 +150,7 @@ pub struct Reedline { kitty_protocol: KittyProtocolGuard, #[cfg(feature = "external_printer")] - external_printer_channel: Option, + external_printer: Option, } struct BufferEditor { @@ -221,7 +221,7 @@ impl Reedline { bracketed_paste: BracketedPasteGuard::default(), kitty_protocol: KittyProtocolGuard::default(), #[cfg(feature = "external_printer")] - external_printer_channel: None, + external_printer: None, } } @@ -675,13 +675,14 @@ impl Reedline { let mut paste_enter_state = false; #[cfg(feature = "external_printer")] - if let Some(channel) = &self.external_printer_channel { - if !channel.receiver().is_empty() { - self.painter.print_external_message( - channel, - self.editor.line_buffer(), - prompt, - )?; + if let Some(printer) = &self.external_printer { + let any_messages = self.painter.print_external_messages( + printer.receiver(), + self.editor.line_buffer(), + prompt, + )?; + + if any_messages { self.repaint(prompt)?; } } @@ -1720,22 +1721,22 @@ impl Reedline { ) } - /// Returns a reference to the external printer's channel, or none if one was not set + /// Returns a reference to the external printer, or none if one was not set /// /// ## Required feature: /// `external_printer` #[cfg(feature = "external_printer")] - pub fn external_printer(&self) -> Option<&ExternalPrinterChannel> { - self.external_printer_channel.as_ref() + pub fn external_printer(&self) -> Option<&ExternalPrinter> { + self.external_printer.as_ref() } - /// Adds an external printer + /// Add a new external printer, or overwrite the existing one /// /// ## Required feature: /// `external_printer` #[cfg(feature = "external_printer")] - pub fn with_external_printer(mut self, channel: ExternalPrinterChannel) -> Self { - self.external_printer_channel = Some(channel); + pub fn with_external_printer(mut self, printer: ExternalPrinter) -> Self { + self.external_printer = Some(printer); self } diff --git a/src/external_printer.rs b/src/external_printer.rs index 988f6be3..36f55a15 100644 --- a/src/external_printer.rs +++ b/src/external_printer.rs @@ -1,46 +1,39 @@ -use std::fmt::Display; +use std::sync::mpsc::{self, Receiver, SyncSender}; -use crossbeam::channel::{bounded, Receiver, SendError, Sender}; - -/// The channel to which external messages can be sent -/// -/// Use the [`sender`](Self::sender) to create [`ExternalPrinter`]s for use in other threads. -/// /// An external printer allows one to print messages of text while editing a line. /// The message is printed as a new line, and the line-edit will continue below the output. /// +/// Use [`sender`](Self::sender) to receive a [`SyncSender`] for use in other threads. +/// /// ## Required feature: /// `external_printer` #[derive(Debug)] -pub struct ExternalPrinterChannel { - sender: Sender>, +pub struct ExternalPrinter { + sender: SyncSender>, receiver: Receiver>, } -impl ExternalPrinterChannel { +impl ExternalPrinter { /// The default maximum number of lines that can be queued up for printing - /// - /// If the channel is full, calls to [`ExternalPrinter::print`] will block - /// and wait for the channel to have spare capacity again. pub const DEFAULT_CAPACITY: usize = 20; - /// Create a new `ExternalPrinterChannel` with default capacity + /// Create a new `ExternalPrinter` with the [default capacity](Self::DEFAULT_CAPACITY) pub fn new() -> Self { Self::default() } - /// Create a new `ExternalPrinterChannel` with the given capacity + /// Create a new `ExternalPrinter` with the given capacity /// /// The capacity determines the maximum number of lines that can be queued up for printing - /// before subsequent calls to [`ExternalPrinter::print`] will block. + /// before subsequent [`send`](SyncSender::send) calls on the [`sender`](Self::sender) will block. pub fn with_capacity(capacity: usize) -> Self { - let (sender, receiver) = bounded(capacity); + let (sender, receiver) = mpsc::sync_channel(capacity); Self { sender, receiver } } - /// Returns a new [`ExternalPrinter`] which can be used in other threads to queue messages to print - pub fn sender(&self) -> ExternalPrinter { - ExternalPrinter(self.sender.clone()) + /// Returns a new [`SyncSender`] which can be used in other threads to queue messages to print + pub fn sender(&self) -> SyncSender> { + self.sender.clone() } pub(crate) fn receiver(&self) -> &Receiver> { @@ -48,91 +41,30 @@ impl ExternalPrinterChannel { } } -impl Default for ExternalPrinterChannel { +impl Default for ExternalPrinter { fn default() -> Self { Self::with_capacity(Self::DEFAULT_CAPACITY) } } -/// An [`ExternalPrinter`] queue messages for printing by sending them to an [`ExternalPrinterChannel`] -/// -/// [`ExternalPrinter`] are created through [`ExternalPrinterChannel::sender`] -/// or by cloning an existing [`ExternalPrinter`]. -/// -/// An external printer allows one to print messages of text while editing a line. -/// The message is printed as a new line, and the line-edit will continue below the output. -/// -/// ## Required feature: -/// `external_printer` -#[derive(Debug, Clone)] -pub struct ExternalPrinter(Sender>); - -impl ExternalPrinter { - /// Queues a `Vec` of bytes to be written - /// - /// This will block if the corresponding [`ExternalPrinterChannel`] is full. - /// Also, if the channel has been dropped, - /// then the `Vec` of bytes that would have been sent are returned as an `Err`. - /// - /// Sending raw bytes allows non-UTF-8 byte sequences to be printed. - /// To send a UTF-8 `String`, prefer [`print`](Self::print) instead. - pub fn write(&self, bytes: Vec) -> Result<(), Vec> { - self.0.send(bytes).map_err(SendError::into_inner) - } - - /// Queues a string message to be printed - /// - /// This will block if the corresponding [`ExternalPrinterChannel`] is full. - /// Also, if the channel has been dropped, - /// then the `String` message that would have been sent is returned as an `Err`. - /// - /// To print any type that implements [`Display`] use [`display`](Self::display). - pub fn print(&self, string: impl Into) -> Result<(), String> { - self.0 - .send(string.into().into_bytes()) - .map_err(|SendError(bytes)| String::from_utf8(bytes).expect("valid utf-8")) - } - - /// Queues a value to be printed, using the result of its [`Display`] implementation as the message - /// - /// This will block if the corresponding [`ExternalPrinterChannel`] is full. - /// Also, if the channel has been dropped, - /// then the `String` message that would have been sent is returned as an `Err`. - /// - /// If `T` is a string-like type, use [`print`](Self::print) instead, - /// since it can be more efficient. - pub fn display(&self, value: &T) -> Result<(), String> { - self.print(value.to_string()) - } -} - #[cfg(test)] mod tests { use super::*; #[test] - fn thread_safe() { - fn send_sync(_: &T) {} + fn impls_send() { + fn impls_send(_: &T) {} - let channel = ExternalPrinterChannel::new(); - send_sync(&channel); - send_sync(&channel.sender()) + let printer = ExternalPrinter::new(); + impls_send(&printer); + impls_send(&printer.sender()) } #[test] fn receives_message() { - let channel = ExternalPrinterChannel::new(); - let sender = channel.sender(); - assert!(sender.print("some text").is_ok()); - assert_eq!(channel.receiver().recv(), Ok("some text".into())) - } - - #[test] - fn print_error_does_not_panic() { - let channel = ExternalPrinterChannel::new(); - let sender = channel.sender(); - drop(channel); - let res = sender.print("some text"); - assert_eq!(res, Err("some text".into())) + let printer = ExternalPrinter::new(); + let sender = printer.sender(); + assert!(sender.send(b"some text".into()).is_ok()); + assert_eq!(printer.receiver().recv(), Ok(b"some text".into())) } } diff --git a/src/lib.rs b/src/lib.rs index 3a0d9dd5..c408151e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -287,7 +287,7 @@ pub use terminal_extensions::kitty_protocol_available; #[cfg(feature = "external_printer")] mod external_printer; #[cfg(feature = "external_printer")] -pub use external_printer::{ExternalPrinter, ExternalPrinterChannel}; +pub use external_printer::ExternalPrinter; mod utils; pub use utils::{ diff --git a/src/painting/painter.rs b/src/painting/painter.rs index 3bde5842..ab683581 100644 --- a/src/painting/painter.rs +++ b/src/painting/painter.rs @@ -16,10 +16,7 @@ use { std::io::{Result, Write}, }; #[cfg(feature = "external_printer")] -use { - crate::{ExternalPrinterChannel, LineBuffer}, - crossterm::cursor::MoveUp, -}; +use {crate::LineBuffer, crossterm::cursor::MoveUp, std::sync::mpsc::Receiver}; // Returns a string that skips N number of lines with the next offset of lines // An offset of 0 would return only one line after skipping the required lines @@ -491,17 +488,22 @@ impl Painter { self.stdout.flush() } - /// Prints an external message + /// Prints external messages from a channel receiver, returning true if there were any messages /// /// This function doesn't flush the buffer. So buffer should be flushed /// afterwards perhaps by repainting the prompt via `repaint_buffer()`. #[cfg(feature = "external_printer")] - pub(crate) fn print_external_message( + pub(crate) fn print_external_messages( &mut self, - channel: &ExternalPrinterChannel, + receiver: &Receiver>, line_buffer: &LineBuffer, prompt: &dyn Prompt, - ) -> Result<()> { + ) -> Result { + let mut messages = receiver.try_iter().peekable(); + if messages.peek().is_none() { + return Ok(false); + } + // adding 3 seems to be right for first line-wrap let prompt_len = prompt.render_prompt_right().len() + 3; let mut buffer_num_lines = 0_u16; @@ -527,7 +529,7 @@ impl Painter { } let erase_line = format!("\r{}\r", " ".repeat(self.screen_width().into())); - for mut message in channel.receiver().try_iter() { + for mut message in messages { // add a new line for next message // messages that already end in '\n' will have a blank line between it and the next message. message.push(b'\n'); @@ -561,7 +563,7 @@ impl Painter { } } } - Ok(()) + Ok(true) } /// Queue scroll of `num` lines to `self.stdout`.