From c1488267e59a341ece2a8bb274e66127dca72088 Mon Sep 17 00:00:00 2001 From: Gabriel Hansson Date: Sun, 9 Jul 2023 22:50:24 +0200 Subject: [PATCH] (Updated) Apply motion API refinements (#6078) * _apply_motion generalization where possible API encourages users to not forget setting `editor.last_motion` when applying a motion. But also not setting `last_motion` without applying a motion first. * (rename) will_find_char -> find_char method name makes it sound like it would be returning a boolean. * use _apply_motion in find_char Feature that falls out from this is that repetitions of t,T,f,F are saved with the context extention/move and count. (Not defaulting to extend by 1 count). * Finalize apply_motion API last_motion is now a private field and can only be set by calling Editor.apply_motion(). Removing need (and possibility) of writing: `motion(editor); editor.last_motion = motion` Now it's just: `editor.apply_motion(motion)` * editor.last_message: rm Box wrap around Arc * Use pre-existing `Direction` rather than custom `SearchDirection`. * `LastMotion` type alias for `Option>` * Take motion rather than cloning it. Co-authored-by: Michael Davis * last_motion as Option. * Use `Box` over `Arc` for `last_motion`. --------- Co-authored-by: Michael Davis --- helix-term/src/commands.rs | 68 ++++++++++++++++---------------------- helix-view/src/editor.rs | 30 +++++++++-------- 2 files changed, 45 insertions(+), 53 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 10b336ce..b90f418a 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -34,7 +34,7 @@ use helix_core::{ use helix_view::{ clipboard::ClipboardType, document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, - editor::{Action, CompleteAction, Motion}, + editor::{Action, CompleteAction}, info::Info, input::KeyEvent, keyboard::KeyCode, @@ -1099,8 +1099,7 @@ where .transform(|range| move_fn(text, range, count, behavior)); doc.set_selection(view.id, selection); }; - motion(cx.editor); - cx.editor.last_motion = Some(Motion(Box::new(motion))); + cx.editor.apply_motion(motion) } fn goto_prev_paragraph(cx: &mut Context) { @@ -1240,10 +1239,7 @@ fn extend_next_long_word_end(cx: &mut Context) { extend_word_impl(cx, movement::move_next_long_word_end) } -fn will_find_char(cx: &mut Context, search_fn: F, inclusive: bool, extend: bool) -where - F: Fn(RopeSlice, char, usize, usize, bool) -> Option + 'static, -{ +fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bool) { // TODO: count is reset to 1 before next key so we move it into the closure here. // Would be nice to carry over. let count = cx.count(); @@ -1275,11 +1271,18 @@ where } => ch, _ => return, }; + let motion = move |editor: &mut Editor| { + match direction { + Direction::Forward => { + find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count) + } + Direction::Backward => { + find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count) + } + }; + }; - find_char_impl(cx.editor, &search_fn, inclusive, extend, ch, count); - cx.editor.last_motion = Some(Motion(Box::new(move |editor: &mut Editor| { - find_char_impl(editor, &search_fn, inclusive, extend, ch, 1); - }))); + cx.editor.apply_motion(motion); }) } @@ -1358,46 +1361,39 @@ fn find_prev_char_impl( } fn find_till_char(cx: &mut Context) { - will_find_char(cx, find_next_char_impl, false, false) + find_char(cx, Direction::Forward, false, false); } fn find_next_char(cx: &mut Context) { - will_find_char(cx, find_next_char_impl, true, false) + find_char(cx, Direction::Forward, true, false) } fn extend_till_char(cx: &mut Context) { - will_find_char(cx, find_next_char_impl, false, true) + find_char(cx, Direction::Forward, false, true) } fn extend_next_char(cx: &mut Context) { - will_find_char(cx, find_next_char_impl, true, true) + find_char(cx, Direction::Forward, true, true) } fn till_prev_char(cx: &mut Context) { - will_find_char(cx, find_prev_char_impl, false, false) + find_char(cx, Direction::Backward, false, false) } fn find_prev_char(cx: &mut Context) { - will_find_char(cx, find_prev_char_impl, true, false) + find_char(cx, Direction::Backward, true, false) } fn extend_till_prev_char(cx: &mut Context) { - will_find_char(cx, find_prev_char_impl, false, true) + find_char(cx, Direction::Backward, false, true) } fn extend_prev_char(cx: &mut Context) { - will_find_char(cx, find_prev_char_impl, true, true) + find_char(cx, Direction::Backward, true, true) } fn repeat_last_motion(cx: &mut Context) { - let count = cx.count(); - let last_motion = cx.editor.last_motion.take(); - if let Some(m) = &last_motion { - for _ in 0..count { - m.run(cx.editor); - } - cx.editor.last_motion = last_motion; - } + cx.editor.repeat_last_motion(cx.count()) } fn replace(cx: &mut Context) { @@ -3248,8 +3244,7 @@ fn goto_next_change_impl(cx: &mut Context, direction: Direction) { doc.set_selection(view.id, selection) }; - motion(cx.editor); - cx.editor.last_motion = Some(Motion(Box::new(motion))); + cx.editor.apply_motion(motion); } /// Returns the [Range] for a [Hunk] in the given text. @@ -4584,8 +4579,7 @@ fn expand_selection(cx: &mut Context) { } } }; - motion(cx.editor); - cx.editor.last_motion = Some(Motion(Box::new(motion))); + cx.editor.apply_motion(motion); } fn shrink_selection(cx: &mut Context) { @@ -4609,8 +4603,7 @@ fn shrink_selection(cx: &mut Context) { doc.set_selection(view.id, selection); } }; - motion(cx.editor); - cx.editor.last_motion = Some(Motion(Box::new(motion))); + cx.editor.apply_motion(motion); } fn select_sibling_impl(cx: &mut Context, sibling_fn: &'static F) @@ -4628,8 +4621,7 @@ where doc.set_selection(view.id, selection); } }; - motion(cx.editor); - cx.editor.last_motion = Some(Motion(Box::new(motion))); + cx.editor.apply_motion(motion); } fn select_next_sibling(cx: &mut Context) { @@ -4912,8 +4904,7 @@ fn goto_ts_object_impl(cx: &mut Context, object: &'static str, direction: Direct editor.set_status("Syntax-tree is not available in current buffer"); } }; - motion(cx.editor); - cx.editor.last_motion = Some(Motion(Box::new(motion))); + cx.editor.apply_motion(motion); } fn goto_next_function(cx: &mut Context) { @@ -5034,8 +5025,7 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) { }); doc.set_selection(view.id, selection); }; - textobject(cx.editor); - cx.editor.last_motion = Some(Motion(Box::new(textobject))); + cx.editor.apply_motion(textobject); } }); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 630ea5cf..affe87dd 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -834,18 +834,6 @@ impl Default for SearchConfig { } } -pub struct Motion(pub Box); -impl Motion { - pub fn run(&self, e: &mut Editor) { - (self.0)(e) - } -} -impl std::fmt::Debug for Motion { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("motion") - } -} - #[derive(Debug, Clone, Default)] pub struct Breakpoint { pub id: Option, @@ -910,8 +898,7 @@ pub struct Editor { pub auto_pairs: Option, pub idle_timer: Pin>, - pub last_motion: Option, - + last_motion: Option, pub last_completion: Option, pub exit_code: i32, @@ -945,6 +932,8 @@ pub struct Editor { pub completion_request_handle: Option>, } +pub type Motion = Box; + pub type RedrawHandle = (Arc, Arc>); #[derive(Debug)] @@ -1051,6 +1040,19 @@ impl Editor { } } + pub fn apply_motion(&mut self, motion: F) { + motion(self); + self.last_motion = Some(Box::new(motion)); + } + + pub fn repeat_last_motion(&mut self, count: usize) { + if let Some(motion) = self.last_motion.take() { + for _ in 0..count { + motion(self); + } + self.last_motion = Some(motion); + } + } /// Current editing mode for the [`Editor`]. pub fn mode(&self) -> Mode { self.mode