Fix f,t on soft-wrapped lines

Also remove the (dangerously confusing) display_map.find_while

crates/editor/src/display_map.rs

@@ -555,67 +555,6 @@ impl DisplaySnapshot {
             })
     }
 
-    /// Returns an iterator of the start positions of the occurrences of `target` in the `self` after `from`
-    /// Stops if `condition` returns false for any of the character position pairs observed.
-    pub fn find_while<'a>(
-        &'a self,
-        from: DisplayPoint,
-        target: &str,
-        condition: impl FnMut(char, DisplayPoint) -> bool + 'a,
-    ) -> impl Iterator<Item = DisplayPoint> + 'a {
-        Self::find_internal(self.chars_at(from), target.chars().collect(), condition)
-    }
-
-    /// Returns an iterator of the end positions of the occurrences of `target` in the `self` before `from`
-    /// Stops if `condition` returns false for any of the character position pairs observed.
-    pub fn reverse_find_while<'a>(
-        &'a self,
-        from: DisplayPoint,
-        target: &str,
-        condition: impl FnMut(char, DisplayPoint) -> bool + 'a,
-    ) -> impl Iterator<Item = DisplayPoint> + 'a {
-        Self::find_internal(
-            self.reverse_chars_at(from),
-            target.chars().rev().collect(),
-            condition,
-        )
-    }
-
-    fn find_internal<'a>(
-        iterator: impl Iterator<Item = (char, DisplayPoint)> + 'a,
-        target: Vec<char>,
-        mut condition: impl FnMut(char, DisplayPoint) -> bool + 'a,
-    ) -> impl Iterator<Item = DisplayPoint> + 'a {
-        // List of partial matches with the index of the last seen character in target and the starting point of the match
-        let mut partial_matches: Vec<(usize, DisplayPoint)> = Vec::new();
-        iterator
-            .take_while(move |(ch, point)| condition(*ch, *point))
-            .filter_map(move |(ch, point)| {
-                if Some(&ch) == target.get(0) {
-                    partial_matches.push((0, point));
-                }
-
-                let mut found = None;
-                // Keep partial matches that have the correct next character
-                partial_matches.retain_mut(|(match_position, match_start)| {
-                    if target.get(*match_position) == Some(&ch) {
-                        *match_position += 1;
-                        if *match_position == target.len() {
-                            found = Some(match_start.clone());
-                            // This match is completed. No need to keep tracking it
-                            false
-                        } else {
-                            true
-                        }
-                    } else {
-                        false
-                    }
-                });
-
-                found
-            })
-    }
-
     pub fn column_to_chars(&self, display_row: u32, target: u32) -> u32 {
         let mut count = 0;
         let mut column = 0;
@@ -933,7 +872,7 @@ pub mod tests {
     use smol::stream::StreamExt;
     use std::{env, sync::Arc};
     use theme::SyntaxTheme;
-    use util::test::{marked_text_offsets, marked_text_ranges, sample_text};
+    use util::test::{marked_text_ranges, sample_text};
     use Bias::*;
 
     #[gpui::test(iterations = 100)]
@@ -1744,32 +1683,6 @@ pub mod tests {
         )
     }
 
-    #[test]
-    fn test_find_internal() {
-        assert("This is a ˇtest of find internal", "test");
-        assert("Some text ˇaˇaˇaa with repeated characters", "aa");
-
-        fn assert(marked_text: &str, target: &str) {
-            let (text, expected_offsets) = marked_text_offsets(marked_text);
-
-            let chars = text
-                .chars()
-                .enumerate()
-                .map(|(index, ch)| (ch, DisplayPoint::new(0, index as u32)));
-            let target = target.chars();
-
-            assert_eq!(
-                expected_offsets
-                    .into_iter()
-                    .map(|offset| offset as u32)
-                    .collect::<Vec<_>>(),
-                DisplaySnapshot::find_internal(chars, target.collect(), |_, _| true)
-                    .map(|point| point.column())
-                    .collect::<Vec<_>>()
-            )
-        }
-    }
-
     fn syntax_chunks<'a>(
         rows: Range<u32>,
         map: &ModelHandle<DisplayMap>,

crates/vim/src/motion.rs

@@ -1,9 +1,9 @@
-use std::{cmp, sync::Arc};
+use std::cmp;
 
 use editor::{
     char_kind,
     display_map::{DisplaySnapshot, FoldPoint, ToDisplayPoint},
-    movement::{self, FindRange},
+    movement::{self, find_boundary, find_preceding_boundary, FindRange},
     Bias, CharKind, DisplayPoint, ToOffset,
 };
 use gpui::{actions, impl_actions, AppContext, WindowContext};
@@ -37,8 +37,8 @@ pub enum Motion {
     StartOfDocument,
     EndOfDocument,
     Matching,
-    FindForward { before: bool, text: Arc<str> },
-    FindBackward { after: bool, text: Arc<str> },
+    FindForward { before: bool, char: char },
+    FindBackward { after: bool, char: char },
     NextLineStart,
 }
 
@@ -233,25 +233,25 @@ pub(crate) fn motion(motion: Motion, cx: &mut WindowContext) {
 
 fn repeat_motion(backwards: bool, cx: &mut WindowContext) {
     let find = match Vim::read(cx).workspace_state.last_find.clone() {
-        Some(Motion::FindForward { before, text }) => {
+        Some(Motion::FindForward { before, char }) => {
             if backwards {
                 Motion::FindBackward {
                     after: before,
-                    text,
+                    char,
                 }
             } else {
-                Motion::FindForward { before, text }
+                Motion::FindForward { before, char }
             }
         }
 
-        Some(Motion::FindBackward { after, text }) => {
+        Some(Motion::FindBackward { after, char }) => {
             if backwards {
                 Motion::FindForward {
                     before: after,
-                    text,
+                    char,
                 }
             } else {
-                Motion::FindBackward { after, text }
+                Motion::FindBackward { after, char }
             }
         }
         _ => return,
@@ -403,12 +403,12 @@ impl Motion {
                 SelectionGoal::None,
             ),
             Matching => (matching(map, point), SelectionGoal::None),
-            FindForward { before, text } => (
-                find_forward(map, point, *before, text.clone(), times),
+            FindForward { before, char } => (
+                find_forward(map, point, *before, *char, times),
                 SelectionGoal::None,
             ),
-            FindBackward { after, text } => (
-                find_backward(map, point, *after, text.clone(), times),
+            FindBackward { after, char } => (
+                find_backward(map, point, *after, *char, times),
                 SelectionGoal::None,
             ),
             NextLineStart => (next_line_start(map, point, times), SelectionGoal::None),
@@ -793,44 +793,55 @@ fn find_forward(
     map: &DisplaySnapshot,
     from: DisplayPoint,
     before: bool,
-    target: Arc<str>,
+    target: char,
     times: usize,
 ) -> DisplayPoint {
-    map.find_while(from, target.as_ref(), |ch, _| ch != '\n')
-        .skip_while(|found_at| found_at == &from)
-        .nth(times - 1)
-        .map(|mut found| {
-            if before {
-                *found.column_mut() -= 1;
-                found = map.clip_point(found, Bias::Right);
-                found
-            } else {
-                found
-            }
-        })
-        .unwrap_or(from)
+    let mut to = from;
+    let mut found = false;
+
+    for _ in 0..times {
+        found = false;
+        to = find_boundary(map, to, FindRange::SingleLine, |_, right| {
+            found = right == target;
+            found
+        });
+    }
+
+    if found {
+        if before && to.column() > 0 {
+            *to.column_mut() -= 1;
+            map.clip_point(to, Bias::Left)
+        } else {
+            to
+        }
+    } else {
+        from
+    }
 }
 
 fn find_backward(
     map: &DisplaySnapshot,
     from: DisplayPoint,
     after: bool,
-    target: Arc<str>,
+    target: char,
     times: usize,
 ) -> DisplayPoint {
-    map.reverse_find_while(from, target.as_ref(), |ch, _| ch != '\n')
-        .skip_while(|found_at| found_at == &from)
-        .nth(times - 1)
-        .map(|mut found| {
-            if after {
-                *found.column_mut() += 1;
-                found = map.clip_point(found, Bias::Left);
-                found
-            } else {
-                found
-            }
-        })
-        .unwrap_or(from)
+    let mut to = from;
+
+    for _ in 0..times {
+        to = find_preceding_boundary(map, to, FindRange::SingleLine, |_, right| right == target);
+    }
+
+    if map.buffer_snapshot.chars_at(to.to_point(map)).next() == Some(target) {
+        if after {
+            *to.column_mut() += 1;
+            map.clip_point(to, Bias::Right)
+        } else {
+            to
+        }
+    } else {
+        from
+    }
 }
 
 fn next_line_start(map: &DisplaySnapshot, point: DisplayPoint, times: usize) -> DisplayPoint {

crates/vim/src/normal.rs

@@ -780,6 +780,7 @@ mod test {
     #[gpui::test]
     async fn test_f_and_t(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;
+
         for count in 1..=3 {
             let test_case = indoc! {"
                 ˇaaaˇbˇ ˇbˇ   ˇbˇbˇ aˇaaˇbaaa

crates/vim/src/test.rs

@@ -449,6 +449,13 @@ async fn test_wrapped_lines(cx: &mut gpui::TestAppContext) {
         fourteen char
     "})
         .await;
+    cx.simulate_shared_keystrokes(["j", "shift-f", "e", "f", "r"])
+        .await;
+    cx.assert_shared_state(indoc! {"
+        fourteen•
+        fourteen chaˇr
+    "})
+        .await;
 }
 
 #[gpui::test]

crates/vim/src/vim.rs

@@ -295,14 +295,20 @@ impl Vim {
 
         match Vim::read(cx).active_operator() {
             Some(Operator::FindForward { before }) => {
-                let find = Motion::FindForward { before, text };
+                let find = Motion::FindForward {
+                    before,
+                    char: text.chars().next().unwrap(),
+                };
                 Vim::update(cx, |vim, _| {
                     vim.workspace_state.last_find = Some(find.clone())
                 });
                 motion::motion(find, cx)
             }
             Some(Operator::FindBackward { after }) => {
-                let find = Motion::FindBackward { after, text };
+                let find = Motion::FindBackward {
+                    after,
+                    char: text.chars().next().unwrap(),
+                };
                 Vim::update(cx, |vim, _| {
                     vim.workspace_state.last_find = Some(find.clone())
                 });

crates/vim/test_data/test_wrapped_lines.json

@@ -53,3 +53,9 @@
 {"Key":"i"}
 {"Key":"w"}
 {"Get":{"state":"fourteenˇ \nfourteen char\n","mode":"Normal"}}
+{"Key":"j"}
+{"Key":"shift-f"}
+{"Key":"e"}
+{"Key":"f"}
+{"Key":"r"}
+{"Get":{"state":"fourteen \nfourteen chaˇr\n","mode":"Normal"}}