From 52f73e0c2dc0093a213fdf9a235517d757ce98d4 Mon Sep 17 00:00:00 2001 From: 5brian Date: Mon, 24 Feb 2025 20:30:21 -0500 Subject: [PATCH] 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 | 8 +- crates/vim/src/object.rs | 134 +++++++----------- crates/vim/src/visual.rs | 5 +- ...ltiline_surrounding_character_objects.json | 20 --- 4 files changed, 57 insertions(+), 110 deletions(-) delete mode 100644 crates/vim/test_data/test_multiline_surrounding_character_objects.json diff --git a/crates/vim/src/normal/yank.rs b/crates/vim/src/normal/yank.rs index a6b827c8cf2bb5019422f9f872e846cbccbfdacb..09551a35cd95407c86d11e82d7e183d81cb9927d 100644 --- a/crates/vim/src/normal/yank.rs +++ b/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); }); }); diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index 2120df91afad18a715878302e5d0bc502799d3d2..689763f8b426286a360cd151f90e70d782d4cc14 100644 --- a/crates/vim/src/object.rs +++ b/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) { - 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 { - // Handle trailing newline after opening - if let Some((ch, range)) = movement::chars_after(map, opening.end).next() { - if ch == '\n' { - opening.end = range.end; - - // After newline, skip leading whitespace - let mut chars = movement::chars_after(map, opening.end).peekable(); - while let Some((ch, range)) = chars.peek() { - if !ch.is_whitespace() { - break; - } - opening.end = range.end; - chars.next(); + // Adjust selection to remove leading and trailing whitespace for multiline inner brackets + if !around && open_marker != close_marker { + let start_point = opening.end.to_display_point(map); + let end_point = closing.start.to_display_point(map); + let start_offset = start_point.to_offset(map, Bias::Left); + let end_offset = end_point.to_offset(map, Bias::Left); + + if start_point.row() != end_point.row() + && map + .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; } } - } - - // 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; + for (ch, offset) in map.reverse_buffer_chars_at(end_offset) { + if !ch.is_whitespace() { + last_non_ws = Some(offset + ch.len_utf8()); + break; + } } - if ch == '\n' { - last_newline_end = Some(range.end); - break; + if let Some(start) = first_non_ws { + opening.end = start; } - } - // Adjust closing.start to exclude whitespace after a newline, if present - if let Some(end) = last_newline_end { - 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; diff --git a/crates/vim/src/visual.rs b/crates/vim/src/visual.rs index 1a4cda5e2241b73747c903aae18e91d4aa0cbaec..a9e4dd9767fc48ec706d132f782f4ea82b12675a 100644 --- a/crates/vim/src/visual.rs +++ b/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 diff --git a/crates/vim/test_data/test_multiline_surrounding_character_objects.json b/crates/vim/test_data/test_multiline_surrounding_character_objects.json deleted file mode 100644 index c61b7b9145b94ef2767f9de2241e1be1f2232408..0000000000000000000000000000000000000000 --- a/crates/vim/test_data/test_multiline_surrounding_character_objects.json +++ /dev/null @@ -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"}}