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
This commit is contained in:
Hans 2024-03-28 17:16:54 +08:00 committed by GitHub
parent 96a1af7b0f
commit eaec04632a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 86 additions and 14 deletions

View File

@ -165,9 +165,10 @@ impl Object {
pub fn range( pub fn range(
self, self,
map: &DisplaySnapshot, map: &DisplaySnapshot,
relative_to: DisplayPoint, selection: Selection<DisplayPoint>,
around: bool, around: bool,
) -> Option<Range<DisplayPoint>> { ) -> Option<Range<DisplayPoint>> {
let relative_to = selection.head();
match self { match self {
Object::Word { ignore_punctuation } => { Object::Word { ignore_punctuation } => {
if around { if around {
@ -193,7 +194,7 @@ impl Object {
Object::Parentheses => { Object::Parentheses => {
surrounding_markers(map, relative_to, around, self.is_multiline(), '(', ')') 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 => { Object::SquareBrackets => {
surrounding_markers(map, relative_to, around, self.is_multiline(), '[', ']') surrounding_markers(map, relative_to, around, self.is_multiline(), '[', ']')
} }
@ -213,7 +214,7 @@ impl Object {
selection: &mut Selection<DisplayPoint>, selection: &mut Selection<DisplayPoint>,
around: bool, around: bool,
) -> 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.start = range.start;
selection.end = range.end; selection.end = range.end;
true true
@ -256,8 +257,8 @@ fn in_word(
fn surrounding_html_tag( fn surrounding_html_tag(
map: &DisplaySnapshot, map: &DisplaySnapshot,
relative_to: DisplayPoint, selection: Selection<DisplayPoint>,
surround: bool, around: bool,
) -> Option<Range<DisplayPoint>> { ) -> Option<Range<DisplayPoint>> {
fn read_tag(chars: impl Iterator<Item = char>) -> String { fn read_tag(chars: impl Iterator<Item = char>) -> String {
chars chars
@ -278,7 +279,7 @@ fn surrounding_html_tag(
} }
let snapshot = &map.buffer_snapshot; 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 excerpt = snapshot.excerpt_containing(offset..offset)?;
let buffer = excerpt.buffer(); let buffer = excerpt.buffer();
let offset = excerpt.map_offset_to_buffer(offset); 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) { 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 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())); let close_tag = close_tag(buffer.chars_for_range(last_child.byte_range()));
if open_tag.is_some() // It needs to be handled differently according to the selection length
&& open_tag == close_tag let is_valid = if selection.end.to_offset(map, Bias::Left)
&& (first_child.end_byte() + 1..last_child.start_byte()).contains(&offset) - 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 first_child.byte_range().start..last_child.byte_range().end
} else { } else {
first_child.byte_range().end..last_child.byte_range().start first_child.byte_range().end..last_child.byte_range().start
@ -320,6 +328,7 @@ fn surrounding_html_tag(
} }
None None
} }
/// Returns a range that surrounds the word and following whitespace /// Returns a range that surrounds the word and following whitespace
/// relative_to is in. /// relative_to is in.
/// ///
@ -1601,5 +1610,63 @@ mod test {
"<html><head></head>«<body><b>hi!</b></body>ˇ»", "<html><head></head>«<body><b>hi!</b></body>ˇ»",
Mode::Visual, Mode::Visual,
); );
// The cursor is before the tag
cx.set_state(
"<html><head></head><body> ˇ <b>hi!</b></body>",
Mode::Normal,
);
cx.simulate_keystrokes(["v", "i", "t"]);
cx.assert_state(
"<html><head></head><body> <b>«hi!ˇ»</b></body>",
Mode::Visual,
);
cx.simulate_keystrokes(["a", "t"]);
cx.assert_state(
"<html><head></head><body> «<b>hi!</b>ˇ»</body>",
Mode::Visual,
);
// The cursor is in the open tag
cx.set_state(
"<html><head></head><body><bˇ>hi!</b><b>hello!</b></body>",
Mode::Normal,
);
cx.simulate_keystrokes(["v", "a", "t"]);
cx.assert_state(
"<html><head></head><body>«<b>hi!</b>ˇ»<b>hello!</b></body>",
Mode::Visual,
);
cx.simulate_keystrokes(["i", "t"]);
cx.assert_state(
"<html><head></head><body>«<b>hi!</b><b>hello!</b>ˇ»</body>",
Mode::Visual,
);
// current selection length greater than 1
cx.set_state(
"<html><head></head><body><«b>hi!ˇ»</b></body>",
Mode::Visual,
);
cx.simulate_keystrokes(["i", "t"]);
cx.assert_state(
"<html><head></head><body><b>«hi!ˇ»</b></body>",
Mode::Visual,
);
cx.simulate_keystrokes(["a", "t"]);
cx.assert_state(
"<html><head></head><body>«<b>hi!</b>ˇ»</body>",
Mode::Visual,
);
cx.set_state(
"<html><head></head><body><«b>hi!</ˇ»b></body>",
Mode::Visual,
);
cx.simulate_keystrokes(["a", "t"]);
cx.assert_state(
"<html><head></head>«<body><b>hi!</b></body>ˇ»",
Mode::Visual,
);
} }
} }

View File

@ -255,16 +255,21 @@ pub fn visual_object(object: Object, cx: &mut WindowContext) {
vim.update_active_editor(cx, |_, editor, cx| { vim.update_active_editor(cx, |_, editor, cx| {
editor.change_selections(Some(Autoscroll::fit()), cx, |s| { editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
s.move_with(|map, selection| { 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 // all our motions assume that the current character is
// after the cursor; however in the case of a visual selection // after the cursor; however in the case of a visual selection
// the current character is before the cursor. // the current character is before the cursor.
if !selection.reversed { // But this will affect the judgment of the html tag
head = movement::left(map, head); // 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() { if !range.is_empty() {
let expand_both_ways = object.always_expands_both_ways() let expand_both_ways = object.always_expands_both_ways()
|| selection.is_empty() || selection.is_empty()