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

@@ -50,34 +50,23 @@ impl Clipboard for LocalClipboard {
     }
 }
 
+/// Creates a local clipboard
+pub fn get_local_clipboard() -> Box<dyn Clipboard> {
+    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<dyn Clipboard> {
+pub fn get_system_clipboard() -> Box<dyn Clipboard> {
     SystemClipboard::new().map_or_else(
         |_e| Box::new(LocalClipboard::new()) as Box<dyn Clipboard>,
         |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<dyn Clipboard> {
-    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;
 

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<dyn Clipboard>,
+    #[cfg(feature = "system_clipboard")]
+    system_clipboard: Box<dyn Clipboard>,
     edit_stack: EditStack<LineBuffer>,
     last_undo_behavior: UndoBehavior,
     selection_anchor: Option<usize>,
@@ -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);
+        }
+    }
 }

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;

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),
     );
 }
 

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,
         }
     }
 }