From 092c7058a49e4386afbb935740f0c220c3cafde0 Mon Sep 17 00:00:00 2001 From: Juan Pablo Briones Date: Mon, 6 Apr 2026 22:51:54 -0400 Subject: [PATCH] vim: Fix % for multiline comments and preprocessor directives (#53148) Implements: [49806](https://github.com/zed-industries/zed/discussions/49806) Closes: [24820](https://github.com/zed-industries/zed/issues/24820) Zeds impl of `%` didn't handle preprocessor directives and multiline To implement this feature for multiline comment, a tree-sitter query is used to check if we are inside a comment range and then replicate the logic used in brackets. For preprocessor directives using `TextObjects` wasn't a option, so it was implemented through a text based query that searches for the next preprocessor directives. Using text based queries might not be the best for performance, so I'm open to any suggestions. Release Notes: - Fixed vim's matching '%' to handle multiline comments `/* */` and preprocessor directives `#if #else #endif`. --- crates/vim/src/motion.rs | 261 +++++++++++++++++- .../vim/test_data/test_matching_comments.json | 10 + ...test_matching_preprocessor_directives.json | 18 ++ 3 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 crates/vim/test_data/test_matching_comments.json create mode 100644 crates/vim/test_data/test_matching_preprocessor_directives.json diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 6bf2afd09ae07ff8453a481a8d6e6e6a254e670f..6e992704f54bf7aba3cc775d906a90281234dbd0 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -7,7 +7,7 @@ use editor::{ }, }; use gpui::{Action, Context, Window, actions, px}; -use language::{CharKind, Point, Selection, SelectionGoal}; +use language::{CharKind, Point, Selection, SelectionGoal, TextObject, TreeSitterOptions}; use multi_buffer::MultiBufferRow; use schemars::JsonSchema; use serde::Deserialize; @@ -2451,6 +2451,10 @@ fn find_matching_bracket_text_based( .take_while(|(_, char_offset)| *char_offset < line_range.end) .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); + } + let (open, close, is_opening) = bracket_info?.0; let bracket_offset = bracket_info?.1; @@ -2482,6 +2486,122 @@ fn find_matching_bracket_text_based( None } +fn find_matching_c_preprocessor_directive( + map: &DisplaySnapshot, + line_range: Range, +) -> Option { + let line_start = map + .buffer_chars_at(line_range.start) + .skip_while(|(c, _)| *c == ' ' || *c == '\t') + .map(|(c, _)| c) + .take(6) + .collect::(); + + if line_start.starts_with("#if") + || line_start.starts_with("#else") + || line_start.starts_with("#elif") + { + let mut depth = 0i32; + for (ch, char_offset) in map.buffer_chars_at(line_range.end) { + if ch != '\n' { + continue; + } + let mut line_offset = char_offset + '\n'.len_utf8(); + + // Skip leading whitespace + map.buffer_chars_at(line_offset) + .take_while(|(c, _)| *c == ' ' || *c == '\t') + .for_each(|(_, _)| line_offset += 1); + + // Check what directive starts the next line + let next_line_start = map + .buffer_chars_at(line_offset) + .map(|(c, _)| c) + .take(6) + .collect::(); + + if next_line_start.starts_with("#if") { + depth += 1; + } else if next_line_start.starts_with("#endif") { + if depth > 0 { + depth -= 1; + } else { + return Some(line_offset); + } + } else if next_line_start.starts_with("#else") || next_line_start.starts_with("#elif") { + if depth == 0 { + return Some(line_offset); + } + } + } + } else if line_start.starts_with("#endif") { + let mut depth = 0i32; + for (ch, char_offset) in + map.reverse_buffer_chars_at(line_range.start.saturating_sub_usize(1)) + { + let mut line_offset = if char_offset == MultiBufferOffset(0) { + MultiBufferOffset(0) + } else if ch != '\n' { + continue; + } else { + char_offset + '\n'.len_utf8() + }; + + // Skip leading whitespace + map.buffer_chars_at(line_offset) + .take_while(|(c, _)| *c == ' ' || *c == '\t') + .for_each(|(_, _)| line_offset += 1); + + // Check what directive starts this line + let line_start = map + .buffer_chars_at(line_offset) + .skip_while(|(c, _)| *c == ' ' || *c == '\t') + .map(|(c, _)| c) + .take(6) + .collect::(); + + if line_start.starts_with("\n\n") { + // empty line + continue; + } else if line_start.starts_with("#endif") { + depth += 1; + } else if line_start.starts_with("#if") { + if depth > 0 { + depth -= 1; + } else { + return Some(line_offset); + } + } + } + } + None +} + +fn comment_delimiter_pair( + map: &DisplaySnapshot, + offset: MultiBufferOffset, +) -> Option<(Range, Range)> { + let snapshot = map.buffer_snapshot(); + snapshot + .text_object_ranges(offset..offset, TreeSitterOptions::default()) + .find_map(|(range, obj)| { + if !matches!(obj, TextObject::InsideComment | TextObject::AroundComment) + || !range.contains(&offset) + { + return None; + } + + let mut chars = snapshot.chars_at(range.start); + if (Some('/'), Some('*')) != (chars.next(), chars.next()) { + return None; + } + + let open_range = range.start..range.start + 2usize; + let close_range = range.end - 2..range.end; + Some((open_range, close_range)) + }) +} + fn matching( map: &DisplaySnapshot, display_point: DisplayPoint, @@ -2609,6 +2729,32 @@ 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(|| { @@ -3497,6 +3643,119 @@ mod test { ); } + #[gpui::test] + async fn test_matching_comments(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + + cx.set_shared_state(indoc! {r"ˇ/* + this is a comment + */"}) + .await; + cx.simulate_shared_keystrokes("%").await; + cx.shared_state().await.assert_eq(indoc! {r"/* + this is a comment + ˇ*/"}); + cx.simulate_shared_keystrokes("%").await; + cx.shared_state().await.assert_eq(indoc! {r"ˇ/* + this is a comment + */"}); + cx.simulate_shared_keystrokes("%").await; + cx.shared_state().await.assert_eq(indoc! {r"/* + this is a comment + ˇ*/"}); + + cx.set_shared_state("ˇ// comment").await; + cx.simulate_shared_keystrokes("%").await; + cx.shared_state().await.assert_eq("ˇ// comment"); + } + + #[gpui::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 + + #else + + #endif + "}) + .await; + cx.simulate_shared_keystrokes("%").await; + 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 + + #else + + ˇ#endif + "}); + + cx.simulate_shared_keystrokes("%").await; + cx.shared_state().await.assert_eq(indoc! {r"ˇ#if + + #else + + #endif + "}); + + cx.set_shared_state(indoc! {r" + #ˇif + #if + + #else + + #endif + + #else + #endif + "}) + .await; + + cx.simulate_shared_keystrokes("%").await; + cx.shared_state().await.assert_eq(indoc! {r" + #if + #if + + #else + + #endif + + ˇ#else + #endif + "}); + + cx.simulate_shared_keystrokes("% %").await; + cx.shared_state().await.assert_eq(indoc! {r" + ˇ#if + #if + + #else + + #endif + + #else + #endif + "}); + cx.simulate_shared_keystrokes("j % % %").await; + cx.shared_state().await.assert_eq(indoc! {r" + #if + ˇ#if + + #else + + #endif + + #else + #endif + "}); + } + #[gpui::test] async fn test_unmatched_forward(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; diff --git a/crates/vim/test_data/test_matching_comments.json b/crates/vim/test_data/test_matching_comments.json new file mode 100644 index 0000000000000000000000000000000000000000..7fcf5e46e1ea16f2be794ff76b583242b33aabc0 --- /dev/null +++ b/crates/vim/test_data/test_matching_comments.json @@ -0,0 +1,10 @@ +{"Put":{"state":"ˇ/*\n this is a comment\n*/"}} +{"Key":"%"} +{"Get":{"state":"/*\n this is a comment\nˇ*/","mode":"Normal"}} +{"Key":"%"} +{"Get":{"state":"ˇ/*\n this is a comment\n*/","mode":"Normal"}} +{"Key":"%"} +{"Get":{"state":"/*\n this is a comment\nˇ*/","mode":"Normal"}} +{"Put":{"state":"ˇ// comment"}} +{"Key":"%"} +{"Get":{"state":"ˇ// comment","mode":"Normal"}} diff --git a/crates/vim/test_data/test_matching_preprocessor_directives.json b/crates/vim/test_data/test_matching_preprocessor_directives.json new file mode 100644 index 0000000000000000000000000000000000000000..9f0bd9792ee8dad5029f4ecaf325c231755530e1 --- /dev/null +++ b/crates/vim/test_data/test_matching_preprocessor_directives.json @@ -0,0 +1,18 @@ +{"Put":{"state":"#ˇif\n\n#else\n\n#endif\n"}} +{"Key":"%"} +{"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"}} +{"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"}} +{"Key":"%"} +{"Get":{"state":"#if\n #if\n\n #else\n\n #endif\n\nˇ#else\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"}} +{"Key":"j"} +{"Key":"%"} +{"Key":"%"} +{"Key":"%"} +{"Get":{"state":"#if\n ˇ#if\n\n #else\n\n #endif\n\n#else\n#endif\n","mode":"Normal"}}