From 8b50aaabbe1820bc24046af73d2883c9dc74edca Mon Sep 17 00:00:00 2001 From: dinocosta Date: Wed, 19 Feb 2025 10:39:24 +0000 Subject: [PATCH 1/4] fix(vim): maintain visual selection when jumping to mark Fix the way selections are handled when jumping to marks in Vim mode so as to not reset these when vim is in visual mode. --- crates/vim/src/normal/mark.rs | 68 ++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/crates/vim/src/normal/mark.rs b/crates/vim/src/normal/mark.rs index 4cc717133019c0..552e73b80313bc 100644 --- a/crates/vim/src/normal/mark.rs +++ b/crates/vim/src/normal/mark.rs @@ -120,7 +120,7 @@ impl Vim { ) } } else { - self.update_editor(window, cx, |_, editor, window, cx| { + self.update_editor(window, cx, |vim, editor, window, cx| { let map = editor.snapshot(window, cx); let mut ranges: Vec> = Vec::new(); for mut anchor in anchors { @@ -131,14 +131,74 @@ impl Vim { .display_snapshot .buffer_snapshot .anchor_before(point.to_point(&map.display_snapshot)); + + if vim.mode == Mode::Visual + || vim.mode == Mode::VisualLine + || vim.mode == Mode::VisualBlock + { + editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| { + s.move_with(|map, selection| { + let was_reversed = selection.reversed; + let mut current_head = selection.head(); + + // our motions assume the current character is after the cursor, + // but in (forward) visual mode the current character is just + // before the end of the selection. + + // If the file ends with a newline (which is common) we don't do this. + // so that if you go to the end of such a file you can use "up" to go + // to the previous line and have it work somewhat as expected. + #[allow(clippy::nonminimal_bool)] + if !selection.reversed + && !selection.is_empty() + && !(selection.end.column() == 0 + && selection.end == map.max_point()) + { + current_head = movement::left(map, selection.end) + } + + selection.set_head(point, SelectionGoal::None); + + // ensure the current character is included in the selection. + if !selection.reversed { + let next_point = if vim.mode == Mode::VisualBlock { + movement::saturating_right(map, selection.end) + } else { + movement::right(map, selection.end) + }; + + if !(next_point.column() == 0 + && next_point == map.max_point()) + { + selection.end = next_point; + } + } + + // vim always ensures the anchor character stays selected. + // if our selection has reversed, we need to move the opposite end + // to ensure the anchor is still selected. + if was_reversed && !selection.reversed { + selection.start = movement::left(map, selection.start); + } else if !was_reversed && selection.reversed { + selection.end = movement::right(map, selection.end); + } + }) + }); + } } if ranges.last() != Some(&(anchor..anchor)) { ranges.push(anchor..anchor); } } - editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| { - s.select_anchor_ranges(ranges) - }) + + if vim.mode != Mode::Visual + && vim.mode != Mode::VisualLine + && vim.mode != Mode::VisualBlock + { + editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| { + s.select_anchor_ranges(ranges) + }) + } }); } } From 1d7761c2eba30be555336bd76b21719c7d66007d Mon Sep 17 00:00:00 2001 From: dinocosta Date: Wed, 19 Feb 2025 18:53:30 +0000 Subject: [PATCH 2/4] refactor: use vim.motion to jump to a given mark's anchor --- crates/vim/src/normal/mark.rs | 66 ++++++----------------------------- 1 file changed, 10 insertions(+), 56 deletions(-) diff --git a/crates/vim/src/normal/mark.rs b/crates/vim/src/normal/mark.rs index 552e73b80313bc..6fdd789cfe7ee4 100644 --- a/crates/vim/src/normal/mark.rs +++ b/crates/vim/src/normal/mark.rs @@ -105,7 +105,7 @@ impl Vim { _ => self.marks.get(&*text).cloned(), }; - let Some(anchors) = anchors else { return }; + let Some(mut anchors) = anchors else { return }; let is_active_operator = self.active_operator().is_some(); if is_active_operator { @@ -120,6 +120,9 @@ impl Vim { ) } } else { + // Save the last anchor so as to jump to it later. + let anchor: Option = anchors.last_mut().map(|last| last.clone()); + self.update_editor(window, cx, |vim, editor, window, cx| { let map = editor.snapshot(window, cx); let mut ranges: Vec> = Vec::new(); @@ -131,61 +134,8 @@ impl Vim { .display_snapshot .buffer_snapshot .anchor_before(point.to_point(&map.display_snapshot)); - - if vim.mode == Mode::Visual - || vim.mode == Mode::VisualLine - || vim.mode == Mode::VisualBlock - { - editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| { - s.move_with(|map, selection| { - let was_reversed = selection.reversed; - let mut current_head = selection.head(); - - // our motions assume the current character is after the cursor, - // but in (forward) visual mode the current character is just - // before the end of the selection. - - // If the file ends with a newline (which is common) we don't do this. - // so that if you go to the end of such a file you can use "up" to go - // to the previous line and have it work somewhat as expected. - #[allow(clippy::nonminimal_bool)] - if !selection.reversed - && !selection.is_empty() - && !(selection.end.column() == 0 - && selection.end == map.max_point()) - { - current_head = movement::left(map, selection.end) - } - - selection.set_head(point, SelectionGoal::None); - - // ensure the current character is included in the selection. - if !selection.reversed { - let next_point = if vim.mode == Mode::VisualBlock { - movement::saturating_right(map, selection.end) - } else { - movement::right(map, selection.end) - }; - - if !(next_point.column() == 0 - && next_point == map.max_point()) - { - selection.end = next_point; - } - } - - // vim always ensures the anchor character stays selected. - // if our selection has reversed, we need to move the opposite end - // to ensure the anchor is still selected. - if was_reversed && !selection.reversed { - selection.start = movement::left(map, selection.start); - } else if !was_reversed && selection.reversed { - selection.end = movement::right(map, selection.end); - } - }) - }); - } } + if ranges.last() != Some(&(anchor..anchor)) { ranges.push(anchor..anchor); } @@ -197,9 +147,13 @@ impl Vim { { editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| { s.select_anchor_ranges(ranges) - }) + }); } }); + + if let Some(anchor) = anchor { + self.motion(Motion::Jump { anchor, line }, window, cx) + } } } } From 660f43c501219eeb6f64c8d9fefab2ea20a1052f Mon Sep 17 00:00:00 2001 From: dinocosta Date: Thu, 20 Feb 2025 10:18:10 +0000 Subject: [PATCH 3/4] fix(vim): only use jump motion if in visual mode Fix regression in mark's behaviour, where a mark with multiple anchors was not activating all anchors again when jumping to that mark in normal mode, as `Motion::Jump` was being used even if not in visual mode. This commit fixes the issue by only using jump motion if in visual mode, making sure that the behaviour stays the same as before. --- crates/vim/src/normal/mark.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/crates/vim/src/normal/mark.rs b/crates/vim/src/normal/mark.rs index 6fdd789cfe7ee4..7040d55afd116c 100644 --- a/crates/vim/src/normal/mark.rs +++ b/crates/vim/src/normal/mark.rs @@ -122,8 +122,11 @@ impl Vim { } else { // Save the last anchor so as to jump to it later. let anchor: Option = anchors.last_mut().map(|last| last.clone()); + let should_jump = self.mode == Mode::Visual + || self.mode == Mode::VisualLine + || self.mode == Mode::VisualBlock; - self.update_editor(window, cx, |vim, editor, window, cx| { + self.update_editor(window, cx, |_, editor, window, cx| { let map = editor.snapshot(window, cx); let mut ranges: Vec> = Vec::new(); for mut anchor in anchors { @@ -141,18 +144,22 @@ impl Vim { } } - if vim.mode != Mode::Visual - && vim.mode != Mode::VisualLine - && vim.mode != Mode::VisualBlock - { + if !should_jump { editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| { s.select_anchor_ranges(ranges) }); } }); - if let Some(anchor) = anchor { - self.motion(Motion::Jump { anchor, line }, window, cx) + if should_jump && anchor.is_some() { + self.motion( + Motion::Jump { + anchor: anchor.unwrap(), + line, + }, + window, + cx, + ) } } } From 4dd4dd12161991efef3610ab541844a4e4d41f3a Mon Sep 17 00:00:00 2001 From: dinocosta Date: Fri, 21 Feb 2025 22:00:32 +0000 Subject: [PATCH 4/4] fix(clippy): tackle clippy warnings --- crates/vim/src/normal/mark.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/crates/vim/src/normal/mark.rs b/crates/vim/src/normal/mark.rs index 7040d55afd116c..72194b728c330c 100644 --- a/crates/vim/src/normal/mark.rs +++ b/crates/vim/src/normal/mark.rs @@ -121,7 +121,7 @@ impl Vim { } } else { // Save the last anchor so as to jump to it later. - let anchor: Option = anchors.last_mut().map(|last| last.clone()); + let anchor: Option = anchors.last_mut().map(|anchor| *anchor); let should_jump = self.mode == Mode::Visual || self.mode == Mode::VisualLine || self.mode == Mode::VisualBlock; @@ -151,15 +151,10 @@ impl Vim { } }); - if should_jump && anchor.is_some() { - self.motion( - Motion::Jump { - anchor: anchor.unwrap(), - line, - }, - window, - cx, - ) + if should_jump { + if let Some(anchor) = anchor { + self.motion(Motion::Jump { anchor, line }, window, cx) + } } } }