diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index 5aad101e08632b119cb86036cb1b8557c82cd702..8bdd4e7ef50feaefc77d5b3ccc9e7bb13ef6c674 100644 --- a/crates/vim/src/normal.rs +++ b/crates/vim/src/normal.rs @@ -563,10 +563,13 @@ impl Vim { waiting_operator = Some(Operator::DeleteSurrounds); } Some(Operator::ChangeSurrounds { target: None, .. }) => { - if self.check_and_move_to_valid_bracket_pair(object, window, cx) { + let bracket_anchors = + self.prepare_and_move_to_valid_bracket_pair(object, window, cx); + if !bracket_anchors.is_empty() { waiting_operator = Some(Operator::ChangeSurrounds { target: Some(object), opening, + bracket_anchors, }); } } diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 661853930c97cdfe6455e10bfec404b3ade2f231..0442387c395544cfd80899b7eeefec0ad8cee1d7 100644 --- a/crates/vim/src/state.rs +++ b/crates/vim/src/state.rs @@ -112,6 +112,8 @@ pub enum Operator { /// Represents whether the opening bracket was used for the target /// object. opening: bool, + /// Computed anchors for the opening and closing bracket characters, + bracket_anchors: Vec>, }, DeleteSurrounds, Mark, diff --git a/crates/vim/src/surrounds.rs b/crates/vim/src/surrounds.rs index 3732475b6b9d97118f8c2b13b4259446a8314387..ace06a5a88defc1e50033b0b0c0178086ea593cc 100644 --- a/crates/vim/src/surrounds.rs +++ b/crates/vim/src/surrounds.rs @@ -4,7 +4,7 @@ use crate::{ object::{Object, surrounding_markers}, state::Mode, }; -use editor::{Bias, MultiBufferOffset, movement}; +use editor::{Anchor, Bias, MultiBufferOffset, ToOffset, movement}; use gpui::{Context, Window}; use language::BracketPair; @@ -275,6 +275,7 @@ impl Vim { text: Arc, target: Object, opening: bool, + bracket_anchors: Vec>, window: &mut Window, cx: &mut Context, ) { @@ -301,94 +302,66 @@ impl Vim { will_replace_pair.start == will_replace_pair.end || !opening; let display_map = editor.display_snapshot(cx); - let selections = editor.selections.all_adjusted_display(&display_map); let mut edits = Vec::new(); - let mut anchors = Vec::new(); - for selection in &selections { - let start = selection.start.to_offset(&display_map, Bias::Left); - if let Some(range) = - target.range(&display_map, selection.clone(), true, None) + // Collect (open_offset, close_offset) pairs to replace from the + // pre-computed anchors stored during check_and_move_to_valid_bracket_pair. + let mut pairs_to_replace: Vec<(MultiBufferOffset, MultiBufferOffset)> = + Vec::new(); + let snapshot = display_map.buffer_snapshot(); + for anchors in &bracket_anchors { + let Some((open_anchor, close_anchor)) = anchors else { + continue; + }; + let pair = ( + open_anchor.to_offset(&snapshot), + close_anchor.to_offset(&snapshot), + ); + if !pairs_to_replace.contains(&pair) { + pairs_to_replace.push(pair); + } + } + + for (open_offset, close_offset) in pairs_to_replace { + let mut open_str = pair.start.clone(); + let mut chars_and_offset = + display_map.buffer_chars_at(open_offset).peekable(); + chars_and_offset.next(); // skip the bracket itself + let mut open_range_end = open_offset + 1usize; + while let Some((next_ch, _)) = chars_and_offset.next() + && next_ch == ' ' { - if !target.is_multiline() { - let is_same_row = selection.start.row() == range.start.row() - && selection.end.row() == range.end.row(); - if !is_same_row { - anchors.push(start..start); - continue; - } + open_range_end += 1; + if preserve_space { + open_str.push(next_ch); } - - // Keeps track of the length of the string that is - // going to be edited on the start so we can ensure - // that the end replacement string does not exceed - // this value. Helpful when dealing with newlines. - let mut edit_len = 0; - let mut open_range_end = MultiBufferOffset(0); - let mut chars_and_offset = display_map - .buffer_chars_at(range.start.to_offset(&display_map, Bias::Left)) - .peekable(); - - while let Some((ch, offset)) = chars_and_offset.next() { - if ch.to_string() == will_replace_pair.start { - let mut open_str = pair.start.clone(); - let start = offset; - open_range_end = start + 1usize; - while let Some((next_ch, _)) = chars_and_offset.next() - && next_ch == ' ' - { - open_range_end += 1; - - if preserve_space { - open_str.push(next_ch); - } - } - - if add_space { - open_str.push(' '); - }; - - edit_len = open_range_end - start; - edits.push((start..open_range_end, open_str)); - anchors.push(start..start); - break; - } + } + if add_space { + open_str.push(' '); + } + let edit_len = open_range_end - open_offset; + edits.push((open_offset..open_range_end, open_str)); + + let mut close_str = String::new(); + let close_end = close_offset + 1usize; + let mut close_start = close_offset; + for (next_ch, _) in display_map.reverse_buffer_chars_at(close_offset) { + if next_ch != ' ' + || close_str.len() >= edit_len - 1 + || close_start <= open_range_end + { + break; } - - let mut reverse_chars_and_offsets = display_map - .reverse_buffer_chars_at( - range.end.to_offset(&display_map, Bias::Left), - ) - .peekable(); - while let Some((ch, offset)) = reverse_chars_and_offsets.next() { - if ch.to_string() == will_replace_pair.end { - let mut close_str = String::new(); - let mut start = offset; - let end = start + 1usize; - while let Some((next_ch, _)) = reverse_chars_and_offsets.next() - && next_ch == ' ' - && close_str.len() < edit_len - 1 - && start > open_range_end - { - start -= 1; - - if preserve_space { - close_str.push(next_ch); - } - } - - if add_space { - close_str.push(' '); - }; - - close_str.push_str(&pair.end); - edits.push((start..end, close_str)); - break; - } + close_start -= 1; + if preserve_space { + close_str.push(next_ch); } - } else { - anchors.push(start..start); } + if add_space { + close_str.push(' '); + } + close_str.push_str(&pair.end); + edits.push((close_start..close_end, close_str)); } let stable_anchors = editor @@ -411,67 +384,83 @@ impl Vim { } } - /// Checks if any of the current cursors are surrounded by a valid pair of brackets. + /// **Only intended for use by the `cs` (change surrounds) operator.** /// - /// This method supports multiple cursors and checks each cursor for a valid pair of brackets. - /// A pair of brackets is considered valid if it is well-formed and properly closed. + /// For each cursor, checks whether it is surrounded by a valid bracket pair for the given + /// object. Moves each cursor to the opening bracket of its found pair, and returns a + /// `Vec>` with one entry per selection containing the pre-computed + /// open and close bracket positions. /// - /// If a valid pair of brackets is found, the method returns `true` and the cursor is automatically moved to the start of the bracket pair. - /// If no valid pair of brackets is found for any cursor, the method returns `false`. - pub fn check_and_move_to_valid_bracket_pair( + /// Storing these anchors avoids re-running the bracket search from the moved cursor position, + /// which can misidentify the opening bracket for symmetric quote characters when the same + /// character appears earlier on the line (e.g. `I'm 'good'`). + /// + /// Returns an empty `Vec` if no valid pair was found for any cursor. + pub fn prepare_and_move_to_valid_bracket_pair( &mut self, object: Object, window: &mut Window, cx: &mut Context, - ) -> bool { - let mut valid = false; + ) -> Vec> { + let mut matched_pair_anchors: Vec> = Vec::new(); if let Some(pair) = self.object_to_bracket_pair(object, cx) { self.update_editor(cx, |_, editor, cx| { editor.transact(window, cx, |editor, window, cx| { editor.set_clip_at_line_ends(false, cx); let display_map = editor.display_snapshot(cx); let selections = editor.selections.all_adjusted_display(&display_map); - let mut anchors = Vec::new(); + let mut updated_cursor_ranges = Vec::new(); for selection in &selections { let start = selection.start.to_offset(&display_map, Bias::Left); - if let Some(range) = - object.range(&display_map, selection.clone(), true, None) - { - // If the current parenthesis object is single-line, - // then we need to filter whether it is the current line or not - if object.is_multiline() - || (!object.is_multiline() - && selection.start.row() == range.start.row() - && selection.end.row() == range.end.row()) - { - valid = true; - let chars_and_offset = display_map - .buffer_chars_at( - range.start.to_offset(&display_map, Bias::Left), - ) - .peekable(); - for (ch, offset) in chars_and_offset { - if ch.to_string() == pair.start { - anchors.push(offset..offset); - break; - } - } - } else { - anchors.push(start..start) - } + let in_range = object + .range(&display_map, selection.clone(), true, None) + .filter(|range| { + object.is_multiline() + || (selection.start.row() == range.start.row() + && selection.end.row() == range.end.row()) + }); + let Some(range) = in_range else { + updated_cursor_ranges.push(start..start); + matched_pair_anchors.push(None); + continue; + }; + + let range_start = range.start.to_offset(&display_map, Bias::Left); + let range_end = range.end.to_offset(&display_map, Bias::Left); + let open_offset = display_map + .buffer_chars_at(range_start) + .find(|(ch, _)| ch.to_string() == pair.start) + .map(|(_, offset)| offset); + let close_offset = display_map + .reverse_buffer_chars_at(range_end) + .find(|(ch, _)| ch.to_string() == pair.end) + .map(|(_, offset)| offset); + + if let (Some(open), Some(close)) = (open_offset, close_offset) { + let snapshot = &display_map.buffer_snapshot(); + updated_cursor_ranges.push(open..open); + matched_pair_anchors.push(Some(( + snapshot.anchor_before(open), + snapshot.anchor_before(close), + ))); } else { - anchors.push(start..start) + updated_cursor_ranges.push(start..start); + matched_pair_anchors.push(None); } } editor.change_selections(Default::default(), window, cx, |s| { - s.select_ranges(anchors); + s.select_ranges(updated_cursor_ranges); }); editor.set_clip_at_line_ends(true, cx); + + if !matched_pair_anchors.iter().any(|a| a.is_some()) { + matched_pair_anchors.clear(); + } }); }); } - valid + matched_pair_anchors } fn object_to_bracket_pair( @@ -1285,6 +1274,14 @@ mod test { cx.set_state(indoc! {"'ˇfoobar'"}, Mode::Normal); cx.simulate_keystrokes("c s ' }"); cx.assert_state(indoc! {"ˇ{foobar}"}, Mode::Normal); + + cx.set_state(indoc! {"I'm 'goˇod'"}, Mode::Normal); + cx.simulate_keystrokes("c s ' \""); + cx.assert_state(indoc! {"I'm ˇ\"good\""}, Mode::Normal); + + cx.set_state(indoc! {"I'm 'goˇod'"}, Mode::Normal); + cx.simulate_keystrokes("c s ' {"); + cx.assert_state(indoc! {"I'm ˇ{ good }"}, Mode::Normal); } #[gpui::test] diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index fea9735f27b36bcc0b56e7ba8155e9f51a767813..ce0a83258910294432b501d3847a88e19f334b24 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -770,6 +770,7 @@ impl Vim { Operator::ChangeSurrounds { target: action.target, opening: false, + bracket_anchors: Vec::new(), }, window, cx, @@ -1980,10 +1981,14 @@ impl Vim { } _ => self.clear_operator(window, cx), }, - Some(Operator::ChangeSurrounds { target, opening }) => match self.mode { + Some(Operator::ChangeSurrounds { + target, + opening, + bracket_anchors, + }) => match self.mode { Mode::Normal => { if let Some(target) = target { - self.change_surrounds(text, target, opening, window, cx); + self.change_surrounds(text, target, opening, bracket_anchors, window, cx); self.clear_operator(window, cx); } }