vim: Refactor and fix multiline operations (#25055)

Changes:
- [x] Cursor at the start during yank operations on objects (`yip`,
`yab` etc).
- [x] Refactors this: Trim all leading and trailing whitespace from
inner multiline bracket selection.
  - This leaves a nicely indented line when doing `ci{` `vi{d` etc
  - [x] Checks for empty selection
  - [x] Removed moving cursor to the start in visual bracket operations

This cleans up the previous implementation by providing a simpler check
in `surrounding_markers`, instead of calling a new function in
`expand_object`. No functionality was changed there except for handling
the empty selection and removing some cursor adjustments that should not
have been there after further testing.

Release Notes:

- N/A

crates/vim/src/normal/yank.rs

@@ -58,18 +58,18 @@ impl Vim {
         self.update_editor(window, cx, |vim, editor, window, cx| {
             editor.transact(window, cx, |editor, window, cx| {
                 editor.set_clip_at_line_ends(false, cx);
-                let mut original_positions: HashMap<_, _> = Default::default();
+                let mut start_positions: HashMap<_, _> = Default::default();
                 editor.change_selections(None, window, cx, |s| {
                     s.move_with(|map, selection| {
-                        let original_position = (selection.head(), selection.goal);
                         object.expand_selection(map, selection, around);
-                        original_positions.insert(selection.id, original_position);
+                        let start_position = (selection.start, selection.goal);
+                        start_positions.insert(selection.id, start_position);
                     });
                 });
                 vim.yank_selections_content(editor, false, cx);
                 editor.change_selections(None, window, cx, |s| {
                     s.move_with(|_, selection| {
-                        let (head, goal) = original_positions.remove(&selection.id).unwrap();
+                        let (head, goal) = start_positions.remove(&selection.id).unwrap();
                         selection.collapse_to(head, goal);
                     });
                 });

crates/vim/src/object.rs

@@ -560,9 +560,6 @@ impl Object {
         if let Some(range) = self.range(map, selection.clone(), around) {
             selection.start = range.start;
             selection.end = range.end;
-            if !around && self.is_multiline() {
-                preserve_indented_newline(map, selection);
-            }
             true
         } else {
             false
@@ -570,50 +567,6 @@ impl Object {
     }
 }
 
-/// Returns a range without the final newline char.
-///
-/// If the selection spans multiple lines and is preceded by an opening brace (`{`),
-/// this function will trim the selection to exclude the final newline
-/// in order to preserve a properly indented line.
-pub fn preserve_indented_newline(map: &DisplaySnapshot, selection: &mut Selection<DisplayPoint>) {
-    let (start_point, end_point) = (selection.start.to_point(map), selection.end.to_point(map));
-
-    if start_point.row == end_point.row {
-        return;
-    }
-
-    let start_offset = selection.start.to_offset(map, Bias::Left);
-    let mut pos = start_offset;
-
-    while pos > 0 {
-        pos -= 1;
-        let current_char = map.buffer_chars_at(pos).next().map(|(ch, _)| ch);
-
-        match current_char {
-            Some(ch) if !ch.is_whitespace() => break,
-            Some('\n') if pos > 0 => {
-                let prev_char = map.buffer_chars_at(pos - 1).next().map(|(ch, _)| ch);
-                if prev_char == Some('{') {
-                    let end_pos = selection.end.to_offset(map, Bias::Left);
-                    for (ch, offset) in map.reverse_buffer_chars_at(end_pos) {
-                        match ch {
-                            '\n' => {
-                                selection.end = offset.to_display_point(map);
-                                selection.reversed = true;
-                                break;
-                            }
-                            ch if !ch.is_whitespace() => break,
-                            _ => continue,
-                        }
-                    }
-                }
-                break;
-            }
-            _ => continue,
-        }
-    }
-}
-
 /// Returns a range that surrounds the word `relative_to` is in.
 ///
 /// If `relative_to` is at the start of a word, return the word.
@@ -1518,38 +1471,37 @@ fn surrounding_markers(
         }
     }
 
-    if !around && search_across_lines {
+    // Adjust selection to remove leading and trailing whitespace for multiline inner brackets
-        // Handle trailing newline after opening
+    if !around && open_marker != close_marker {
-        if let Some((ch, range)) = movement::chars_after(map, opening.end).next() {
+        let start_point = opening.end.to_display_point(map);
-            if ch == '\n' {
+        let end_point = closing.start.to_display_point(map);
-                opening.end = range.end;
+        let start_offset = start_point.to_offset(map, Bias::Left);
+        let end_offset = end_point.to_offset(map, Bias::Left);
 
-                // After newline, skip leading whitespace
+        if start_point.row() != end_point.row()
-                let mut chars = movement::chars_after(map, opening.end).peekable();
+            && map
-                while let Some((ch, range)) = chars.peek() {
+                .buffer_chars_at(start_offset)
+                .take_while(|(_, offset)| offset < &end_offset)
+                .any(|(ch, _)| !ch.is_whitespace())
+        {
+            let mut first_non_ws = None;
+            let mut last_non_ws = None;
+            for (ch, offset) in map.buffer_chars_at(start_offset) {
                 if !ch.is_whitespace() {
+                    first_non_ws = Some(offset);
                     break;
                 }
-                    opening.end = range.end;
-                    chars.next();
             }
-            }
+            for (ch, offset) in map.reverse_buffer_chars_at(end_offset) {
-        }
-
-        // Handle leading whitespace before closing
-        let mut last_newline_end = None;
-        for (ch, range) in movement::chars_before(map, closing.start) {
                 if !ch.is_whitespace() {
-                break;
+                    last_non_ws = Some(offset + ch.len_utf8());
-            }
-            if ch == '\n' {
-                last_newline_end = Some(range.end);
                     break;
                 }
             }
-        // Adjust closing.start to exclude whitespace after a newline, if present
+            if let Some(start) = first_non_ws {
-        if let Some(end) = last_newline_end {
+                opening.end = start;
-            if end > opening.end {
+            }
+            if let Some(end) = last_non_ws {
                 closing.start = end;
             }
         }
@@ -1904,10 +1856,10 @@ mod test {
         cx.assert_state(
             indoc! {
                 "func empty(a string) bool {
-                   «ˇif a == \"\" {
+                   «if a == \"\" {
                       return true
                    }
-                   return false»
+                   return falseˇ»
                 }"
             },
             Mode::Visual,
@@ -1929,7 +1881,7 @@ mod test {
             indoc! {
                 "func empty(a string) bool {
                      if a == \"\" {
-                         «ˇreturn true»
+                         «return trueˇ»
                      }
                      return false
                 }"
@@ -1953,7 +1905,7 @@ mod test {
             indoc! {
                 "func empty(a string) bool {
                      if a == \"\" {
-                         «ˇreturn true»
+                         «return trueˇ»
                      }
                      return false
                 }"
@@ -1976,14 +1928,33 @@ mod test {
         cx.assert_state(
             indoc! {
                 "func empty(a string) bool {
-                     «ˇif a == \"\" {
+                     «if a == \"\" {
                          return true
                      }
-                     return false»
+                     return falseˇ»
                 }"
             },
             Mode::Visual,
         );
+
+        cx.set_state(
+            indoc! {
+                "func empty(a string) bool {
+                             if a == \"\" {
+                             ˇ
+
+                             }"
+            },
+            Mode::Normal,
+        );
+        cx.simulate_keystrokes("c i {");
+        cx.assert_state(
+            indoc! {
+                "func empty(a string) bool {
+                             if a == \"\" {ˇ}"
+            },
+            Mode::Insert,
+        );
     }
 
     #[gpui::test]
@@ -2603,7 +2574,6 @@ mod test {
     #[gpui::test]
     async fn test_anybrackets_trailing_space(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;
-
         cx.set_shared_state("(trailingˇ whitespace          )")
             .await;
         cx.simulate_shared_keystrokes("v i b").await;

crates/vim/src/visual.rs

@@ -16,7 +16,7 @@ use workspace::searchable::Direction;
 
 use crate::{
     motion::{first_non_whitespace, next_line_end, start_of_line, Motion},
-    object::{self, Object},
+    object::Object,
     state::{Mode, Operator},
     Vim,
 };
@@ -375,9 +375,6 @@ impl Vim {
                                 } else {
                                     selection.end = range.end;
                                 }
-                                if !around && object.is_multiline() {
-                                    object::preserve_indented_newline(map, selection);
-                                }
                             }
 
                             // In the visual selection result of a paragraph object, the cursor is

crates/vim/test_data/test_multiline_surrounding_character_objects.json

@@ -1,20 +0,0 @@
-{"Put":{"state":"func empty(a string) bool {\n     if a == \"\" {\n      return true\n   }\n   ˇreturn false\n}"}}
-{"Key":"v"}
-{"Key":"i"}
-{"Key":"{"}
-{"Get":{"state":"func empty(a string) bool {\n     «ˇif a == \"\" {\n      return true\n   }\n   return false»\n}","mode":"Visual"}}
-{"Put":{"state":"func empty(a string) bool {\n     if a == \"\" {\n         ˇreturn true\n     }\n     return false\n}"}}
-{"Key":"v"}
-{"Key":"i"}
-{"Key":"{"}
-{"Get":{"state":"func empty(a string) bool {\n     if a == \"\" {\n         «ˇreturn true»\n     }\n     return false\n}","mode":"Visual"}}
-{"Put":{"state":"func empty(a string) bool {\n     if a == \"\" ˇ{\n         return true\n     }\n     return false\n}"}}
-{"Key":"v"}
-{"Key":"i"}
-{"Key":"{"}
-{"Get":{"state":"func empty(a string) bool {\n     if a == \"\" {\n         «ˇreturn true»\n     }\n     return false\n}","mode":"Visual"}}
-{"Put":{"state":"func empty(a string) bool {\n     if a == \"\" {\n         return true\n     }\n     return false\nˇ}"}}
-{"Key":"v"}
-{"Key":"i"}
-{"Key":"{"}
-{"Get":{"state":"func empty(a string) bool {\n     «ˇif a == \"\" {\n         return true\n     }\n     return false»\n}","mode":"Visual"}}