on master help sign in
(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<Arc<dyn Fn(&mut Editor)>>` * Take motion rather than cloning it. Co-authored-by: Michael Davis <mcarsondavis@gmail.com> * last_motion as Option<Motion>. * Use `Box` over `Arc` for `last_motion`. --------- Co-authored-by: Michael Davis <mcarsondavis@gmail.com>

commitc1488267e59a341ece2a8bb274e66127dca72088 authorGabriel Hansson <gabrielhansson00@gmail.com> dateSun 9 Jul 2023, 22:50 (3 years ago) parent9a324f3
2 files changed+45−53
helix-term/src/commands.rs+29−39 helix-view/src/editor.rs+16−14
helix-term/src/commands.rs+29−39
@@ -34,7 +34,7 @@ use helix_core::{
3434use helix_view::{
3535 clipboard::ClipboardType,
3636 document::{FormatterError, Mode, SCRATCH_BUFFER_NAME},
37− editor::{Action, CompleteAction, Motion},
37+ editor::{Action, CompleteAction},
3838 info::Info,
3939 input::KeyEvent,
4040 keyboard::KeyCode,
@@ -1099,8 +1099,7 @@ where
10991099 .transform(|range| move_fn(text, range, count, behavior));
11001100 doc.set_selection(view.id, selection);
11011101 };
1102− motion(cx.editor);
1103− cx.editor.last_motion = Some(Motion(Box::new(motion)));
1102+ cx.editor.apply_motion(motion)
11041103}
11051104
11061105fn goto_prev_paragraph(cx: &mut Context) {
@@ -1240,10 +1239,7 @@ fn extend_next_long_word_end(cx: &mut Context) {
12401239 extend_word_impl(cx, movement::move_next_long_word_end)
12411240}
12421241
1243−fn will_find_char<F>(cx: &mut Context, search_fn: F, inclusive: bool, extend: bool)
1244−where
1245− F: Fn(RopeSlice, char, usize, usize, bool) -> Option<usize> + 'static,
1246−{
1242+fn find_char(cx: &mut Context, direction: Direction, inclusive: bool, extend: bool) {
12471243 // TODO: count is reset to 1 before next key so we move it into the closure here.
12481244 // Would be nice to carry over.
12491245 let count = cx.count();
@@ -1275,11 +1271,18 @@ where
12751271 } => ch,
12761272 _ => return,
12771273 };
1274+ let motion = move |editor: &mut Editor| {
1275+ match direction {
1276+ Direction::Forward => {
1277+ find_char_impl(editor, &find_next_char_impl, inclusive, extend, ch, count)
1278+ }
1279+ Direction::Backward => {
1280+ find_char_impl(editor, &find_prev_char_impl, inclusive, extend, ch, count)
1281+ }
1282+ };
1283+ };
12781284
1279− find_char_impl(cx.editor, &search_fn, inclusive, extend, ch, count);
1280− cx.editor.last_motion = Some(Motion(Box::new(move |editor: &mut Editor| {
1281− find_char_impl(editor, &search_fn, inclusive, extend, ch, 1);
1282− })));
1285+ cx.editor.apply_motion(motion);
12831286 })
12841287}
12851288
@@ -1358,46 +1361,39 @@ fn find_prev_char_impl(
13581361}
13591362
13601363fn find_till_char(cx: &mut Context) {
1361− will_find_char(cx, find_next_char_impl, false, false)
1364+ find_char(cx, Direction::Forward, false, false);
13621365}
13631366
13641367fn find_next_char(cx: &mut Context) {
1365− will_find_char(cx, find_next_char_impl, true, false)
1368+ find_char(cx, Direction::Forward, true, false)
13661369}
13671370
13681371fn extend_till_char(cx: &mut Context) {
1369− will_find_char(cx, find_next_char_impl, false, true)
1372+ find_char(cx, Direction::Forward, false, true)
13701373}
13711374
13721375fn extend_next_char(cx: &mut Context) {
1373− will_find_char(cx, find_next_char_impl, true, true)
1376+ find_char(cx, Direction::Forward, true, true)
13741377}
13751378
13761379fn till_prev_char(cx: &mut Context) {
1377− will_find_char(cx, find_prev_char_impl, false, false)
1380+ find_char(cx, Direction::Backward, false, false)
13781381}
13791382
13801383fn find_prev_char(cx: &mut Context) {
1381− will_find_char(cx, find_prev_char_impl, true, false)
1384+ find_char(cx, Direction::Backward, true, false)
13821385}
13831386
13841387fn extend_till_prev_char(cx: &mut Context) {
1385− will_find_char(cx, find_prev_char_impl, false, true)
1388+ find_char(cx, Direction::Backward, false, true)
13861389}
13871390
13881391fn extend_prev_char(cx: &mut Context) {
1389− will_find_char(cx, find_prev_char_impl, true, true)
1392+ find_char(cx, Direction::Backward, true, true)
13901393}
13911394
13921395fn repeat_last_motion(cx: &mut Context) {
1393− let count = cx.count();
1394− let last_motion = cx.editor.last_motion.take();
1395− if let Some(m) = &last_motion {
1396− for _ in 0..count {
1397− m.run(cx.editor);
1398− }
1399− cx.editor.last_motion = last_motion;
1400− }
1396+ cx.editor.repeat_last_motion(cx.count())
14011397}
14021398
14031399fn replace(cx: &mut Context) {
@@ -3248,8 +3244,7 @@ fn goto_next_change_impl(cx: &mut Context, direction: Direction) {
32483244
32493245 doc.set_selection(view.id, selection)
32503246 };
3251− motion(cx.editor);
3252− cx.editor.last_motion = Some(Motion(Box::new(motion)));
3247+ cx.editor.apply_motion(motion);
32533248}
32543249
32553250/// Returns the [Range] for a [Hunk] in the given text.
@@ -4584,8 +4579,7 @@ fn expand_selection(cx: &mut Context) {
45844579 }
45854580 }
45864581 };
4587− motion(cx.editor);
4588− cx.editor.last_motion = Some(Motion(Box::new(motion)));
4582+ cx.editor.apply_motion(motion);
45894583}
45904584
45914585fn shrink_selection(cx: &mut Context) {
@@ -4609,8 +4603,7 @@ fn shrink_selection(cx: &mut Context) {
46094603 doc.set_selection(view.id, selection);
46104604 }
46114605 };
4612− motion(cx.editor);
4613− cx.editor.last_motion = Some(Motion(Box::new(motion)));
4606+ cx.editor.apply_motion(motion);
46144607}
46154608
46164609fn select_sibling_impl<F>(cx: &mut Context, sibling_fn: &'static F)
@@ -4628,8 +4621,7 @@ where
46284621 doc.set_selection(view.id, selection);
46294622 }
46304623 };
4631− motion(cx.editor);
4632− cx.editor.last_motion = Some(Motion(Box::new(motion)));
4624+ cx.editor.apply_motion(motion);
46334625}
46344626
46354627fn select_next_sibling(cx: &mut Context) {
@@ -4912,8 +4904,7 @@ fn goto_ts_object_impl(cx: &mut Context, object: &'static str, direction: Direct
49124904 editor.set_status("Syntax-tree is not available in current buffer");
49134905 }
49144906 };
4915− motion(cx.editor);
4916− cx.editor.last_motion = Some(Motion(Box::new(motion)));
4907+ cx.editor.apply_motion(motion);
49174908}
49184909
49194910fn goto_next_function(cx: &mut Context) {
@@ -5034,8 +5025,7 @@ fn select_textobject(cx: &mut Context, objtype: textobject::TextObject) {
50345025 });
50355026 doc.set_selection(view.id, selection);
50365027 };
5037− textobject(cx.editor);
5038− cx.editor.last_motion = Some(Motion(Box::new(textobject)));
5028+ cx.editor.apply_motion(textobject);
50395029 }
50405030 });
50415031
helix-view/src/editor.rs+16−14
@@ -834,18 +834,6 @@ impl Default for SearchConfig {
834834 }
835835}
836836
837−pub struct Motion(pub Box<dyn Fn(&mut Editor)>);
838−impl Motion {
839− pub fn run(&self, e: &mut Editor) {
840− (self.0)(e)
841− }
842−}
843−impl std::fmt::Debug for Motion {
844− fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
845− f.write_str("motion")
846− }
847−}
848−
849837#[derive(Debug, Clone, Default)]
850838pub struct Breakpoint {
851839 pub id: Option<usize>,
@@ -910,8 +898,7 @@ pub struct Editor {
910898 pub auto_pairs: Option<AutoPairs>,
911899
912900 pub idle_timer: Pin<Box<Sleep>>,
913− pub last_motion: Option<Motion>,
914−
901+ last_motion: Option<Motion>,
915902 pub last_completion: Option<CompleteAction>,
916903
917904 pub exit_code: i32,
@@ -945,6 +932,8 @@ pub struct Editor {
945932 pub completion_request_handle: Option<oneshot::Sender<()>>,
946933}
947934
935+pub type Motion = Box<dyn Fn(&mut Editor)>;
936+
948937pub type RedrawHandle = (Arc<Notify>, Arc<RwLock<()>>);
949938
950939#[derive(Debug)]
@@ -1051,6 +1040,19 @@ impl Editor {
10511040 }
10521041 }
10531042
1043+ pub fn apply_motion<F: Fn(&mut Self) + 'static>(&mut self, motion: F) {
1044+ motion(self);
1045+ self.last_motion = Some(Box::new(motion));
1046+ }
1047+
1048+ pub fn repeat_last_motion(&mut self, count: usize) {
1049+ if let Some(motion) = self.last_motion.take() {
1050+ for _ in 0..count {
1051+ motion(self);
1052+ }
1053+ self.last_motion = Some(motion);
1054+ }
1055+ }
10541056 /// Current editing mode for the [`Editor`].
10551057 pub fn mode(&self) -> Mode {
10561058 self.mode