From bffde43c12bc2d85a5a5595e41e3f7131a4ff274 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Wed, 13 Mar 2024 23:12:12 -0600 Subject: [PATCH] Improve vim undo (#9317) The important change here is to ensure that undo never lands you in visual mode; but we also take care to restore the selection the same way vim does (visual line goes to beginning of line, visual block to the top left, etc.). To help make this behaviour feel right we also group any deletions that started insert mode with the first text inserted. Fixes: #7521 Release Notes: - vim: Improved undo. It will now restore you to normal mode in the same position as vim, and group deletions caused by `c` or `s` with the concomitant insert. ([#7521](https://github.com/zed-industries/zed/issues/7521)). --- assets/keymaps/vim.json | 4 +- crates/editor/src/editor.rs | 17 ++++ crates/vim/src/editor_events.rs | 1 - crates/vim/src/state.rs | 7 +- crates/vim/src/test.rs | 52 ++++++++++++ crates/vim/src/vim.rs | 120 ++++++++++++++++++++++------ crates/vim/test_data/test_undo.json | 45 +++++++++++ 7 files changed, 219 insertions(+), 27 deletions(-) create mode 100644 crates/vim/test_data/test_undo.json diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 5cc0d20038..ef10432479 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -515,7 +515,9 @@ "ctrl-w": "editor::DeleteToPreviousWordStart", "ctrl-u": "editor::DeleteToBeginningOfLine", "ctrl-t": "vim::Indent", - "ctrl-d": "vim::Outdent" + "ctrl-d": "vim::Outdent", + "ctrl-r \"": "editor::Paste", + "ctrl-r +": "editor::Paste" } }, { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 76ad4a6eac..1db383e6eb 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5697,6 +5697,9 @@ impl Editor { self.unmark_text(cx); self.refresh_copilot_suggestions(true, cx); cx.emit(EditorEvent::Edited); + cx.emit(EditorEvent::TransactionUndone { + transaction_id: tx_id, + }); } } @@ -5724,6 +5727,11 @@ impl Editor { .update(cx, |buffer, cx| buffer.finalize_last_transaction(cx)); } + pub fn group_until_transaction(&mut self, tx_id: TransactionId, cx: &mut ViewContext) { + self.buffer + .update(cx, |buffer, cx| buffer.group_until_transaction(tx_id, cx)); + } + pub fn move_left(&mut self, _: &MoveLeft, cx: &mut ViewContext) { self.change_selections(Some(Autoscroll::fit()), cx, |s| { let line_mode = s.line_mode; @@ -8551,6 +8559,9 @@ impl Editor { { self.selection_history .insert_transaction(tx_id, self.selections.disjoint_anchors()); + cx.emit(EditorEvent::TransactionBegun { + transaction_id: tx_id, + }) } } @@ -10165,6 +10176,12 @@ pub enum EditorEvent { autoscroll: bool, }, Closed, + TransactionUndone { + transaction_id: clock::Lamport, + }, + TransactionBegun { + transaction_id: clock::Lamport, + }, } impl EventEmitter for Editor {} diff --git a/crates/vim/src/editor_events.rs b/crates/vim/src/editor_events.rs index 6d3d78b51b..3d69a7a231 100644 --- a/crates/vim/src/editor_events.rs +++ b/crates/vim/src/editor_events.rs @@ -29,7 +29,6 @@ pub fn init(cx: &mut AppContext) { }) .detach(); } - fn focused(editor: View, cx: &mut WindowContext) { Vim::update(cx, |vim, cx| { if !vim.enabled { diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 994a8f2b76..664e213431 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -1,8 +1,9 @@ use std::{fmt::Display, ops::Range, sync::Arc}; use collections::HashMap; +use editor::Anchor; use gpui::{Action, KeyContext}; -use language::CursorShape; +use language::{CursorShape, Selection, TransactionId}; use serde::{Deserialize, Serialize}; use workspace::searchable::Direction; @@ -66,6 +67,10 @@ pub struct EditorState { pub post_count: Option, pub operator_stack: Vec, + + pub current_tx: Option, + pub current_anchor: Option>, + pub undo_modes: HashMap, } #[derive(Default, Clone, Debug)] diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index 8628be7298..dbc993dbc4 100644 --- a/crates/vim/src/test.rs +++ b/crates/vim/src/test.rs @@ -981,3 +981,55 @@ async fn test_remap(cx: &mut gpui::TestAppContext) { cx.simulate_keystrokes(["g", "t"]); cx.assert_state("12ˇ 34", Mode::Normal); } + +#[gpui::test] +async fn test_undo(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + + cx.set_shared_state("hello quˇoel world").await; + cx.simulate_shared_keystrokes(["v", "i", "w", "s", "c", "o", "escape", "u"]) + .await; + cx.assert_shared_state("hello ˇquoel world").await; + cx.simulate_shared_keystrokes(["ctrl-r"]).await; + cx.assert_shared_state("hello ˇco world").await; + cx.simulate_shared_keystrokes(["a", "o", "right", "l", "escape"]) + .await; + cx.assert_shared_state("hello cooˇl world").await; + cx.simulate_shared_keystrokes(["u"]).await; + cx.assert_shared_state("hello cooˇ world").await; + cx.simulate_shared_keystrokes(["u"]).await; + cx.assert_shared_state("hello cˇo world").await; + cx.simulate_shared_keystrokes(["u"]).await; + cx.assert_shared_state("hello ˇquoel world").await; + + cx.set_shared_state("hello quˇoel world").await; + cx.simulate_shared_keystrokes(["v", "i", "w", "~", "u"]) + .await; + cx.assert_shared_state("hello ˇquoel world").await; + + cx.set_shared_state("\nhello quˇoel world\n").await; + cx.simulate_shared_keystrokes(["shift-v", "s", "c", "escape", "u"]) + .await; + cx.assert_shared_state("\nˇhello quoel world\n").await; + + cx.set_shared_state(indoc! {" + ˇ1 + 2 + 3"}) + .await; + + cx.simulate_shared_keystrokes(["ctrl-v", "shift-g", "ctrl-a"]) + .await; + cx.assert_shared_state(indoc! {" + ˇ2 + 3 + 4"}) + .await; + + cx.simulate_shared_keystrokes(["u"]).await; + cx.assert_shared_state(indoc! {" + ˇ1 + 2 + 3"}) + .await; +} diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index 9d42aef538..2171153fbb 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -25,7 +25,7 @@ use gpui::{ actions, impl_actions, Action, AppContext, EntityId, Global, KeystrokeEvent, Subscription, View, ViewContext, WeakView, WindowContext, }; -use language::{CursorShape, Point, Selection, SelectionGoal}; +use language::{CursorShape, Point, Selection, SelectionGoal, TransactionId}; pub use mode_indicator::ModeIndicator; use motion::Motion; use normal::normal_replace; @@ -209,9 +209,11 @@ impl Vim { EditorEvent::SelectionsChanged { local: true } => { let editor = editor.read(cx); if editor.leader_peer_id().is_none() { - let newest = editor.selections.newest::(cx); + let newest = editor.selections.newest_anchor().clone(); let is_multicursor = editor.selections.count() > 1; - local_selections_changed(newest, is_multicursor, cx); + Vim::update(cx, |vim, cx| { + vim.local_selections_changed(newest, is_multicursor, cx); + }) } } EditorEvent::InputIgnored { text } => { @@ -222,6 +224,12 @@ impl Vim { text, utf16_range_to_replace: range_to_replace, } => Vim::record_insertion(text, range_to_replace.clone(), cx), + EditorEvent::TransactionBegun { transaction_id } => Vim::update(cx, |vim, cx| { + vim.transaction_begun(*transaction_id, cx); + }), + EditorEvent::TransactionUndone { transaction_id } => Vim::update(cx, |vim, cx| { + vim.transaction_undone(transaction_id, cx); + }), _ => {} })); @@ -350,10 +358,13 @@ impl Vim { let state = self.state(); let last_mode = state.mode; let prior_mode = state.last_mode; + let prior_tx = state.current_tx; self.update_state(|state| { state.last_mode = last_mode; state.mode = mode; state.operator_stack.clear(); + state.current_tx.take(); + state.current_anchor.take(); }); if mode != Mode::Insert { self.take_count(cx); @@ -372,6 +383,11 @@ impl Vim { { visual_block_motion(true, editor, cx, |_, point, goal| Some((point, goal))) } + if last_mode == Mode::Insert { + if let Some(prior_tx) = prior_tx { + editor.group_until_transaction(prior_tx, cx) + } + } editor.change_selections(None, cx, |s| { // we cheat with visual block mode and use multiple cursors. @@ -472,6 +488,83 @@ impl Vim { self.state().operator_stack.last().copied() } + fn transaction_begun(&mut self, transaction_id: TransactionId, _: &mut WindowContext) { + self.update_state(|state| { + let mode = if (state.mode == Mode::Insert || state.mode == Mode::Normal) + && state.current_tx.is_none() + { + state.current_tx = Some(transaction_id); + state.last_mode + } else { + state.mode + }; + if mode == Mode::VisualLine || mode == Mode::VisualBlock { + state.undo_modes.insert(transaction_id, mode); + } + }); + } + + fn transaction_undone(&mut self, transaction_id: &TransactionId, cx: &mut WindowContext) { + if !self.state().mode.is_visual() { + return; + }; + self.update_active_editor(cx, |vim, editor, cx| { + let original_mode = vim.state().undo_modes.get(transaction_id); + editor.change_selections(None, cx, |s| match original_mode { + Some(Mode::VisualLine) => { + s.move_with(|map, selection| { + selection.collapse_to( + map.prev_line_boundary(selection.start.to_point(map)).1, + SelectionGoal::None, + ) + }); + } + Some(Mode::VisualBlock) => { + let mut first = s.first_anchor(); + first.collapse_to(first.start, first.goal); + s.select_anchors(vec![first]); + } + _ => { + s.move_with(|_, selection| { + selection.collapse_to(selection.start, selection.goal); + }); + } + }); + }); + self.switch_mode(Mode::Normal, true, cx) + } + + fn local_selections_changed( + &mut self, + newest: Selection, + is_multicursor: bool, + cx: &mut WindowContext, + ) { + let state = self.state(); + if state.mode == Mode::Insert && state.current_tx.is_some() { + if state.current_anchor.is_none() { + self.update_state(|state| state.current_anchor = Some(newest)); + } else if state.current_anchor.as_ref().unwrap() != &newest { + if let Some(tx_id) = self.update_state(|state| state.current_tx.take()) { + self.update_active_editor(cx, |_, editor, cx| { + editor.group_until_transaction(tx_id, cx) + }); + } + } + } else if state.mode == Mode::Normal && newest.start != newest.end { + if matches!(newest.goal, SelectionGoal::HorizontalRange { .. }) { + self.switch_mode(Mode::VisualBlock, false, cx); + } else { + self.switch_mode(Mode::Visual, false, cx) + } + } else if newest.start == newest.end + && !is_multicursor + && [Mode::Visual, Mode::VisualLine, Mode::VisualBlock].contains(&state.mode) + { + self.switch_mode(Mode::Normal, true, cx) + } + } + fn active_editor_input_ignored(text: Arc, cx: &mut WindowContext) { if text.is_empty() { return; @@ -667,24 +760,3 @@ impl Settings for VimSettings { Self::load_via_json_merge(default_value, user_values) } } - -fn local_selections_changed( - newest: Selection, - is_multicursor: bool, - cx: &mut WindowContext, -) { - Vim::update(cx, |vim, cx| { - if vim.state().mode == Mode::Normal && !newest.is_empty() { - if matches!(newest.goal, SelectionGoal::HorizontalRange { .. }) { - vim.switch_mode(Mode::VisualBlock, false, cx); - } else { - vim.switch_mode(Mode::Visual, false, cx) - } - } else if newest.is_empty() - && !is_multicursor - && [Mode::Visual, Mode::VisualLine, Mode::VisualBlock].contains(&vim.state().mode) - { - vim.switch_mode(Mode::Normal, true, cx) - } - }) -} diff --git a/crates/vim/test_data/test_undo.json b/crates/vim/test_data/test_undo.json new file mode 100644 index 0000000000..e7f275743e --- /dev/null +++ b/crates/vim/test_data/test_undo.json @@ -0,0 +1,45 @@ +{"Put":{"state":"hello quˇoel world"}} +{"Key":"v"} +{"Key":"i"} +{"Key":"w"} +{"Key":"s"} +{"Key":"c"} +{"Key":"o"} +{"Key":"escape"} +{"Key":"u"} +{"Get":{"state":"hello ˇquoel world","mode":"Normal"}} +{"Key":"ctrl-r"} +{"Get":{"state":"hello ˇco world","mode":"Normal"}} +{"Key":"a"} +{"Key":"o"} +{"Key":"right"} +{"Key":"l"} +{"Key":"escape"} +{"Get":{"state":"hello cooˇl world","mode":"Normal"}} +{"Key":"u"} +{"Get":{"state":"hello cooˇ world","mode":"Normal"}} +{"Key":"u"} +{"Get":{"state":"hello cˇo world","mode":"Normal"}} +{"Key":"u"} +{"Get":{"state":"hello ˇquoel world","mode":"Normal"}} +{"Put":{"state":"hello quˇoel world"}} +{"Key":"v"} +{"Key":"i"} +{"Key":"w"} +{"Key":"~"} +{"Key":"u"} +{"Get":{"state":"hello ˇquoel world","mode":"Normal"}} +{"Put":{"state":"\nhello quˇoel world\n"}} +{"Key":"shift-v"} +{"Key":"s"} +{"Key":"c"} +{"Key":"escape"} +{"Key":"u"} +{"Get":{"state":"\nˇhello quoel world\n","mode":"Normal"}} +{"Put":{"state":"ˇ1\n2\n3"}} +{"Key":"ctrl-v"} +{"Key":"shift-g"} +{"Key":"ctrl-a"} +{"Get":{"state":"ˇ2\n3\n4","mode":"Normal"}} +{"Key":"u"} +{"Get":{"state":"ˇ1\n2\n3","mode":"Normal"}}