From 9e260eb2356f8475c3b6807a4df1a0e948ba6e5d Mon Sep 17 00:00:00 2001 From: dinocosta Date: Fri, 4 Apr 2025 17:37:44 -0700 Subject: [PATCH 1/5] feat(vim): add support for g flag in substitute command - 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 ocurrence instead of all ocurrences in the line. --- crates/search/src/buffer_search.rs | 2 +- crates/vim/src/normal/search.rs | 55 ++++++++++++++++++------------ 2 files changed, 35 insertions(+), 22 deletions(-) diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index e9a837b16802ce..f79e75dadb1d16 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -1421,7 +1421,7 @@ impl BufferSearchBar { } } - fn replace_next(&mut self, _: &ReplaceNext, window: &mut Window, cx: &mut Context) { + pub fn replace_next(&mut self, _: &ReplaceNext, window: &mut Window, cx: &mut Context) { let mut should_propagate = true; if !self.dismissed && self.active_search.is_some() { if let Some(searchable_item) = self.active_searchable_item.as_ref() { diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 8eb22f2fe2b51a..65a47981152841 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -479,26 +479,39 @@ impl Vim { 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, - ) - }); + } else { + // TODO: Confirm whether we want to have the same + // behaviour as the regular search bar, where the next + // ocurrence, relative to the cursor, is replaced, or if + // we want to have the same behaviour as NeoVim, where + // regardless of the cursor position, the first + // ocurrence in the line is the one that gets replaced. + // For example, for the string "this and this and this", + // if the cursor is in the second "this", then a Zed + // search and replace will replace the second "this", + // while NeoVim would replace the first one. + search_bar.replace_next(&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, + ) + }); })?; anyhow::Ok(()) }) @@ -564,13 +577,13 @@ 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' | 'I' => replacement.should_replace_all = true, 'c' | 'n' => replacement.should_replace_all = false, 'i' => replacement.is_case_sensitive = false, _ => {} From ab4be19bca661f2493d2813802b80a0e161d161a Mon Sep 17 00:00:00 2001 From: dinocosta Date: Fri, 4 Apr 2025 18:37:34 -0700 Subject: [PATCH 2/5] refactor(vim): update substitute command default behaviour Update the default behaviour of the substitute command in vim mode to match what NeoVim does when replacing only one ocurrence of a string, where it replaces only the first ocurrence in the line, instead of the first ocurrence relative to the cursor. - Introduce BufferSarchBar::select_first_match to update the active match index to 0, so as to be able to select the first match in the line, regardless of the cursor's position. - Update the Vim::replace_command so as to call BufferSearchBar::select_first_match if should_replace_all is set to false, before replacing the next match with BufferSearchBar::replace_next . --- crates/search/src/buffer_search.rs | 15 +++++++++++++++ crates/vim/src/normal/search.rs | 11 +---------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index f79e75dadb1d16..40364fddd6dca1 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -1056,6 +1056,21 @@ impl BufferSearchBar { } } + pub fn select_first_match(&mut self, _window: &mut Window, _cx: &mut Context) { + if let Some(searchable_item) = self.active_searchable_item.as_ref() { + if let Some(matches) = self + .searchable_items_with_matches + .get(&searchable_item.downgrade()) + { + if matches.is_empty() { + return; + } + + self.active_match_index = Some(0); + } + } + } + pub fn select_last_match(&mut self, window: &mut Window, cx: &mut Context) { if let Some(searchable_item) = self.active_searchable_item.as_ref() { if let Some(matches) = self diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 65a47981152841..8d337f33bccc96 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -480,16 +480,7 @@ impl Vim { search_bar.select_last_match(window, cx); search_bar.replace_all(&Default::default(), window, cx); } else { - // TODO: Confirm whether we want to have the same - // behaviour as the regular search bar, where the next - // ocurrence, relative to the cursor, is replaced, or if - // we want to have the same behaviour as NeoVim, where - // regardless of the cursor position, the first - // ocurrence in the line is the one that gets replaced. - // For example, for the string "this and this and this", - // if the cursor is in the second "this", then a Zed - // search and replace will replace the second "this", - // while NeoVim would replace the first one. + search_bar.select_first_match(window, cx); search_bar.replace_next(&Default::default(), window, cx); } From 4d209ef11e08619abfb787594e2417b7d21a013a Mon Sep 17 00:00:00 2001 From: dinocosta Date: Mon, 7 Apr 2025 21:54:18 -0700 Subject: [PATCH 3/5] fix: correctly support replacing single match per line This commit introduces the necessary changes in order to support replacing only the first match per line when using vim's subsitute command, for example, :s/{pattern}/{string}/ . In order to support this, the following changes have been introduced: - Update `SearchQuery::Regex` with a new field, `one_match_per_line`, which controls whether all matches will be replaced when using replace all, or if only one match per line should be replaced. - Add `ONE_MATCH_PER_LINE` to `SearchOptions` struct. - Update `Vim::replace_command` to set the `SearchOptions::ONE_MATCH_PER_LINE` option, if `replacement.should_replace_all` is false. - Update `SearchQuery::regex` method in order to expect the `one_match_per_line` argument to be provided, allowing callers to specify whether the search query should be used to replace only one match per line. Lastly, since they're no longer necessary, the `BufferSearchBar::select_first_match` method has been removed, and the `BufferSearchBar::replace_next` method has been moved to private again. --- .../assistant_tools/src/regex_search_tool.rs | 1 + .../src/chat_panel/message_editor.rs | 1 + crates/editor/src/items.rs | 14 ++++++++++++++ crates/project/src/search.rs | 17 +++++++++++++++++ crates/search/src/buffer_search.rs | 19 +++---------------- crates/search/src/project_search.rs | 2 ++ crates/search/src/search.rs | 1 + crates/vim/src/normal/search.rs | 14 +++++++------- 8 files changed, 46 insertions(+), 23 deletions(-) diff --git a/crates/assistant_tools/src/regex_search_tool.rs b/crates/assistant_tools/src/regex_search_tool.rs index 948afcf1fd83f1..635a741df0555d 100644 --- a/crates/assistant_tools/src/regex_search_tool.rs +++ b/crates/assistant_tools/src/regex_search_tool.rs @@ -96,6 +96,7 @@ impl Tool for RegexSearchTool { false, false, false, + false, PathMatcher::default(), PathMatcher::default(), None, diff --git a/crates/collab_ui/src/chat_panel/message_editor.rs b/crates/collab_ui/src/chat_panel/message_editor.rs index 31d25f0054bd79..74d9335071b5fc 100644 --- a/crates/collab_ui/src/chat_panel/message_editor.rs +++ b/crates/collab_ui/src/chat_panel/message_editor.rs @@ -34,6 +34,7 @@ static MENTIONS_SEARCH: LazyLock = LazyLock::new(|| { false, false, false, + false, Default::default(), Default::default(), None, diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index b1cf73b442822e..bebd21ab0e234a 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1535,8 +1535,22 @@ impl SearchableItem for Editor { let text = self.buffer.read(cx); let text = text.snapshot(cx); let mut edits = vec![]; + let mut last_point: Point = Point::default(); + for m in matches { + let point = m.start.to_point(&text); let text = text.text_for_range(m.clone()).collect::>(); + + // 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 point.row != last_point.row { + last_point = 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 { diff --git a/crates/project/src/search.rs b/crates/project/src/search.rs index 917eb2ed16e8e6..06745c82f42760 100644 --- a/crates/project/src/search.rs +++ b/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>>, @@ -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 { + 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 { diff --git a/crates/search/src/buffer_search.rs b/crates/search/src/buffer_search.rs index 40364fddd6dca1..c0815c51254e5d 100644 --- a/crates/search/src/buffer_search.rs +++ b/crates/search/src/buffer_search.rs @@ -1056,21 +1056,6 @@ impl BufferSearchBar { } } - pub fn select_first_match(&mut self, _window: &mut Window, _cx: &mut Context) { - if let Some(searchable_item) = self.active_searchable_item.as_ref() { - if let Some(matches) = self - .searchable_items_with_matches - .get(&searchable_item.downgrade()) - { - if matches.is_empty() { - return; - } - - self.active_match_index = Some(0); - } - } - } - pub fn select_last_match(&mut self, window: &mut Window, cx: &mut Context) { if let Some(searchable_item) = self.active_searchable_item.as_ref() { if let Some(matches) = self @@ -1246,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, @@ -1436,7 +1423,7 @@ impl BufferSearchBar { } } - pub fn replace_next(&mut self, _: &ReplaceNext, window: &mut Window, cx: &mut Context) { + fn replace_next(&mut self, _: &ReplaceNext, window: &mut Window, cx: &mut Context) { let mut should_propagate = true; if !self.dismissed && self.active_search.is_some() { if let Some(searchable_item) = self.active_searchable_item.as_ref() { diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 571ae967291bcd..23e51054d60fa1 100644 --- a/crates/search/src/project_search.rs +++ b/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, diff --git a/crates/search/src/search.rs b/crates/search/src/search.rs index 593950d4e408e8..b8aa6c42b86b9a 100644 --- a/crates/search/src/search.rs +++ b/crates/search/src/search.rs @@ -47,6 +47,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; } diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 8d337f33bccc96..a671eca7c5a29f 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -468,6 +468,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,13 +481,8 @@ 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); - } else { - search_bar.select_first_match(window, cx); - search_bar.replace_next(&Default::default(), 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() From cbb54f708f6504329910a7732f4626bae3047f91 Mon Sep 17 00:00:00 2001 From: dinocosta Date: Tue, 8 Apr 2025 11:16:00 -0700 Subject: [PATCH 4/5] fix(vim): % range in substitution and search options reset Fix two issues introduced with the recent changes that update :s/{pattern}/{string} to allow only replacing the first occurrence on each line, namely: - Fix `:%s/{pattern}/{string}` to actually work on all lines, instead of only from line 1. This issue was happening because the default point we were checking against in `editor::items::replace_all` was having its `row` value set to `0`, which would always end up being the same as the first line, so that line would be skipped if the `one_match_per_line` option was set to true. - Disabled the search bar's `ONE_MATCH_PER_LINE` search option after running vim's replace command, as having leaving this enabled would cause the "Replace All Matches" functionality to only replace the first occurrence on each line, and I'm not sure if this is something Zed wants to eventually support and add an icon for in the search bar. --- crates/editor/src/items.rs | 8 +++++--- crates/vim/src/normal/search.rs | 10 +++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index bebd21ab0e234a..f9c97506825c32 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1535,7 +1535,7 @@ impl SearchableItem for Editor { let text = self.buffer.read(cx); let text = text.snapshot(cx); let mut edits = vec![]; - let mut last_point: Point = Point::default(); + let mut last_point: Option = None; for m in matches { let point = m.start.to_point(&text); @@ -1545,8 +1545,10 @@ impl SearchableItem for Editor { // 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 point.row != last_point.row { - last_point = point; + 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; } diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index a671eca7c5a29f..1ee6f9e09ee29c 100644 --- a/crates/vim/src/normal/search.rs +++ b/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::() 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) } @@ -503,6 +504,13 @@ impl Vim { 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(()) }) From cdcfe4d8988f0b768dfb05c4ee646564cefcaab3 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 8 Apr 2025 12:49:33 -0600 Subject: [PATCH 5/5] Fix I --- crates/vim/src/normal/search.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/vim/src/normal/search.rs b/crates/vim/src/normal/search.rs index 1ee6f9e09ee29c..4cbc50cc1df798 100644 --- a/crates/vim/src/normal/search.rs +++ b/crates/vim/src/normal/search.rs @@ -582,9 +582,10 @@ impl Replacement { for c in flags.chars() { match c { - 'g' | 'I' => replacement.should_replace_all = true, + '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, _ => {} } }