Append changes to history before & after each command

Related to <https://redirect.github.com/helix-editor/helix/pull/7226>

helix-term/src/commands.rs

@@ -59,7 +59,7 @@ use movement::Movement;
 use crate::{
     args,
     compositor::{self, Component, Compositor},
-    filter_picker_entry,
+    events, filter_picker_entry,
     job::Callback,
     keymap::ReverseKeymap,
     ui::{self, menu::Item, overlay::overlaid, Picker, Popup, Prompt, PromptEvent},
@@ -142,6 +142,45 @@ impl<'a> Context<'a> {
     pub fn count(&self) -> usize {
         self.count.map_or(1, |v| v.get())
     }
+
+    pub fn execute<F: FnOnce(&mut Self)>(&mut self, execute_fn: F) {
+        self.execute_impl(execute_fn)
+    }
+
+    pub fn execute_command(&mut self, command: &MappableCommand) {
+        self.execute_impl(|cx| {
+            command.execute(cx);
+            helix_event::dispatch(events::PostCommand { command, cx });
+        })
+    }
+
+    fn execute_impl<F: FnOnce(&mut Self)>(&mut self, execute_fn: F) {
+        let pre_command_mode = self.editor.mode();
+        if pre_command_mode != Mode::Insert {
+            let (view, doc) = current!(self.editor);
+            doc.append_changes_to_history(view);
+        }
+
+        execute_fn(self);
+
+        let post_command_mode = self.editor.mode();
+        if post_command_mode != pre_command_mode {
+            helix_event::dispatch(events::OnModeSwitch {
+                old_mode: pre_command_mode,
+                new_mode: post_command_mode,
+                cx: self,
+            });
+        }
+
+        if !self.editor.tree.is_empty() {
+            let scrolloff = self.editor.config().scrolloff;
+            let (view, doc) = current!(self.editor);
+            if post_command_mode != Mode::Insert {
+                doc.append_changes_to_history(view);
+            }
+            view.ensure_cursor_in_view(doc, scrolloff);
+        }
+    }
 }
 
 #[inline]
@@ -1981,7 +2020,6 @@ fn search_impl(
     regex: &rope::Regex,
     movement: Movement,
     direction: Direction,
-    scrolloff: usize,
     wrap_around: bool,
     show_warnings: bool,
 ) {
@@ -2054,7 +2092,6 @@ fn search_impl(
         };
 
         doc.set_selection(view.id, selection);
-        view.ensure_cursor_in_view_center(doc, scrolloff);
     };
 }
 
@@ -2078,7 +2115,6 @@ fn rsearch(cx: &mut Context) {
 fn searcher(cx: &mut Context, direction: Direction) {
     let reg = cx.register.unwrap_or('/');
     let config = cx.editor.config();
-    let scrolloff = config.scrolloff;
     let wrap_around = config.search.wrap_around;
     let movement = if cx.editor.mode() == Mode::Select {
         Movement::Extend
@@ -2106,15 +2142,7 @@ fn searcher(cx: &mut Context, direction: Direction) {
             } else if event != PromptEvent::Update {
                 return;
             }
-            search_impl(
-                cx.editor,
-                &regex,
-                movement,
-                direction,
-                scrolloff,
-                wrap_around,
-                false,
-            );
+            search_impl(cx.editor, &regex, movement, direction, wrap_around, false);
         },
     );
 }
@@ -2125,7 +2153,6 @@ fn search_next_or_prev_impl(cx: &mut Context, movement: Movement, direction: Dir
         .register
         .unwrap_or(cx.editor.registers.last_search_register);
     let config = cx.editor.config();
-    let scrolloff = config.scrolloff;
     if let Some(query) = cx.editor.registers.first(register, cx.editor) {
         let search_config = &config.search;
         let case_insensitive = if search_config.smart_case {
@@ -2143,15 +2170,7 @@ fn search_next_or_prev_impl(cx: &mut Context, movement: Movement, direction: Dir
             .build(&query)
         {
             for _ in 0..count {
-                search_impl(
-                    cx.editor,
-                    &regex,
-                    movement,
-                    direction,
-                    scrolloff,
-                    wrap_around,
-                    true,
-                );
+                search_impl(cx.editor, &regex, movement, direction, wrap_around, true);
             }
         } else {
             let error = format!("Invalid regex: {}", query);
@@ -3188,22 +3207,8 @@ pub fn command_palette(cx: &mut Context) {
                     on_next_key_callback: None,
                     jobs: cx.jobs,
                 };
-                let focus = view!(ctx.editor).id;
-
-                command.execute(&mut ctx);
-
-                if ctx.editor.tree.contains(focus) {
-                    let config = ctx.editor.config();
-                    let mode = ctx.editor.mode();
-                    let view = view_mut!(ctx.editor, focus);
-                    let doc = doc_mut!(ctx.editor, &view.doc);
 
-                    view.ensure_cursor_in_view(doc, config.scrolloff);
-
-                    if mode != Mode::Insert {
-                        doc.append_changes_to_history(view);
-                    }
-                }
+                ctx.execute_command(command);
             });
             compositor.push(Box::new(overlaid(picker)));
         },
@@ -4329,7 +4334,6 @@ fn replace_with_yanked_impl(editor: &mut Editor, register: char, count: usize) {
         return;
     };
     let values: Vec<_> = values.map(|value| value.to_string()).collect();
-    let scrolloff = editor.config().scrolloff;
 
     let (view, doc) = current!(editor);
     let repeat = std::iter::repeat(
@@ -4352,8 +4356,6 @@ fn replace_with_yanked_impl(editor: &mut Editor, register: char, count: usize) {
     });
 
     doc.apply(&transaction, view.id);
-    doc.append_changes_to_history(view);
-    view.ensure_cursor_in_view(doc, scrolloff);
 }
 
 fn replace_selections_with_clipboard(cx: &mut Context) {
@@ -5037,7 +5039,6 @@ fn match_brackets(cx: &mut Context) {
 
 fn jump_forward(cx: &mut Context) {
     let count = cx.count();
-    let config = cx.editor.config();
     let view = view_mut!(cx.editor);
     let doc_id = view.doc;
 
@@ -5051,13 +5052,11 @@ fn jump_forward(cx: &mut Context) {
         }
 
         doc.set_selection(view.id, selection);
-        view.ensure_cursor_in_view_center(doc, config.scrolloff);
     };
 }
 
 fn jump_backward(cx: &mut Context) {
     let count = cx.count();
-    let config = cx.editor.config();
     let (view, doc) = current!(cx.editor);
     let doc_id = doc.id();
 
@@ -5071,7 +5070,6 @@ fn jump_backward(cx: &mut Context) {
         }
 
         doc.set_selection(view.id, selection);
-        view.ensure_cursor_in_view_center(doc, config.scrolloff);
     };
 }
 
@@ -5789,10 +5787,6 @@ fn shell(cx: &mut compositor::Context, cmd: &str, behavior: &ShellBehavior) {
         doc.apply(&transaction, view.id);
         doc.append_changes_to_history(view);
     }
-
-    // after replace cursor may be out of bounds, do this to
-    // make sure cursor is in view and update scroll as well
-    view.ensure_cursor_in_view(doc, config.scrolloff);
 }
 
 fn shell_prompt(cx: &mut Context, prompt: Cow<'static, str>, behavior: ShellBehavior) {

helix-term/src/commands/typed.rs

@@ -1269,11 +1269,8 @@ fn reload(
         return Ok(());
     }
 
-    let scrolloff = cx.editor.config().scrolloff;
     let (view, doc) = current!(cx.editor);
-    doc.reload(view, &cx.editor.diff_providers).map(|_| {
-        view.ensure_cursor_in_view(doc, scrolloff);
-    })?;
+    doc.reload(view, &cx.editor.diff_providers)?;
     if let Some(path) = doc.path() {
         cx.editor
             .language_servers
@@ -2029,7 +2026,6 @@ fn sort_impl(
     _args: &[Cow<str>],
     reverse: bool,
 ) -> anyhow::Result<()> {
-    let scrolloff = cx.editor.config().scrolloff;
     let (view, doc) = current!(cx.editor);
     let text = doc.text().slice(..);
 
@@ -2054,8 +2050,6 @@ fn sort_impl(
     );
 
     doc.apply(&transaction, view.id);
-    doc.append_changes_to_history(view);
-    view.ensure_cursor_in_view(doc, scrolloff);
 
     Ok(())
 }
@@ -2069,7 +2063,6 @@ fn reflow(
         return Ok(());
     }
 
-    let scrolloff = cx.editor.config().scrolloff;
     let cfg_text_width: usize = cx.editor.config().text_width;
     let (view, doc) = current!(cx.editor);
 
@@ -2095,8 +2088,6 @@ fn reflow(
     });
 
     doc.apply(&transaction, view.id);
-    doc.append_changes_to_history(view);
-    view.ensure_cursor_in_view(doc, scrolloff);
 
     Ok(())
 }
@@ -2296,7 +2287,6 @@ fn reset_diff_change(
     ensure!(args.is_empty(), ":reset-diff-change takes no arguments");
 
     let editor = &mut cx.editor;
-    let scrolloff = editor.config().scrolloff;
 
     let (view, doc) = current!(editor);
     let Some(handle) = doc.diff_handle() else {
@@ -2334,8 +2324,6 @@ fn reset_diff_change(
     // select inserted text
     let text_len = before_end - before_start;
     doc.set_selection(view.id, Selection::single(anchor, anchor + text_len));
-    doc.append_changes_to_history(view);
-    view.ensure_cursor_in_view(doc, scrolloff);
     Ok(())
 }
 
@@ -2460,7 +2448,6 @@ fn read(cx: &mut compositor::Context, args: &[Cow<str>], event: PromptEvent) ->
         return Ok(());
     }
 
-    let scrolloff = cx.editor.config().scrolloff;
     let (view, doc) = current!(cx.editor);
 
     ensure!(!args.is_empty(), "file name is expected");
@@ -2482,8 +2469,6 @@ fn read(cx: &mut compositor::Context, args: &[Cow<str>], event: PromptEvent) ->
     let selection = doc.selection(view.id);
     let transaction = Transaction::insert(doc.text(), selection, contents);
     doc.apply(&transaction, view.id);
-    doc.append_changes_to_history(view);
-    view.ensure_cursor_in_view(doc, scrolloff);
 
     Ok(())
 }
@@ -3190,9 +3175,17 @@ pub(super) fn command_mode(cx: &mut Context) {
                 let shellwords = Shellwords::from(input);
                 let args = shellwords.words();
 
+                let (view, doc) = current!(cx.editor);
+                doc.append_changes_to_history(view);
                 if let Err(e) = (cmd.fun)(cx, &args[1..], event) {
                     cx.editor.set_error(format!("{}", e));
                 }
+                if !cx.editor.tree.is_empty() {
+                    let scrolloff = cx.editor.config().scrolloff;
+                    let (view, doc) = current!(cx.editor);
+                    doc.append_changes_to_history(view);
+                    view.ensure_cursor_in_view(doc, scrolloff);
+                }
             } else if event == PromptEvent::Validate {
                 cx.editor
                     .set_error(format!("no such command: '{}'", parts[0]));

helix-term/src/ui/editor.rs

@@ -1,7 +1,6 @@
 use crate::{
     commands::{self, OnKeyCallback},
     compositor::{Component, Context, Event, EventResult},
-    events::{OnModeSwitch, PostCommand},
     key,
     keymap::{KeymapResult, Keymaps},
     ui::{
@@ -867,17 +866,10 @@ impl EditorView {
         cxt.editor.autoinfo = self.keymaps.sticky().map(|node| node.infobox());
 
         let mut execute_command = |command: &commands::MappableCommand| {
-            command.execute(cxt);
-            helix_event::dispatch(PostCommand { command, cx: cxt });
+            cxt.execute_command(command);
 
             let current_mode = cxt.editor.mode();
             if current_mode != last_mode {
-                helix_event::dispatch(OnModeSwitch {
-                    old_mode: last_mode,
-                    new_mode: current_mode,
-                    cx: cxt,
-                });
-
                 // HAXX: if we just entered insert mode from normal, clear key buf
                 // and record the command that got us into this mode.
                 if current_mode == Mode::Insert {
@@ -887,7 +879,6 @@ impl EditorView {
                     self.last_insert.1.clear();
                 }
             }
-
             last_mode = current_mode;
         };
 
@@ -911,18 +902,18 @@ impl EditorView {
             match keyresult {
                 KeymapResult::NotFound => {
                     if let Some(ch) = event.char() {
-                        commands::insert::insert_char(cx, ch)
+                        cx.execute(|cx| commands::insert::insert_char(cx, ch))
                     }
                 }
                 KeymapResult::Cancelled(pending) => {
                     for ev in pending {
                         match ev.char() {
-                            Some(ch) => commands::insert::insert_char(cx, ch),
+                            Some(ch) => cx.execute(|cx| commands::insert::insert_char(cx, ch)),
                             None => {
                                 if let KeymapResult::Matched(command) =
                                     self.keymaps.get(Mode::Insert, ev)
                                 {
-                                    command.execute(cx);
+                                    cx.execute_command(&command);
                                 }
                             }
                         }
@@ -949,7 +940,7 @@ impl EditorView {
             (key!('.'), _) if self.keymaps.pending().is_empty() => {
                 for _ in 0..cxt.editor.count.map_or(1, NonZeroUsize::into) {
                     // first execute whatever put us into insert mode
-                    self.last_insert.0.execute(cxt);
+                    cxt.execute_command(&self.last_insert.0);
                     let mut last_savepoint = None;
                     let mut last_request_savepoint = None;
                     // then replay the inputs
@@ -1240,8 +1231,9 @@ impl EditorView {
                 };
 
                 if should_yank {
-                    commands::MappableCommand::yank_main_selection_to_primary_clipboard
-                        .execute(cxt);
+                    cxt.execute_command(
+                        &commands::MappableCommand::yank_main_selection_to_primary_clipboard,
+                    );
                     EventResult::Consumed(None)
                 } else {
                     EventResult::Ignored(None)
@@ -1261,9 +1253,11 @@ impl EditorView {
                             doc.set_selection(view_id, Selection::point(pos));
                             match modifiers {
                                 KeyModifiers::ALT => {
-                                    commands::MappableCommand::dap_edit_log.execute(cxt)
+                                    cxt.execute_command(&commands::MappableCommand::dap_edit_log)
                                 }
-                                _ => commands::MappableCommand::dap_edit_condition.execute(cxt),
+                                _ => cxt.execute_command(
+                                    &commands::MappableCommand::dap_edit_condition,
+                                ),
                             };
                         }
                     }
@@ -1281,8 +1275,9 @@ impl EditorView {
                 }
 
                 if modifiers == KeyModifiers::ALT {
-                    commands::MappableCommand::replace_selections_with_primary_clipboard
-                        .execute(cxt);
+                    cxt.execute_command(
+                        &commands::MappableCommand::replace_selections_with_primary_clipboard,
+                    );
 
                     return EventResult::Consumed(None);
                 }
@@ -1291,7 +1286,7 @@ impl EditorView {
                     let doc = doc_mut!(editor, &view!(editor, view_id).doc);
                     doc.set_selection(view_id, Selection::point(pos));
                     cxt.editor.focus(view_id);
-                    commands::MappableCommand::paste_primary_clipboard_before.execute(cxt);
+                    cxt.execute_command(&commands::MappableCommand::paste_primary_clipboard_before);
 
                     return EventResult::Consumed(None);
                 }
@@ -1323,20 +1318,9 @@ impl Component for EditorView {
             Event::Paste(contents) => {
                 self.handle_non_key_input(&mut cx);
                 cx.count = cx.editor.count;
-                commands::paste_bracketed_value(&mut cx, contents.clone());
+                cx.execute(|cx| commands::paste_bracketed_value(cx, contents.clone()));
                 cx.editor.count = None;
 
-                let config = cx.editor.config();
-                let mode = cx.editor.mode();
-                let (view, doc) = current!(cx.editor);
-                view.ensure_cursor_in_view(doc, config.scrolloff);
-
-                // Store a history state if not in insert mode. Otherwise wait till we exit insert
-                // to include any edits to the paste in the history state.
-                if mode != Mode::Insert {
-                    doc.append_changes_to_history(view);
-                }
-
                 EventResult::Consumed(None)
             }
             Event::Resize(_width, _height) => {
@@ -1355,7 +1339,7 @@ impl Component for EditorView {
 
                 if let Some(on_next_key) = self.on_next_key.take() {
                     // if there's a command waiting input, do that first
-                    on_next_key(&mut cx, key);
+                    cx.execute(|cx| on_next_key(cx, key));
                 } else {
                     match mode {
                         Mode::Insert => {
@@ -1419,17 +1403,6 @@ impl Component for EditorView {
                     return EventResult::Ignored(None);
                 }
 
-                let config = cx.editor.config();
-                let mode = cx.editor.mode();
-                let (view, doc) = current!(cx.editor);
-
-                view.ensure_cursor_in_view(doc, config.scrolloff);
-
-                // Store a history state if not in insert mode. This also takes care of
-                // committing changes when leaving insert mode.
-                if mode != Mode::Insert {
-                    doc.append_changes_to_history(view);
-                }
                 let callback = if callbacks.is_empty() {
                     None
                 } else {