From dc7063ea4260b7a74ad055f9c34ed14a70333afe Mon Sep 17 00:00:00 2001 From: Tastaturtaste <31124715+Tastaturtaste@users.noreply.github.com> Date: Tue, 12 Mar 2024 20:40:08 +0100 Subject: [PATCH 1/2] Use the OS clipboard only for explicit cut/copy/paste operations (#761) * Use the system-clipboard only for explicit cut/copy/paste operation * Update reedline to use the system-clipboard only for explicit cut/copy/paste operation * Use separate variants to differentiate between local cut buffer and system clipboard. Compile out all system clipboard functionality statically if feature is not active. --- src/core_editor/clip_buffer.rs | 52 ++++++++------- src/core_editor/editor.rs | 111 ++++++++++++++++++++++++++------- src/core_editor/mod.rs | 4 +- src/edit_mode/keybindings.rs | 9 ++- src/enums.rs | 36 +++++++++-- 5 files changed, 162 insertions(+), 50 deletions(-) diff --git a/src/core_editor/clip_buffer.rs b/src/core_editor/clip_buffer.rs index 4a74664b..8297a3bb 100644 --- a/src/core_editor/clip_buffer.rs +++ b/src/core_editor/clip_buffer.rs @@ -50,34 +50,23 @@ impl Clipboard for LocalClipboard { } } +/// Creates a local clipboard +pub fn get_local_clipboard() -> Box { + Box::new(LocalClipboard::new()) +} + #[cfg(feature = "system_clipboard")] pub use system_clipboard::SystemClipboard; +/// Creates a handle for the OS clipboard #[cfg(feature = "system_clipboard")] -/// Helper to get a clipboard based on the `system_clipboard` feature flag: -/// -/// Enabled -> [`SystemClipboard`], which talks to the system. If the system clipboard can't be -/// accessed, it will default to [`LocalClipboard`]. -/// -/// Disabled -> [`LocalClipboard`], which supports cutting and pasting limited to the [`crate::Reedline`] instance -pub fn get_default_clipboard() -> Box { +pub fn get_system_clipboard() -> Box { SystemClipboard::new().map_or_else( |_e| Box::new(LocalClipboard::new()) as Box, |cb| Box::new(cb), ) } -#[cfg(not(feature = "system_clipboard"))] -/// Helper to get a clipboard based on the `system_clipboard` feature flag: -/// -/// Enabled -> `SystemClipboard`, which talks to the system. If the system clipboard can't be -/// accessed, it will default to [`LocalClipboard`]. -/// -/// Disabled -> [`LocalClipboard`], which supports cutting and pasting limited to the [`crate::Reedline`] instance -pub fn get_default_clipboard() -> Box { - Box::new(LocalClipboard::new()) -} - #[cfg(feature = "system_clipboard")] mod system_clipboard { use super::*; @@ -124,10 +113,31 @@ mod system_clipboard { #[cfg(test)] mod tests { - use super::{get_default_clipboard, ClipboardMode}; + #[cfg(feature = "system_clipboard")] + use super::get_system_clipboard; + use super::{get_local_clipboard, ClipboardMode}; + #[test] + fn reads_back_local() { + let mut cb = get_local_clipboard(); + // If the system clipboard is used we want to persist it for the user + let previous_state = cb.get().0; + + // Actual test + cb.set("test", ClipboardMode::Normal); + assert_eq!(cb.len(), 4); + assert_eq!(cb.get().0, "test".to_owned()); + cb.clear(); + assert_eq!(cb.get().0, String::new()); + + // Restore! + + cb.set(&previous_state, ClipboardMode::Normal); + } + + #[cfg(feature = "system_clipboard")] #[test] - fn reads_back() { - let mut cb = get_default_clipboard(); + fn reads_back_system() { + let mut cb = get_system_clipboard(); // If the system clipboard is used we want to persist it for the user let previous_state = cb.get().0; diff --git a/src/core_editor/editor.rs b/src/core_editor/editor.rs index c39e6550..227606eb 100644 --- a/src/core_editor/editor.rs +++ b/src/core_editor/editor.rs @@ -1,6 +1,9 @@ use super::{edit_stack::EditStack, Clipboard, ClipboardMode, LineBuffer}; +#[cfg(feature = "system_clipboard")] +use crate::core_editor::get_system_clipboard; use crate::enums::{EditType, UndoBehavior}; -use crate::{core_editor::get_default_clipboard, EditCommand}; +use crate::{core_editor::get_local_clipboard, EditCommand}; +use std::ops::DerefMut; /// Stateful editor executing changes to the underlying [`LineBuffer`] /// @@ -9,6 +12,8 @@ use crate::{core_editor::get_default_clipboard, EditCommand}; pub struct Editor { line_buffer: LineBuffer, cut_buffer: Box, + #[cfg(feature = "system_clipboard")] + system_clipboard: Box, edit_stack: EditStack, last_undo_behavior: UndoBehavior, selection_anchor: Option, @@ -18,7 +23,9 @@ impl Default for Editor { fn default() -> Self { Editor { line_buffer: LineBuffer::new(), - cut_buffer: get_default_clipboard(), + cut_buffer: get_local_clipboard(), + #[cfg(feature = "system_clipboard")] + system_clipboard: get_system_clipboard(), edit_stack: EditStack::new(), last_undo_behavior: UndoBehavior::CreateUndoPoint, selection_anchor: None, @@ -110,8 +117,15 @@ impl Editor { self.move_left_until_char(*c, true, true, *select) } EditCommand::SelectAll => self.select_all(), - EditCommand::CutSelection => self.cut_selection(), - EditCommand::CopySelection => self.copy_selection(), + EditCommand::CutSelection => self.cut_selection_to_cut_buffer(), + EditCommand::CopySelection => self.copy_selection_to_cut_buffer(), + EditCommand::Paste => self.paste_cut_buffer(), + #[cfg(feature = "system_clipboard")] + EditCommand::CutSelectionSystem => self.cut_selection_to_system(), + #[cfg(feature = "system_clipboard")] + EditCommand::CopySelectionSystem => self.copy_selection_to_system(), + #[cfg(feature = "system_clipboard")] + EditCommand::PasteSystem => self.paste_from_system(), } if !matches!(command.edit_type(), EditType::MoveCursor { select: true }) { self.selection_anchor = None; @@ -390,21 +404,7 @@ impl Editor { fn insert_cut_buffer_before(&mut self) { self.delete_selection(); - match self.cut_buffer.get() { - (content, ClipboardMode::Normal) => { - self.line_buffer.insert_str(&content); - } - (mut content, ClipboardMode::Lines) => { - // TODO: Simplify that? - self.line_buffer.move_to_line_start(); - self.line_buffer.move_line_up(); - if !content.ends_with('\n') { - // TODO: Make sure platform requirements are met - content.push('\n'); - } - self.line_buffer.insert_str(&content); - } - } + insert_clipboard_content_before(&mut self.line_buffer, self.cut_buffer.deref_mut()) } fn insert_cut_buffer_after(&mut self) { @@ -526,7 +526,17 @@ impl Editor { self.line_buffer.move_to_end(); } - fn cut_selection(&mut self) { + #[cfg(feature = "system_clipboard")] + fn cut_selection_to_system(&mut self) { + if let Some((start, end)) = self.get_selection() { + let cut_slice = &self.line_buffer.get_buffer()[start..end]; + self.system_clipboard.set(cut_slice, ClipboardMode::Normal); + self.line_buffer.clear_range_safe(start, end); + self.selection_anchor = None; + } + } + + fn cut_selection_to_cut_buffer(&mut self) { if let Some((start, end)) = self.get_selection() { let cut_slice = &self.line_buffer.get_buffer()[start..end]; self.cut_buffer.set(cut_slice, ClipboardMode::Normal); @@ -535,7 +545,15 @@ impl Editor { } } - fn copy_selection(&mut self) { + #[cfg(feature = "system_clipboard")] + fn copy_selection_to_system(&mut self) { + if let Some((start, end)) = self.get_selection() { + let cut_slice = &self.line_buffer.get_buffer()[start..end]; + self.system_clipboard.set(cut_slice, ClipboardMode::Normal); + } + } + + fn copy_selection_to_cut_buffer(&mut self) { if let Some((start, end)) = self.get_selection() { let cut_slice = &self.line_buffer.get_buffer()[start..end]; self.cut_buffer.set(cut_slice, ClipboardMode::Normal); @@ -619,6 +637,35 @@ impl Editor { self.delete_selection(); self.line_buffer.insert_newline(); } + + #[cfg(feature = "system_clipboard")] + fn paste_from_system(&mut self) { + self.delete_selection(); + insert_clipboard_content_before(&mut self.line_buffer, self.system_clipboard.deref_mut()); + } + + fn paste_cut_buffer(&mut self) { + self.delete_selection(); + insert_clipboard_content_before(&mut self.line_buffer, self.cut_buffer.deref_mut()); + } +} + +fn insert_clipboard_content_before(line_buffer: &mut LineBuffer, clipboard: &mut dyn Clipboard) { + match clipboard.get() { + (content, ClipboardMode::Normal) => { + line_buffer.insert_str(&content); + } + (mut content, ClipboardMode::Lines) => { + // TODO: Simplify that? + line_buffer.move_to_line_start(); + line_buffer.move_line_up(); + if !content.ends_with('\n') { + // TODO: Make sure platform requirements are met + content.push('\n'); + } + line_buffer.insert_str(&content); + } + } } #[cfg(test)] @@ -829,4 +876,26 @@ mod test { editor.run_edit_command(&EditCommand::Undo); assert_eq!(editor.get_buffer(), "This \r\n is a test"); } + #[cfg(feature = "system_clipboard")] + mod without_system_clipboard { + use super::*; + #[test] + fn test_cut_selection_system() { + let mut editor = editor_with("This is a test!"); + editor.selection_anchor = Some(editor.line_buffer.len()); + editor.line_buffer.set_insertion_point(0); + editor.run_edit_command(&EditCommand::CutSelectionSystem); + assert!(editor.line_buffer.get_buffer().is_empty()); + } + #[test] + fn test_copypaste_selection_system() { + let s = "This is a test!"; + let mut editor = editor_with(s); + editor.selection_anchor = Some(editor.line_buffer.len()); + editor.line_buffer.set_insertion_point(0); + editor.run_edit_command(&EditCommand::CopySelectionSystem); + editor.run_edit_command(&EditCommand::PasteSystem); + pretty_assertions::assert_eq!(editor.line_buffer.len(), s.len() * 2); + } + } } diff --git a/src/core_editor/mod.rs b/src/core_editor/mod.rs index 2bdd7994..b30009bb 100644 --- a/src/core_editor/mod.rs +++ b/src/core_editor/mod.rs @@ -3,6 +3,8 @@ mod edit_stack; mod editor; mod line_buffer; -pub(crate) use clip_buffer::{get_default_clipboard, Clipboard, ClipboardMode}; +#[cfg(feature = "system_clipboard")] +pub(crate) use clip_buffer::get_system_clipboard; +pub(crate) use clip_buffer::{get_local_clipboard, Clipboard, ClipboardMode}; pub use editor::Editor; pub use line_buffer::LineBuffer; diff --git a/src/edit_mode/keybindings.rs b/src/edit_mode/keybindings.rs index bd9f95cd..e2e5e17f 100644 --- a/src/edit_mode/keybindings.rs +++ b/src/edit_mode/keybindings.rs @@ -214,20 +214,23 @@ pub fn add_common_edit_bindings(kb: &mut Keybindings) { // Base commands should not affect cut buffer kb.add_binding(KM::CONTROL, KC::Char('h'), edit_bind(EC::Backspace)); kb.add_binding(KM::CONTROL, KC::Char('w'), edit_bind(EC::BackspaceWord)); + #[cfg(feature = "system_clipboard")] kb.add_binding( KM::CONTROL | KM::SHIFT, KC::Char('x'), - edit_bind(EC::CutSelection), + edit_bind(EC::CutSelectionSystem), ); + #[cfg(feature = "system_clipboard")] kb.add_binding( KM::CONTROL | KM::SHIFT, KC::Char('c'), - edit_bind(EC::CopySelection), + edit_bind(EC::CopySelectionSystem), ); + #[cfg(feature = "system_clipboard")] kb.add_binding( KM::CONTROL | KM::SHIFT, KC::Char('v'), - edit_bind(EC::PasteCutBufferBefore), + edit_bind(EC::PasteSystem), ); } diff --git a/src/enums.rs b/src/enums.rs index 7ebaa343..b56896be 100644 --- a/src/enums.rs +++ b/src/enums.rs @@ -17,6 +17,7 @@ pub enum Signal { /// Editing actions which can be mapped to key bindings. /// /// Executed by `Reedline::run_edit_commands()` +#[non_exhaustive] #[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq, EnumIter)] pub enum EditCommand { /// Move to the start of the buffer @@ -257,11 +258,26 @@ pub enum EditCommand { /// Select whole input buffer SelectAll, - /// Cut selection + /// Cut selection to local buffer CutSelection, - /// Copy selection + /// Copy selection to local buffer CopySelection, + + /// Paste content from local buffer at the current cursor position + Paste, + + /// Cut selection to system clipboard + #[cfg(feature = "system_clipboard")] + CutSelectionSystem, + + /// Copy selection to system clipboard + #[cfg(feature = "system_clipboard")] + CopySelectionSystem, + + /// Paste content from system clipboard at the current cursor position + #[cfg(feature = "system_clipboard")] + PasteSystem, } impl Display for EditCommand { @@ -348,6 +364,13 @@ impl Display for EditCommand { EditCommand::SelectAll => write!(f, "SelectAll"), EditCommand::CutSelection => write!(f, "CutSelection"), EditCommand::CopySelection => write!(f, "CopySelection"), + EditCommand::Paste => write!(f, "Paste"), + #[cfg(feature = "system_clipboard")] + EditCommand::CutSelectionSystem => write!(f, "CutSelectionSystem"), + #[cfg(feature = "system_clipboard")] + EditCommand::CopySelectionSystem => write!(f, "CopySelectionSystem"), + #[cfg(feature = "system_clipboard")] + EditCommand::PasteSystem => write!(f, "PasteSystem"), } } } @@ -380,7 +403,6 @@ impl EditCommand { } EditCommand::SelectAll => EditType::MoveCursor { select: true }, - // Text edits EditCommand::InsertChar(_) | EditCommand::Backspace @@ -418,11 +440,17 @@ impl EditCommand { | EditCommand::CutRightBefore(_) | EditCommand::CutLeftUntil(_) | EditCommand::CutLeftBefore(_) - | EditCommand::CutSelection => EditType::EditText, + | EditCommand::CutSelection + | EditCommand::Paste => EditType::EditText, + + #[cfg(feature = "system_clipboard")] // Sadly cfg attributes in patterns don't work + EditCommand::CutSelectionSystem | EditCommand::PasteSystem => EditType::EditText, EditCommand::Undo | EditCommand::Redo => EditType::UndoRedo, EditCommand::CopySelection => EditType::NoOp, + #[cfg(feature = "system_clipboard")] + EditCommand::CopySelectionSystem => EditType::NoOp, } } } From 6957b5ab7b98c3f090131e9388c18c1826812314 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Tue, 19 Mar 2024 05:04:17 -0500 Subject: [PATCH 2/2] Revert "Move left when exiting insert mode (#699)" (#773) Simulating vim's cursor logic by moving the actual cursor breaks the history traversal. Thus revert. This reverts commit 02f551d42eeda2f277cb98416bb035529b2a91aa. --- src/edit_mode/vi/mod.rs | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/edit_mode/vi/mod.rs b/src/edit_mode/vi/mod.rs index c687a86b..4428c645 100644 --- a/src/edit_mode/vi/mod.rs +++ b/src/edit_mode/vi/mod.rs @@ -136,17 +136,8 @@ impl EditMode for Vi { } (_, KeyModifiers::NONE, KeyCode::Esc) => { self.cache.clear(); - - ReedlineEvent::Multiple(vec![ - if self.mode == ViMode::Insert { - self.mode = ViMode::Normal; - ReedlineEvent::Left - } else { - ReedlineEvent::None - }, - ReedlineEvent::Esc, - ReedlineEvent::Repaint, - ]) + self.mode = ViMode::Normal; + ReedlineEvent::Multiple(vec![ReedlineEvent::Esc, ReedlineEvent::Repaint]) } (_, KeyModifiers::NONE, KeyCode::Enter) => { self.mode = ViMode::Insert; @@ -197,11 +188,7 @@ mod test { assert_eq!( result, - ReedlineEvent::Multiple(vec![ - ReedlineEvent::Left, - ReedlineEvent::Esc, - ReedlineEvent::Repaint - ]) + ReedlineEvent::Multiple(vec![ReedlineEvent::Esc, ReedlineEvent::Repaint]) ); assert!(matches!(vi.mode, ViMode::Normal)); }