vim: Add C preprocessor check in matching function (#55515)

Juan Pablo Briones created 3 days ago

Closes #24820

This PR fixes the bug specified in issue
https://github.com/zed-industries/zed/issues/24820, now the matching
function checks if the cursor is above a comment or a directive before
defaulting to a bracket range as neovim does.

It also fixes fixes the `line_end` calculations so that when `%` is
pressed inside a bracket range


https://github.com/user-attachments/assets/f59daa6f-9769-45e8-bb8c-2d533470b59d

Release Notes:

- `fn matching()` checks for `preprocessor directives` or `comments`
before defaulting to any bracket range.
- In `fn matching()`line_end calculations avoid expanding a blank
current line into start..EOF.

Change summary

crates/vim/src/motion.rs                                        | 137 +-
crates/vim/test_data/test_matching_comments.json                |   3 
crates/vim/test_data/test_matching_preprocessor_directives.json |  14 
3 files changed, 96 insertions(+), 58 deletions(-)

Detailed changes

crates/vim/src/motion.rs

@@ -2452,7 +2452,7 @@ fn find_matching_bracket_text_based(
         .find_map(|(ch, char_offset)| get_bracket_pair(ch).map(|info| (info, char_offset)));
 
     if bracket_info.is_none() {
-        return find_matching_c_preprocessor_directive(map, line_range);
+        return find_matching_c_preprocessor_directive(map, line_range, offset);
     }
 
     let (open, close, is_opening) = bracket_info?.0;
@@ -2489,18 +2489,20 @@ fn find_matching_bracket_text_based(
 fn find_matching_c_preprocessor_directive(
     map: &DisplaySnapshot,
     line_range: Range<MultiBufferOffset>,
+    offset: MultiBufferOffset,
 ) -> Option<MultiBufferOffset> {
     let line_start = map
         .buffer_chars_at(line_range.start)
         .skip_while(|(c, _)| *c == ' ' || *c == '\t')
+        .take_while(|(c, char_offset)| *char_offset < line_range.end && !c.is_whitespace())
         .map(|(c, _)| c)
-        .take(6)
         .collect::<String>();
 
-    if line_start.starts_with("#if")
-        || line_start.starts_with("#else")
-        || line_start.starts_with("#elif")
-    {
+    if line_range.start + line_start.len() < offset {
+        return None;
+    }
+
+    if line_start.starts_with("#if") || line_start.starts_with("#el") {
         let mut depth = 0i32;
         for (ch, char_offset) in map.buffer_chars_at(line_range.end) {
             if ch != '\n' {
@@ -2618,8 +2620,30 @@ fn matching(
 
     // 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);
+    let max_point = map.max_point().to_point(map);
+
+    // Only widen to EOF when the cursor is actually at EOF.
+    // This avoids expanding a blank current line into start..EOF.
+    if line_end == point && point == max_point {
+        line_end = max_point;
+    }
+
+    let line_range = map.prev_line_boundary(point).0..line_end;
+    let line_range = line_range.start.to_offset(&map.buffer_snapshot())
+        ..line_range.end.to_offset(&map.buffer_snapshot());
+
+    if let Some(preproc_range) = find_matching_c_preprocessor_directive(map, line_range, offset) {
+        return preproc_range.to_display_point(map);
+    }
+
+    if let Some((open_range, close_range)) = comment_delimiter_pair(map, offset) {
+        if open_range.contains(&offset) {
+            return close_range.start.to_display_point(map);
+        }
+
+        if close_range.contains(&offset) {
+            return open_range.start.to_display_point(map);
+        }
     }
 
     let is_quote_char = |ch: char| matches!(ch, '\'' | '"' | '`');
@@ -2729,32 +2753,6 @@ fn matching(
             continue;
         }
 
-        if let Some((open_range, close_range)) = comment_delimiter_pair(map, offset) {
-            if open_range.contains(&offset) {
-                return close_range.start.to_display_point(map);
-            }
-
-            if close_range.contains(&offset) {
-                return open_range.start.to_display_point(map);
-            }
-
-            let open_candidate = (open_range.start >= offset
-                && line_range.contains(&open_range.start))
-            .then_some((open_range.start.saturating_sub(offset), close_range.start));
-
-            let close_candidate = (close_range.start >= offset
-                && line_range.contains(&close_range.start))
-            .then_some((close_range.start.saturating_sub(offset), open_range.start));
-
-            if let Some((_, destination)) = [open_candidate, close_candidate]
-                .into_iter()
-                .flatten()
-                .min_by_key(|(distance, _)| *distance)
-            {
-                return destination.to_display_point(map);
-            }
-        }
-
         closest_pair_destination
             .map(|destination| destination.to_display_point(map))
             .unwrap_or_else(|| {
@@ -3663,6 +3661,10 @@ mod test {
         cx.shared_state().await.assert_eq(indoc! {r"/*
           this is a comment
         ˇ*/"});
+        cx.simulate_shared_keystrokes("k %").await;
+        cx.shared_state().await.assert_eq(indoc! {r"/*
+        ˇ  this is a comment
+        */"});
 
         cx.set_shared_state("ˇ// comment").await;
         cx.simulate_shared_keystrokes("%").await;
@@ -3673,48 +3675,53 @@ mod test {
     async fn test_matching_preprocessor_directives(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;
 
-        cx.set_shared_state(indoc! {r"#ˇif
+        cx.set_shared_state(indoc! {r"
+          #ˇif
 
-            #else
+          #else
 
-            #endif
-            "})
+          #endif
+        "})
             .await;
         cx.simulate_shared_keystrokes("%").await;
-        cx.shared_state().await.assert_eq(indoc! {r"#if
+        cx.shared_state().await.assert_eq(indoc! {r"
+          #if
 
           ˇ#else
 
           #endif
-          "});
+        "});
 
         cx.simulate_shared_keystrokes("%").await;
-        cx.shared_state().await.assert_eq(indoc! {r"#if
+        cx.shared_state().await.assert_eq(indoc! {r"
+          #if
 
           #else
 
           ˇ#endif
-          "});
+        "});
 
         cx.simulate_shared_keystrokes("%").await;
-        cx.shared_state().await.assert_eq(indoc! {r"ˇ#if
+        cx.shared_state().await.assert_eq(indoc! {r"
+          ˇ#if
 
           #else
 
           #endif
-          "});
+        "});
 
         cx.set_shared_state(indoc! {r"
-            #ˇif
-              #if
-
-              #else
-
-              #endif
+          #ˇif
+            #if
 
             #else
+
             #endif
-            "})
+
+          #else
+
+          #endif
+        "})
             .await;
 
         cx.simulate_shared_keystrokes("%").await;
@@ -3727,8 +3734,9 @@ mod test {
               #endif
 
             ˇ#else
+
             #endif
-            "});
+          "});
 
         cx.simulate_shared_keystrokes("% %").await;
         cx.shared_state().await.assert_eq(indoc! {r"
@@ -3740,8 +3748,9 @@ mod test {
               #endif
 
             #else
+
             #endif
-            "});
+          "});
         cx.simulate_shared_keystrokes("j % % %").await;
         cx.shared_state().await.assert_eq(indoc! {r"
             #if
@@ -3752,8 +3761,28 @@ mod test {
               #endif
 
             #else
+
             #endif
-            "});
+          "});
+
+        cx.set_shared_state(indoc! {r"
+          #if definedˇ(something)
+
+          #endif
+        "})
+            .await;
+        cx.simulate_shared_keystrokes("%").await;
+        cx.shared_state().await.assert_eq(indoc! {r"
+          #if defined(somethingˇ)
+
+          #endif
+        "});
+        cx.simulate_shared_keystrokes("0 %").await;
+        cx.shared_state().await.assert_eq(indoc! {r"
+          #if defined(something)
+
+          ˇ#endif
+        "});
     }
 
     #[gpui::test]

crates/vim/test_data/test_matching_comments.json

@@ -5,6 +5,9 @@
 {"Get":{"state":"ˇ/*\n  this is a comment\n*/","mode":"Normal"}}
 {"Key":"%"}
 {"Get":{"state":"/*\n  this is a comment\nˇ*/","mode":"Normal"}}
+{"Key":"k"}
+{"Key":"%"}
+{"Get":{"state":"/*\nˇ  this is a comment\n*/","mode":"Normal"}}
 {"Put":{"state":"ˇ// comment"}}
 {"Key":"%"}
 {"Get":{"state":"ˇ// comment","mode":"Normal"}}

crates/vim/test_data/test_matching_preprocessor_directives.json

@@ -5,14 +5,20 @@
 {"Get":{"state":"#if\n\n#else\n\nˇ#endif\n","mode":"Normal"}}
 {"Key":"%"}
 {"Get":{"state":"ˇ#if\n\n#else\n\n#endif\n","mode":"Normal"}}
-{"Put":{"state":"#ˇif\n  #if\n\n  #else\n\n  #endif\n\n#else\n#endif\n"}}
+{"Put":{"state":"#ˇif\n  #if\n\n  #else\n\n  #endif\n\n#else\n\n#endif\n"}}
 {"Key":"%"}
-{"Get":{"state":"#if\n  #if\n\n  #else\n\n  #endif\n\nˇ#else\n#endif\n","mode":"Normal"}}
+{"Get":{"state":"#if\n  #if\n\n  #else\n\n  #endif\n\nˇ#else\n\n#endif\n","mode":"Normal"}}
 {"Key":"%"}
 {"Key":"%"}
-{"Get":{"state":"ˇ#if\n  #if\n\n  #else\n\n  #endif\n\n#else\n#endif\n","mode":"Normal"}}
+{"Get":{"state":"ˇ#if\n  #if\n\n  #else\n\n  #endif\n\n#else\n\n#endif\n","mode":"Normal"}}
 {"Key":"j"}
 {"Key":"%"}
 {"Key":"%"}
 {"Key":"%"}
-{"Get":{"state":"#if\n  ˇ#if\n\n  #else\n\n  #endif\n\n#else\n#endif\n","mode":"Normal"}}
+{"Get":{"state":"#if\n  ˇ#if\n\n  #else\n\n  #endif\n\n#else\n\n#endif\n","mode":"Normal"}}
+{"Put":{"state":"#if definedˇ(something)\n\n#endif\n"}}
+{"Key":"%"}
+{"Get":{"state":"#if defined(somethingˇ)\n\n#endif\n","mode":"Normal"}}
+{"Key":"0"}
+{"Key":"%"}
+{"Get":{"state":"#if defined(something)\n\nˇ#endif\n","mode":"Normal"}}