fix vim percent motion to better match the docs and observed behavior

Kay Simmons created

Change summary

crates/editor/src/multi_buffer.rs                 | 86 ++++++++++------
crates/editor/src/test/editor_lsp_test_context.rs |  7 +
crates/language/src/buffer.rs                     | 19 +--
crates/language/src/buffer_tests.rs               |  4 
crates/vim/src/motion.rs                          | 58 +++++++++--
crates/vim/src/normal.rs                          | 53 ++++++---
crates/vim/test_data/test_o.json                  |  2 
crates/vim/test_data/test_percent.json            |  0 
8 files changed, 150 insertions(+), 79 deletions(-)

Detailed changes

crates/editor/src/multi_buffer.rs 🔗

@@ -2630,6 +2630,9 @@ impl MultiBufferSnapshot {
         self.parse_count
     }
 
+    /// Returns the smallest enclosing bracket ranges containing the given range or
+    /// None if no brackets contain range or the range is not contained in a single
+    /// excerpt
     pub fn innermost_enclosing_bracket_ranges<T: ToOffset>(
         &self,
         range: Range<T>,
@@ -2657,46 +2660,59 @@ impl MultiBufferSnapshot {
         result
     }
 
-    /// Returns enclosinng bracket ranges containing the given range or returns None if the range is
+    /// Returns enclosing bracket ranges containing the given range or returns None if the range is
     /// not contained in a single excerpt
     pub fn enclosing_bracket_ranges<'a, T: ToOffset>(
         &'a self,
         range: Range<T>,
     ) -> Option<impl Iterator<Item = (Range<usize>, Range<usize>)> + 'a> {
         let range = range.start.to_offset(self)..range.end.to_offset(self);
-        self.excerpt_containing(range.clone())
-            .map(|(excerpt, excerpt_offset)| {
-                let excerpt_buffer_start = excerpt.range.context.start.to_offset(&excerpt.buffer);
-                let excerpt_buffer_end = excerpt_buffer_start + excerpt.text_summary.len;
 
-                let start_in_buffer =
-                    excerpt_buffer_start + range.start.saturating_sub(excerpt_offset);
-                let end_in_buffer = excerpt_buffer_start + range.end.saturating_sub(excerpt_offset);
+        self.bracket_ranges(range.clone()).map(|range_pairs| {
+            range_pairs
+                .filter(move |(open, close)| open.start <= range.start && close.end >= range.end)
+        })
+    }
 
-                excerpt
-                    .buffer
-                    .enclosing_bracket_ranges(start_in_buffer..end_in_buffer)
-                    .filter_map(move |(start_bracket_range, end_bracket_range)| {
-                        if start_bracket_range.start < excerpt_buffer_start
-                            || end_bracket_range.end > excerpt_buffer_end
-                        {
-                            return None;
-                        }
+    /// Returns bracket range pairs overlapping the given `range` or returns None if the `range` is
+    /// not contained in a single excerpt
+    pub fn bracket_ranges<'a, T: ToOffset>(
+        &'a self,
+        range: Range<T>,
+    ) -> Option<impl Iterator<Item = (Range<usize>, Range<usize>)> + 'a> {
+        let range = range.start.to_offset(self)..range.end.to_offset(self);
+        let excerpt = self.excerpt_containing(range.clone());
+        excerpt.map(|(excerpt, excerpt_offset)| {
+            let excerpt_buffer_start = excerpt.range.context.start.to_offset(&excerpt.buffer);
+            let excerpt_buffer_end = excerpt_buffer_start + excerpt.text_summary.len;
 
-                        let mut start_bracket_range = start_bracket_range.clone();
-                        start_bracket_range.start =
-                            excerpt_offset + (start_bracket_range.start - excerpt_buffer_start);
-                        start_bracket_range.end =
-                            excerpt_offset + (start_bracket_range.end - excerpt_buffer_start);
-
-                        let mut end_bracket_range = end_bracket_range.clone();
-                        end_bracket_range.start =
-                            excerpt_offset + (end_bracket_range.start - excerpt_buffer_start);
-                        end_bracket_range.end =
-                            excerpt_offset + (end_bracket_range.end - excerpt_buffer_start);
-                        Some((start_bracket_range, end_bracket_range))
-                    })
-            })
+            let start_in_buffer = excerpt_buffer_start + range.start.saturating_sub(excerpt_offset);
+            let end_in_buffer = excerpt_buffer_start + range.end.saturating_sub(excerpt_offset);
+
+            excerpt
+                .buffer
+                .bracket_ranges(start_in_buffer..end_in_buffer)
+                .filter_map(move |(start_bracket_range, end_bracket_range)| {
+                    if start_bracket_range.start < excerpt_buffer_start
+                        || end_bracket_range.end > excerpt_buffer_end
+                    {
+                        return None;
+                    }
+
+                    let mut start_bracket_range = start_bracket_range.clone();
+                    start_bracket_range.start =
+                        excerpt_offset + (start_bracket_range.start - excerpt_buffer_start);
+                    start_bracket_range.end =
+                        excerpt_offset + (start_bracket_range.end - excerpt_buffer_start);
+
+                    let mut end_bracket_range = end_bracket_range.clone();
+                    end_bracket_range.start =
+                        excerpt_offset + (end_bracket_range.start - excerpt_buffer_start);
+                    end_bracket_range.end =
+                        excerpt_offset + (end_bracket_range.end - excerpt_buffer_start);
+                    Some((start_bracket_range, end_bracket_range))
+                })
+        })
     }
 
     pub fn diagnostics_update_count(&self) -> usize {
@@ -2945,10 +2961,14 @@ impl MultiBufferSnapshot {
         let range = range.start.to_offset(self)..range.end.to_offset(self);
 
         let mut cursor = self.excerpts.cursor::<usize>();
-        cursor.seek(&range.start, Bias::Right, &());
+        cursor.seek(&dbg!(range.start), Bias::Right, &());
         let start_excerpt = cursor.item();
 
-        cursor.seek(&range.end, Bias::Right, &());
+        if range.start == range.end {
+            return start_excerpt.map(|excerpt| (excerpt, *cursor.start()));
+        }
+
+        cursor.seek(&dbg!(range.end), Bias::Right, &());
         let end_excerpt = cursor.item();
 
         start_excerpt

crates/editor/src/test/editor_lsp_test_context.rs 🔗

@@ -139,6 +139,13 @@ impl<'a> EditorLspTestContext<'a> {
                 (_ "<" ">" @end) @indent
                 (_ "{" "}" @end) @indent
                 (_ "(" ")" @end) @indent"#})),
+            brackets: Some(Cow::from(indoc! {r#"
+                ("(" @open ")" @close)
+                ("[" @open "]" @close)
+                ("{" @open "}" @close)
+                ("<" @open ">" @close)
+                ("\"" @open "\"" @close)
+                (closure_parameters "|" @open "|" @close)"#})),
             ..Default::default()
         })
         .expect("Could not parse queries");

crates/language/src/buffer.rs 🔗

@@ -2346,18 +2346,18 @@ impl BufferSnapshot {
         Some(items)
     }
 
-    pub fn enclosing_bracket_ranges<'a, T: ToOffset>(
+    /// Returns bracket range pairs overlapping `range`
+    pub fn bracket_ranges<'a, T: ToOffset>(
         &'a self,
         range: Range<T>,
     ) -> impl Iterator<Item = (Range<usize>, Range<usize>)> + 'a {
         // Find bracket pairs that *inclusively* contain the given range.
-        let range = range.start.to_offset(self)..range.end.to_offset(self);
+        let range = range.start.to_offset(self).saturating_sub(1)
+            ..self.len().min(range.end.to_offset(self) + 1);
 
-        let mut matches = self.syntax.matches(
-            range.start.saturating_sub(1)..self.len().min(range.end + 1),
-            &self.text,
-            |grammar| grammar.brackets_config.as_ref().map(|c| &c.query),
-        );
+        let mut matches = self.syntax.matches(range, &self.text, |grammar| {
+            grammar.brackets_config.as_ref().map(|c| &c.query)
+        });
         let configs = matches
             .grammars()
             .iter()
@@ -2380,11 +2380,6 @@ impl BufferSnapshot {
                 matches.advance();
 
                 let Some((open, close)) = open.zip(close) else { continue };
-
-                if open.start > range.start || close.end < range.end {
-                    continue;
-                }
-
                 return Some((open, close));
             }
             None

crates/language/src/buffer_tests.rs 🔗

@@ -2072,9 +2072,7 @@ fn assert_enclosing_bracket_pairs(
         .collect::<Vec<_>>();
 
     assert_set_eq!(
-        buffer
-            .enclosing_bracket_ranges(selection_range)
-            .collect::<Vec<_>>(),
+        buffer.bracket_ranges(selection_range).collect::<Vec<_>>(),
         bracket_pairs
     );
 }

crates/vim/src/motion.rs 🔗

@@ -3,7 +3,7 @@ use std::sync::Arc;
 use editor::{
     char_kind,
     display_map::{DisplaySnapshot, ToDisplayPoint},
-    movement, Bias, CharKind, DisplayPoint,
+    movement, Bias, CharKind, DisplayPoint, ToOffset,
 };
 use gpui::{actions, impl_actions, MutableAppContext};
 use language::{Point, Selection, SelectionGoal};
@@ -450,19 +450,53 @@ fn end_of_document(map: &DisplaySnapshot, point: DisplayPoint, line: usize) -> D
     map.clip_point(new_point, Bias::Left)
 }
 
-fn matching(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint {
-    let offset = point.to_offset(map, Bias::Left);
-    if let Some((open_range, close_range)) = map
-        .buffer_snapshot
-        .innermost_enclosing_bracket_ranges(offset..offset)
-    {
-        if open_range.contains(&offset) {
-            close_range.start.to_display_point(map)
-        } else {
-            open_range.start.to_display_point(map)
+fn matching(map: &DisplaySnapshot, display_point: DisplayPoint) -> DisplayPoint {
+    // https://github.com/vim/vim/blob/1d87e11a1ef201b26ed87585fba70182ad0c468a/runtime/doc/motion.txt#L1200
+    let point = display_point.to_point(map);
+    let offset = point.to_offset(&map.buffer_snapshot);
+
+    // Ensure the range is contained by the current line.
+    let mut line_end = map.next_line_boundary(point).0;
+    if line_end == point {
+        line_end = map.max_point().to_point(map);
+    }
+    line_end.column = line_end.column.saturating_sub(1);
+
+    let line_range = map.prev_line_boundary(point).0..line_end;
+    let ranges = map.buffer_snapshot.bracket_ranges(line_range.clone());
+    if let Some(ranges) = ranges {
+        let line_range = line_range.start.to_offset(&map.buffer_snapshot)
+            ..line_range.end.to_offset(&map.buffer_snapshot);
+        let mut closest_pair_destination = None;
+        let mut closest_distance = usize::MAX;
+
+        for (open_range, close_range) in ranges {
+            if open_range.start >= offset && line_range.contains(&open_range.start) {
+                let distance = open_range.start - offset;
+                if distance < closest_distance {
+                    closest_pair_destination = Some(close_range.start);
+                    closest_distance = distance;
+                    continue;
+                }
+            }
+
+            if close_range.start >= offset && line_range.contains(&close_range.start) {
+                let distance = close_range.start - offset;
+                if distance < closest_distance {
+                    closest_pair_destination = Some(open_range.start);
+                    closest_distance = distance;
+                    continue;
+                }
+            }
+
+            continue;
         }
+
+        closest_pair_destination
+            .map(|destination| destination.to_display_point(map))
+            .unwrap_or(display_point)
     } else {
-        point
+        display_point
     }
 }
 

crates/vim/src/normal.rs 🔗

@@ -824,17 +824,34 @@ mod test {
                 ˇ
                 brown fox"})
             .await;
-        cx.assert(indoc! {"
+
+        cx.assert_manual(
+            indoc! {"
                 fn test() {
                     println!(ˇ);
-                }
-            "})
-            .await;
-        cx.assert(indoc! {"
+                }"},
+            Mode::Normal,
+            indoc! {"
+                fn test() {
+                    println!();
+                    ˇ
+                }"},
+            Mode::Insert,
+        );
+
+        cx.assert_manual(
+            indoc! {"
                 fn test(ˇ) {
                     println!();
-                }"})
-            .await;
+                }"},
+            Mode::Normal,
+            indoc! {"
+                fn test() {
+                    ˇ
+                    println!();
+                }"},
+            Mode::Insert,
+        );
     }
 
     #[gpui::test]
@@ -996,14 +1013,14 @@ mod test {
     #[gpui::test]
     async fn test_capital_f_and_capital_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
-                ˇ    ˇbˇaaˇa ˇbˇbˇb
-                ˇ   
-                ˇb
+        let test_case = indoc! {"
+            ˇaaaˇbˇ ˇbˇ   ˇbˇbˇ aˇaaˇbaaa
+            ˇ    ˇbˇaaˇa ˇbˇbˇb
+            ˇ   
+            ˇb
             "};
 
+        for count in 1..=3 {
             cx.assert_binding_matches_all([&count.to_string(), "shift-f", "b"], test_case)
                 .await;
 
@@ -1014,10 +1031,10 @@ mod test {
 
     #[gpui::test]
     async fn test_percent(cx: &mut gpui::TestAppContext) {
-        let mut cx = NeovimBackedTestContext::new(cx).await;
-        for count in 1..=2 {
-            // let test_case = indoc! {"
-            //     "}
-        }
+        let mut cx = NeovimBackedTestContext::new(cx).await.binding(["%"]);
+        cx.assert_all("ˇconsole.logˇ(ˇvaˇrˇ)ˇ;").await;
+        cx.assert_all("ˇconsole.logˇ(ˇ'var', ˇ[ˇ1, ˇ2, 3ˇ]ˇ)ˇ;")
+            .await;
+        cx.assert_all("let result = curried_funˇ(ˇ)ˇ(ˇ)ˇ;").await;
     }
 }

crates/vim/test_data/test_o.json 🔗

@@ -1 +1 @@
-[{"Text":"\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n\nbrown fox\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\n\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\njumps over\n"},{"Mode":"Insert"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Insert"},{"Text":"The quick\n\n\nbrown fox"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"},{"Text":"fn test() {\n    println!();\n    \n}\n"},{"Mode":"Insert"},{"Selection":{"start":[2,4],"end":[2,4]}},{"Mode":"Insert"},{"Text":"fn test() {\n\n    println!();\n}"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"}]
+[{"Text":"\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\n\nbrown fox\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[1,0],"end":[1,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\n\njumps over"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"},{"Text":"The quick\nbrown fox\njumps over\n"},{"Mode":"Insert"},{"Selection":{"start":[3,0],"end":[3,0]}},{"Mode":"Insert"},{"Text":"The quick\n\n\nbrown fox"},{"Mode":"Insert"},{"Selection":{"start":[2,0],"end":[2,0]}},{"Mode":"Insert"}]