From 602ae328fa894311f19e90769b8ee5fc95d4b467 Mon Sep 17 00:00:00 2001 From: Gabriel Minatel Date: Fri, 1 May 2026 12:17:55 -0400 Subject: [PATCH] fix(helix): system clipboard clobbering --- assets/keymaps/vim.json | 2 +- crates/vim/src/helix.rs | 135 +++++++++++++++++++++++++++------- crates/vim/src/helix/paste.rs | 36 ++------- crates/vim/src/normal.rs | 4 + crates/vim/src/state.rs | 5 ++ 5 files changed, 125 insertions(+), 57 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 188ea2e483a..2e9e830dde6 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -456,7 +456,7 @@ "alt-.": "vim::RepeatFind", // Changes - "shift-r": "editor::Paste", + "shift-r": "vim::HelixReplaceWithYanked", "`": "vim::ConvertToLowerCase", "alt-`": "vim::ConvertToUpperCase", "insert": "vim::InsertBefore", // not a helix default diff --git a/crates/vim/src/helix.rs b/crates/vim/src/helix.rs index eebad4d4382..048a634b2ea 100644 --- a/crates/vim/src/helix.rs +++ b/crates/vim/src/helix.rs @@ -5,7 +5,7 @@ mod paste; mod select; mod surround; -use editor::display_map::{DisplayRow, DisplaySnapshot}; +use editor::display_map::{DisplayRow, DisplaySnapshot, ToDisplayPoint}; use editor::{ DisplayPoint, Editor, EditorSettings, HideMouseCursorOrigin, MultiBufferOffset, NavigationOverlayLabel, NavigationTargetOverlay, SelectionEffects, ToOffset, ToPoint, movement, @@ -57,6 +57,8 @@ actions!( HelixSubstitute, /// Delete the selection and enter edit mode, without yanking the selection. HelixSubstituteNoYank, + /// Replace the selection with the contents of the default register (`R`). + HelixReplaceWithYanked, /// Activate Helix-style word jump labels. HelixJumpToWord, /// Select the next match for the current search query. @@ -74,6 +76,7 @@ pub fn register(editor: &mut Editor, cx: &mut Context) { Vim::action(editor, cx, Vim::helix_yank); Vim::action(editor, cx, Vim::helix_goto_last_modification); Vim::action(editor, cx, Vim::helix_paste); + Vim::action(editor, cx, Vim::helix_replace_with_yanked); Vim::action(editor, cx, Vim::helix_select_regex); Vim::action(editor, cx, Vim::helix_keep_newest_selection); Vim::action(editor, cx, |vim, _: &HelixDuplicateBelow, window, cx| { @@ -571,6 +574,11 @@ impl Vim { .iter() .any(|selection| !selection.is_empty()); + // Plain `y` writes to register `"` only, never the system clipboard + if vim.selected_register.is_none() { + vim.selected_register = Some('"'); + } + if !has_selection { // If no selection, expand to current character (like 'v' does) editor.change_selections(Default::default(), window, cx, |s| { @@ -727,6 +735,62 @@ impl Vim { }); } + /// Replace each selection with the contents of the default + /// register (or an explicitly selected register) + pub fn helix_replace_with_yanked( + &mut self, + _: &HelixReplaceWithYanked, + window: &mut Window, + cx: &mut Context, + ) { + self.record_current_action(cx); + self.update_editor(cx, |vim, editor, cx| { + let selected_register = vim.selected_register.take(); + let Some(register) = Vim::update_globals(cx, |globals, cx| { + if selected_register.is_some() { + globals.read_register(selected_register, Some(editor), cx) + } else { + globals.read_default_register() + } + }) + .filter(|reg| !reg.text.is_empty()) else { + return; + }; + let text = register.text; + + editor.transact(window, cx, |editor, window, cx| { + let display_map = editor.display_snapshot(cx); + let selections = editor.selections.all_adjusted(&display_map); + + let snapshot = display_map.buffer_snapshot(); + let mut edits = Vec::with_capacity(selections.len()); + let mut new_anchors = Vec::with_capacity(selections.len()); + for selection in &selections { + let mut range = selection.start..selection.end; + if range.is_empty() { + range.end = movement::saturating_right( + &display_map, + range.end.to_display_point(&display_map), + ) + .to_point(&display_map); + } + new_anchors.push(snapshot.anchor_before(range.start)); + edits.push((range, text.to_string())); + } + editor.edit(edits, cx); + + let snapshot = editor.buffer().read(cx).snapshot(cx); + editor.change_selections(Default::default(), window, cx, |s| { + s.select_ranges(new_anchors.into_iter().map(|anchor| { + let start = anchor.to_offset(&snapshot); + start..start + text.len() + })); + }); + }); + }); + self.switch_mode(Mode::HelixNormal, true, window, cx); + } + pub fn helix_replace(&mut self, text: &str, window: &mut Window, cx: &mut Context) { self.update_editor(cx, |_, editor, cx| { editor.transact(window, cx, |editor, window, cx| { @@ -878,6 +942,9 @@ impl Vim { }) }); if yank { + if vim.selected_register.is_none() { + vim.selected_register = Some('"'); + } vim.copy_selections_content(editor, MotionKind::Exclusive, window, cx); } let selections = editor @@ -2465,54 +2532,66 @@ mod test { let mut cx = VimTestContext::new(cx, true).await; cx.enable_helix(); - // Test yanking current character with no selection + cx.write_to_clipboard(gpui::ClipboardItem::new_string("external".into())); + + // Test yanking a single character (no explicit selection) should not clobber the system clipboard cx.set_state("hello ˇworld", Mode::HelixNormal); cx.simulate_keystrokes("y"); - - // Test cursor remains at the same position after yanking single character cx.assert_state("hello ˇworld", Mode::HelixNormal); - cx.shared_clipboard().assert_eq("w"); + cx.shared_clipboard().assert_eq("external"); + cx.simulate_keystrokes("l p"); + cx.assert_state("hello wo«wˇ»rld", Mode::HelixNormal); - // Move cursor and yank another character - cx.simulate_keystrokes("l"); - cx.simulate_keystrokes("y"); - cx.shared_clipboard().assert_eq("o"); - - // Test yanking with existing selection + // Test yanking a multi-char selection cx.set_state("hello «worlˇ»d", Mode::HelixNormal); cx.simulate_keystrokes("y"); - cx.shared_clipboard().assert_eq("worl"); + cx.shared_clipboard().assert_eq("external"); cx.assert_state("hello «worlˇ»d", Mode::HelixNormal); - - // Test yanking in select mode character by character - cx.set_state("hello ˇworld", Mode::HelixNormal); - cx.simulate_keystroke("v"); - cx.assert_state("hello «wˇ»orld", Mode::HelixSelect); - cx.simulate_keystroke("y"); - cx.assert_state("hello «wˇ»orld", Mode::HelixNormal); - cx.shared_clipboard().assert_eq("w"); } #[gpui::test] - async fn test_shift_r_paste(cx: &mut gpui::TestAppContext) { + async fn test_helix_delete_preserves_system_clipboard(cx: &mut gpui::TestAppContext) { let mut cx = VimTestContext::new(cx, true).await; cx.enable_helix(); - // First copy some text to clipboard + cx.write_to_clipboard(gpui::ClipboardItem::new_string("external".into())); + cx.shared_clipboard().assert_eq("external"); + + cx.set_state("hello «worlˇ»d", Mode::HelixNormal); + cx.simulate_keystrokes("d"); + cx.shared_clipboard().assert_eq("external"); + + cx.set_state( + indoc! {" + ˇ + second line"}, + Mode::HelixNormal, + ); + cx.simulate_keystrokes("d"); + cx.shared_clipboard().assert_eq("external"); + } + + #[gpui::test] + #[gpui::test] + async fn test_shift_r_replace_with_yanked(cx: &mut gpui::TestAppContext) { + let mut cx = VimTestContext::new(cx, true).await; + cx.enable_helix(); + + // External clipboard content is irrelevant + cx.write_to_clipboard(gpui::ClipboardItem::new_string("external".into())); + cx.set_state("«hello worldˇ»", Mode::HelixNormal); cx.simulate_keystrokes("y"); - // Test paste with shift-r on single cursor + // Test bare cursor cx.set_state("foo ˇbar", Mode::HelixNormal); cx.simulate_keystrokes("shift-r"); + cx.assert_state("foo «hello worldˇ»ar", Mode::HelixNormal); - cx.assert_state("foo hello worldˇbar", Mode::HelixNormal); - - // Test paste with shift-r on selection + // Test multi-char selection cx.set_state("foo «barˇ» baz", Mode::HelixNormal); cx.simulate_keystrokes("shift-r"); - - cx.assert_state("foo hello worldˇ baz", Mode::HelixNormal); + cx.assert_state("foo «hello worldˇ» baz", Mode::HelixNormal); } #[gpui::test] diff --git a/crates/vim/src/helix/paste.rs b/crates/vim/src/helix/paste.rs index c4328142146..d73e978fa12 100644 --- a/crates/vim/src/helix/paste.rs +++ b/crates/vim/src/helix/paste.rs @@ -33,8 +33,13 @@ impl Vim { let selected_register = vim.selected_register.take(); + // With an explicit register, defer to read_register so `+`/`*` still hit the clipboard let Some(register) = Vim::update_globals(cx, |globals, cx| { - globals.read_register(selected_register, Some(editor), cx) + if selected_register.is_some() { + globals.read_register(selected_register, Some(editor), cx) + } else { + globals.read_default_register() + } }) .filter(|reg| !reg.text.is_empty()) else { return; @@ -154,7 +159,7 @@ mod test { use crate::{state::Mode, test::VimTestContext}; #[gpui::test] - async fn test_system_clipboard_paste(cx: &mut gpui::TestAppContext) { + async fn test_helix_paste_ignores_system_clipboard(cx: &mut gpui::TestAppContext) { let mut cx = VimTestContext::new(cx, true).await; cx.enable_helix(); cx.set_state( @@ -169,36 +174,11 @@ mod test { cx.simulate_keystrokes("p"); cx.assert_state( indoc! {" - The quic«clipboardˇ»k brown + The quiˇck brown fox jumps over the lazy dog."}, Mode::HelixNormal, ); - - // Multiple cursors with system clipboard (no metadata) pastes - // the same text at each cursor. - cx.set_state( - indoc! {" - ˇThe quick brown - fox ˇjumps over - the lazy dog."}, - Mode::HelixNormal, - ); - cx.write_to_clipboard(ClipboardItem::new_string("hi".to_string())); - cx.simulate_keystrokes("p"); - cx.assert_state( - indoc! {" - T«hiˇ»he quick brown - fox j«hiˇ»umps over - the lazy dog."}, - Mode::HelixNormal, - ); - - // Multiple cursors on empty lines should paste on those same lines. - cx.set_state("ˇ\nˇ\nˇ\nend", Mode::HelixNormal); - cx.write_to_clipboard(ClipboardItem::new_string("X".to_string())); - cx.simulate_keystrokes("p"); - cx.assert_state("«Xˇ»\n«Xˇ»\n«Xˇ»\nend", Mode::HelixNormal); } #[gpui::test] diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 1d0d0812e82..400084ce7b5 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -155,6 +155,10 @@ pub(crate) fn register(editor: &mut Editor, cx: &mut Context) { }) }) }); + // Plain `d` yanks to register `"` only, never the system clipboard + if vim.selected_register.is_none() { + vim.selected_register = Some('"'); + } vim.visual_delete(false, window, cx); vim.switch_mode(Mode::HelixNormal, true, window, cx); }); diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 3ca4d704c7c..b7af9f4b13a 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -893,6 +893,11 @@ impl VimGlobals { } } + /// Reads the default register `"` literally, ignoring `use_system_clipboard` + pub(crate) fn read_default_register(&self) -> Option { + self.registers.get(&'"').cloned() + } + pub(crate) fn read_register( &self, register: Option,