From eaec04632a1b3ee973a80251353018059f624e96 Mon Sep 17 00:00:00 2001 From: Hans Date: Thu, 28 Mar 2024 17:16:54 +0800 Subject: [PATCH] vim: Fix `t` operand not working correctly when cursor is on tag (#9899) Fix #8994 and #9844 Release notes: * Fixed the `t` object in Vim mode not working correctly when cursor was on a tag. #9844 and #8994 This mr fixes the above two problems, for #9844, because our previous logic is to only think that the minimum html tag containing the current cursor is qualified, but the approach of nvim is to get the tag after the current cursor first, followed by the tag around the current cursor, so I modified the corresponding condition For #8994, the situation is a bit more complicated, in our previous implementation, we could only get the range of the object by a `cursor position`, but there are two possible cases for the html tag: When the current cursor length is 1, nvim will return the first tag after the current cursor, as described above When the current cursor length is greater than 1, nvim will return just the smallest tag that can cover the current selection So we may need to pass the current selection to the inside of the method, and the point alone is not enough to support us in calculating these conditions --- crates/vim/src/object.rs | 87 +++++++++++++++++++++++++++++++++++----- crates/vim/src/visual.rs | 13 ++++-- 2 files changed, 86 insertions(+), 14 deletions(-) diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index a525cda102dc5818deeecaa7c5b0b8355cbf4969..6eb2506c9d0e5512cee495757edae5ca48e3c78f 100644 --- a/crates/vim/src/object.rs +++ b/crates/vim/src/object.rs @@ -165,9 +165,10 @@ impl Object { pub fn range( self, map: &DisplaySnapshot, - relative_to: DisplayPoint, + selection: Selection, around: bool, ) -> Option> { + let relative_to = selection.head(); match self { Object::Word { ignore_punctuation } => { if around { @@ -193,7 +194,7 @@ impl Object { Object::Parentheses => { surrounding_markers(map, relative_to, around, self.is_multiline(), '(', ')') } - Object::Tag => surrounding_html_tag(map, relative_to, around), + Object::Tag => surrounding_html_tag(map, selection, around), Object::SquareBrackets => { surrounding_markers(map, relative_to, around, self.is_multiline(), '[', ']') } @@ -213,7 +214,7 @@ impl Object { selection: &mut Selection, around: bool, ) -> bool { - if let Some(range) = self.range(map, selection.head(), around) { + if let Some(range) = self.range(map, selection.clone(), around) { selection.start = range.start; selection.end = range.end; true @@ -256,8 +257,8 @@ fn in_word( fn surrounding_html_tag( map: &DisplaySnapshot, - relative_to: DisplayPoint, - surround: bool, + selection: Selection, + around: bool, ) -> Option> { fn read_tag(chars: impl Iterator) -> String { chars @@ -278,7 +279,7 @@ fn surrounding_html_tag( } let snapshot = &map.buffer_snapshot; - let offset = relative_to.to_offset(map, Bias::Left); + let offset = selection.head().to_offset(map, Bias::Left); let excerpt = snapshot.excerpt_containing(offset..offset)?; let buffer = excerpt.buffer(); let offset = excerpt.map_offset_to_buffer(offset); @@ -298,11 +299,18 @@ fn surrounding_html_tag( if let (Some(first_child), Some(last_child)) = (first_child, last_child) { let open_tag = open_tag(buffer.chars_for_range(first_child.byte_range())); let close_tag = close_tag(buffer.chars_for_range(last_child.byte_range())); - if open_tag.is_some() - && open_tag == close_tag - && (first_child.end_byte() + 1..last_child.start_byte()).contains(&offset) + // It needs to be handled differently according to the selection length + let is_valid = if selection.end.to_offset(map, Bias::Left) + - selection.start.to_offset(map, Bias::Left) + <= 1 { - let range = if surround { + offset <= last_child.end_byte() + } else { + selection.start.to_offset(map, Bias::Left) >= first_child.start_byte() + && selection.end.to_offset(map, Bias::Left) <= last_child.start_byte() + 1 + }; + if open_tag.is_some() && open_tag == close_tag && is_valid { + let range = if around { first_child.byte_range().start..last_child.byte_range().end } else { first_child.byte_range().end..last_child.byte_range().start @@ -320,6 +328,7 @@ fn surrounding_html_tag( } None } + /// Returns a range that surrounds the word and following whitespace /// relative_to is in. /// @@ -1601,5 +1610,63 @@ mod test { "«hi!ˇ»", Mode::Visual, ); + + // The cursor is before the tag + cx.set_state( + " ˇ hi!", + Mode::Normal, + ); + cx.simulate_keystrokes(["v", "i", "t"]); + cx.assert_state( + " «hi!ˇ»", + Mode::Visual, + ); + cx.simulate_keystrokes(["a", "t"]); + cx.assert_state( + " «hi!ˇ»", + Mode::Visual, + ); + + // The cursor is in the open tag + cx.set_state( + "hi!hello!", + Mode::Normal, + ); + cx.simulate_keystrokes(["v", "a", "t"]); + cx.assert_state( + "«hi!ˇ»hello!", + Mode::Visual, + ); + cx.simulate_keystrokes(["i", "t"]); + cx.assert_state( + "«hi!hello!ˇ»", + Mode::Visual, + ); + + // current selection length greater than 1 + cx.set_state( + "<«b>hi!ˇ»", + Mode::Visual, + ); + cx.simulate_keystrokes(["i", "t"]); + cx.assert_state( + "«hi!ˇ»", + Mode::Visual, + ); + cx.simulate_keystrokes(["a", "t"]); + cx.assert_state( + "«hi!ˇ»", + Mode::Visual, + ); + + cx.set_state( + "<«b>hi!", + Mode::Visual, + ); + cx.simulate_keystrokes(["a", "t"]); + cx.assert_state( + "«hi!ˇ»", + Mode::Visual, + ); } } diff --git a/crates/vim/src/visual.rs b/crates/vim/src/visual.rs index c316340058addadb10f699d06cb559fecc657e23..b20b06bcb2dffcbb0c20e719e8d81f15456f0525 100644 --- a/crates/vim/src/visual.rs +++ b/crates/vim/src/visual.rs @@ -255,16 +255,21 @@ pub fn visual_object(object: Object, cx: &mut WindowContext) { vim.update_active_editor(cx, |_, editor, cx| { editor.change_selections(Some(Autoscroll::fit()), cx, |s| { s.move_with(|map, selection| { - let mut head = selection.head(); + let mut mut_selection = selection.clone(); // all our motions assume that the current character is // after the cursor; however in the case of a visual selection // the current character is before the cursor. - if !selection.reversed { - head = movement::left(map, head); + // But this will affect the judgment of the html tag + // so the html tag needs to skip this logic. + if !selection.reversed && object != Object::Tag { + mut_selection.set_head( + movement::left(map, mut_selection.head()), + mut_selection.goal, + ); } - if let Some(range) = object.range(map, head, around) { + if let Some(range) = object.range(map, mut_selection, around) { if !range.is_empty() { let expand_both_ways = object.always_expands_both_ways() || selection.is_empty()