Update default vim substitute command behavior and add support for 'g' flag (#28138)

Dino and Conrad Irwin created

This Pull Request updates the default behavior of the substitute (`s`)
command in vim mode to only replace the next match by default, instead
of all, and replace all matches only when the `g` flag is provided,
making it more similar to NeoVim's behavior.

In order to achieve this, the following changes were introduced:

- Update `BufferSearchBar::replace_next` to be a public method, so it
can be called from `Vim::replace_command` .
- Update the `Replacement::parse` to set the `should_replace_all` field
to `false` by default, and only set it to `true` if the `'g'` flag is
present in the query.
- Add support for when the `Replacement.should_replace_all` is set to
`false` in `Vim::replace_command`, so as to have it only replace the
next occurrence instead of all occurrences in the line.
- Introduce `BufferSearchBar::select_first_match` so as to activate the
first match on the line under the cursor.

Closes #24450 

Release Notes:

- Improved vim's substitute command so as to only replace the first
match by default, and replace all matches if the `'g'` flag is provided

---------

Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>

Change summary

crates/assistant_tools/src/regex_search_tool.rs   |  1 
crates/collab_ui/src/chat_panel/message_editor.rs |  1 
crates/editor/src/items.rs                        | 16 ++++
crates/project/src/search.rs                      | 17 ++++
crates/search/src/buffer_search.rs                |  2 
crates/search/src/project_search.rs               |  2 
crates/search/src/search.rs                       |  1 
crates/vim/src/normal/search.rs                   | 65 ++++++++++------
8 files changed, 79 insertions(+), 26 deletions(-)

Detailed changes

crates/editor/src/items.rs 🔗

@@ -1540,8 +1540,24 @@ impl SearchableItem for Editor {
         let text = self.buffer.read(cx);
         let text = text.snapshot(cx);
         let mut edits = vec![];
+        let mut last_point: Option<Point> = None;
+
         for m in matches {
+            let point = m.start.to_point(&text);
             let text = text.text_for_range(m.clone()).collect::<Vec<_>>();
+
+            // Check if the row for the current match is different from the last
+            // match. If that's not the case and we're still replacing matches
+            // in the same row/line, skip this match if the `one_match_per_line`
+            // option is enabled.
+            if last_point.is_none() {
+                last_point = Some(point);
+            } else if last_point.is_some() && point.row != last_point.unwrap().row {
+                last_point = Some(point);
+            } else if query.one_match_per_line().is_some_and(|enabled| enabled) {
+                continue;
+            }
+
             let text: Cow<_> = if text.len() == 1 {
                 text.first().cloned().unwrap().into()
             } else {

crates/project/src/search.rs 🔗

@@ -71,6 +71,7 @@ pub enum SearchQuery {
         whole_word: bool,
         case_sensitive: bool,
         include_ignored: bool,
+        one_match_per_line: bool,
         inner: SearchInputs,
     },
 }
@@ -116,6 +117,7 @@ impl SearchQuery {
         whole_word: bool,
         case_sensitive: bool,
         include_ignored: bool,
+        one_match_per_line: bool,
         files_to_include: PathMatcher,
         files_to_exclude: PathMatcher,
         buffers: Option<Vec<Entity<Buffer>>>,
@@ -156,6 +158,7 @@ impl SearchQuery {
             case_sensitive,
             include_ignored,
             inner,
+            one_match_per_line,
         })
     }
 
@@ -166,6 +169,7 @@ impl SearchQuery {
                 message.whole_word,
                 message.case_sensitive,
                 message.include_ignored,
+                false,
                 deserialize_path_matches(&message.files_to_include)?,
                 deserialize_path_matches(&message.files_to_exclude)?,
                 None, // search opened only don't need search remote
@@ -459,6 +463,19 @@ impl SearchQuery {
             Self::Regex { inner, .. } | Self::Text { inner, .. } => inner,
         }
     }
+
+    /// Whether this search should replace only one match per line, instead of
+    /// all matches.
+    /// Returns `None` for text searches, as only regex searches support this
+    /// option.
+    pub fn one_match_per_line(&self) -> Option<bool> {
+        match self {
+            Self::Regex {
+                one_match_per_line, ..
+            } => Some(*one_match_per_line),
+            Self::Text { .. } => None,
+        }
+    }
 }
 
 pub fn deserialize_path_matches(glob_set: &str) -> anyhow::Result<PathMatcher> {

crates/search/src/buffer_search.rs 🔗

@@ -1231,6 +1231,8 @@ impl BufferSearchBar {
                             self.search_options.contains(SearchOptions::WHOLE_WORD),
                             self.search_options.contains(SearchOptions::CASE_SENSITIVE),
                             false,
+                            self.search_options
+                                .contains(SearchOptions::ONE_MATCH_PER_LINE),
                             Default::default(),
                             Default::default(),
                             None,

crates/search/src/project_search.rs 🔗

@@ -1053,6 +1053,8 @@ impl ProjectSearchView {
                 self.search_options.contains(SearchOptions::WHOLE_WORD),
                 self.search_options.contains(SearchOptions::CASE_SENSITIVE),
                 self.search_options.contains(SearchOptions::INCLUDE_IGNORED),
+                self.search_options
+                    .contains(SearchOptions::ONE_MATCH_PER_LINE),
                 included_files,
                 excluded_files,
                 open_buffers,

crates/search/src/search.rs 🔗

@@ -48,6 +48,7 @@ bitflags! {
         const CASE_SENSITIVE = 0b010;
         const INCLUDE_IGNORED = 0b100;
         const REGEX = 0b1000;
+        const ONE_MATCH_PER_LINE = 0b100000;
         /// If set, reverse direction when finding the active match
         const BACKWARDS = 0b10000;
     }

crates/vim/src/normal/search.rs 🔗

@@ -445,6 +445,8 @@ impl Vim {
         }
         let vim = cx.entity().clone();
         pane.update(cx, |pane, cx| {
+            let mut options = SearchOptions::REGEX;
+
             let Some(search_bar) = pane.toolbar().read(cx).item_of_type::<BufferSearchBar>() else {
                 return;
             };
@@ -453,7 +455,6 @@ impl Vim {
                     return None;
                 }
 
-                let mut options = SearchOptions::REGEX;
                 if replacement.is_case_sensitive {
                     options.set(SearchOptions::CASE_SENSITIVE, true)
                 }
@@ -468,6 +469,11 @@ impl Vim {
                         search_bar.is_contains_uppercase(&search),
                     );
                 }
+
+                if !replacement.should_replace_all {
+                    options.set(SearchOptions::ONE_MATCH_PER_LINE, true);
+                }
+
                 search_bar.set_replacement(Some(&replacement.replacement), cx);
                 Some(search_bar.search(&search, Some(options), window, cx))
             });
@@ -476,29 +482,35 @@ impl Vim {
             cx.spawn_in(window, async move |_, cx| {
                 search.await?;
                 search_bar.update_in(cx, |search_bar, window, cx| {
-                    if replacement.should_replace_all {
-                        search_bar.select_last_match(window, cx);
-                        search_bar.replace_all(&Default::default(), window, cx);
-                        cx.spawn(async move |_, cx| {
-                            cx.background_executor()
-                                .timer(Duration::from_millis(200))
-                                .await;
-                            editor
-                                .update(cx, |editor, cx| editor.clear_search_within_ranges(cx))
-                                .ok();
-                        })
-                        .detach();
-                        vim.update(cx, |vim, cx| {
-                            vim.move_cursor(
-                                Motion::StartOfLine {
-                                    display_lines: false,
-                                },
-                                None,
-                                window,
-                                cx,
-                            )
-                        });
-                    }
+                    search_bar.select_last_match(window, cx);
+                    search_bar.replace_all(&Default::default(), window, cx);
+
+                    cx.spawn(async move |_, cx| {
+                        cx.background_executor()
+                            .timer(Duration::from_millis(200))
+                            .await;
+                        editor
+                            .update(cx, |editor, cx| editor.clear_search_within_ranges(cx))
+                            .ok();
+                    })
+                    .detach();
+                    vim.update(cx, |vim, cx| {
+                        vim.move_cursor(
+                            Motion::StartOfLine {
+                                display_lines: false,
+                            },
+                            None,
+                            window,
+                            cx,
+                        )
+                    });
+
+                    // Disable the `ONE_MATCH_PER_LINE` search option when finished, as
+                    // this is not properly supported outside of vim mode, and
+                    // not disabling it makes the "Replace All Matches" button
+                    // actually replace only the first match on each line.
+                    options.set(SearchOptions::ONE_MATCH_PER_LINE, false);
+                    search_bar.set_search_options(options, cx);
                 })?;
                 anyhow::Ok(())
             })
@@ -564,15 +576,16 @@ impl Replacement {
         let mut replacement = Replacement {
             search,
             replacement,
-            should_replace_all: true,
+            should_replace_all: false,
             is_case_sensitive: true,
         };
 
         for c in flags.chars() {
             match c {
-                'g' | 'I' => {}
+                'g' => replacement.should_replace_all = true,
                 'c' | 'n' => replacement.should_replace_all = false,
                 'i' => replacement.is_case_sensitive = false,
+                'I' => replacement.is_case_sensitive = true,
                 _ => {}
             }
         }