Add support for visual ranges ending with a newline

Conrad Irwin created

These primarily happen when first entering visual mode, but can also
be created with objects like `vi{`.

Along the way fix the way ranges like `vi{` are selected to be more
similar to nvim.

Change summary

assets/keymaps/vim.json                                                |   4 
crates/editor/src/display_map.rs                                       |  55 
crates/editor/src/editor.rs                                            |   6 
crates/editor/src/element.rs                                           |  34 
crates/editor/src/movement.rs                                          |  12 
crates/editor/src/selections_collection.rs                             |   1 
crates/vim/src/mode_indicator.rs                                       |   2 
crates/vim/src/motion.rs                                               |  12 
crates/vim/src/normal.rs                                               |  13 
crates/vim/src/normal/change.rs                                        |  11 
crates/vim/src/normal/delete.rs                                        |  14 
crates/vim/src/normal/yank.rs                                          |   5 
crates/vim/src/object.rs                                               |  79 
crates/vim/src/state.rs                                                |   7 
crates/vim/src/test/neovim_connection.rs                               |  15 
crates/vim/src/vim.rs                                                  |  10 
crates/vim/src/visual.rs                                               | 108 
crates/vim/test_data/test_enter_visual_line_mode.json                  |  10 
crates/vim/test_data/test_enter_visual_mode.json                       |   8 
crates/vim/test_data/test_multiline_surrounding_character_objects.json |  10 
crates/vim/test_data/test_visual_word_object.json                      |  16 
21 files changed, 299 insertions(+), 133 deletions(-)

Detailed changes

assets/keymaps/vim.json 🔗

@@ -101,6 +101,8 @@
         "vim::SwitchMode",
         "Normal"
       ],
+      "v": "vim::ToggleVisual",
+      "shift-v": "vim::ToggleVisualLine",
       "*": "vim::MoveToNext",
       "#": "vim::MoveToPrev",
       "0": "vim::StartOfLine", // When no number operator present, use start of line motion
@@ -266,8 +268,6 @@
       "o": "vim::InsertLineBelow",
       "shift-o": "vim::InsertLineAbove",
       "~": "vim::ChangeCase",
-      "v": "vim::ToggleVisual",
-      "shift-v": "vim::ToggleVisualLine",
       "p": "vim::Paste",
       "u": "editor::Undo",
       "ctrl-r": "editor::Redo",

crates/editor/src/display_map.rs 🔗

@@ -35,12 +35,6 @@ pub enum FoldStatus {
     Foldable,
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
-pub enum Clip {
-    None,
-    EndOfLine,
-}
-
 pub trait ToDisplayPoint {
     fn to_display_point(&self, map: &DisplaySnapshot) -> DisplayPoint;
 }
@@ -56,7 +50,7 @@ pub struct DisplayMap {
     wrap_map: ModelHandle<WrapMap>,
     block_map: BlockMap,
     text_highlights: TextHighlights,
-    pub default_clip: Clip,
+    pub clip_at_line_ends: bool,
 }
 
 impl Entity for DisplayMap {
@@ -91,7 +85,7 @@ impl DisplayMap {
             wrap_map,
             block_map,
             text_highlights: Default::default(),
-            default_clip: Clip::None,
+            clip_at_line_ends: false,
         }
     }
 
@@ -115,7 +109,7 @@ impl DisplayMap {
             wrap_snapshot,
             block_snapshot,
             text_highlights: self.text_highlights.clone(),
-            default_clip: self.default_clip,
+            clip_at_line_ends: self.clip_at_line_ends,
         }
     }
 
@@ -302,7 +296,7 @@ pub struct DisplaySnapshot {
     wrap_snapshot: wrap_map::WrapSnapshot,
     block_snapshot: block_map::BlockSnapshot,
     text_highlights: TextHighlights,
-    default_clip: Clip,
+    clip_at_line_ends: bool,
 }
 
 impl DisplaySnapshot {
@@ -361,6 +355,9 @@ impl DisplaySnapshot {
 
     pub fn expand_to_line(&self, range: Range<Point>) -> Range<Point> {
         let mut new_start = self.prev_line_boundary(range.start).0;
+        if range.end.column == 0 {
+            return new_start..range.end;
+        }
         let mut new_end = self.next_line_boundary(range.end).0;
 
         if new_start.row == range.start.row && new_end.row == range.end.row {
@@ -583,33 +580,21 @@ impl DisplaySnapshot {
         column
     }
 
-    pub fn move_left(&self, point: DisplayPoint, clip: Clip) -> DisplayPoint {
-        self.clip_point_with(
-            DisplayPoint::new(point.row(), point.column().saturating_sub(1)),
-            Bias::Left,
-            clip,
-        )
-    }
-
-    pub fn move_right(&self, point: DisplayPoint, clip: Clip) -> DisplayPoint {
-        self.clip_point_with(
-            DisplayPoint::new(point.row(), point.column() + 1),
-            Bias::Right,
-            clip,
-        )
-    }
-
-    pub fn clip_point_with(&self, point: DisplayPoint, bias: Bias, clip: Clip) -> DisplayPoint {
-        let new_point = DisplayPoint(self.block_snapshot.clip_point(point.0, bias));
-        if clip == Clip::EndOfLine && new_point.column() == self.line_len(new_point.row()) {
-            self.move_left(new_point, Clip::None)
-        } else {
-            new_point
+    pub fn clip_point(&self, point: DisplayPoint, bias: Bias) -> DisplayPoint {
+        let mut clipped = self.block_snapshot.clip_point(point.0, bias);
+        if self.clip_at_line_ends {
+            clipped = self.clip_at_line_end(DisplayPoint(clipped)).0
         }
+        DisplayPoint(clipped)
     }
 
-    pub fn clip_point(&self, point: DisplayPoint, bias: Bias) -> DisplayPoint {
-        self.clip_point_with(point, bias, self.default_clip)
+    pub fn clip_at_line_end(&self, point: DisplayPoint) -> DisplayPoint {
+        let mut point = point.0;
+        if point.column == self.line_len(point.row) {
+            point.column = point.column.saturating_sub(1);
+            point = self.block_snapshot.clip_point(point, Bias::Left);
+        }
+        DisplayPoint(point)
     }
 
     pub fn folds_in_range<T>(&self, range: Range<T>) -> impl Iterator<Item = &Range<Anchor>>
@@ -1598,7 +1583,7 @@ pub mod tests {
 
         fn assert(text: &str, cx: &mut gpui::AppContext) {
             let (mut unmarked_snapshot, markers) = marked_display_snapshot(text, cx);
-            unmarked_snapshot.default_clip = Clip::EndOfLine;
+            unmarked_snapshot.clip_at_line_ends = true;
             assert_eq!(
                 unmarked_snapshot.clip_point(markers[1], Bias::Left),
                 markers[0]

crates/editor/src/editor.rs 🔗

@@ -1544,10 +1544,10 @@ impl Editor {
         range.clone()
     }
 
-    pub fn set_default_clip(&mut self, clip: Clip, cx: &mut ViewContext<Self>) {
-        if self.display_map.read(cx).default_clip != clip {
+    pub fn set_clip_at_line_ends(&mut self, clip: bool, cx: &mut ViewContext<Self>) {
+        if self.display_map.read(cx).clip_at_line_ends != clip {
             self.display_map
-                .update(cx, |map, _| map.default_clip = clip);
+                .update(cx, |map, _| map.clip_at_line_ends = clip);
         }
     }
 

crates/editor/src/element.rs 🔗

@@ -847,23 +847,26 @@ impl EditorElement {
 
                 if editor.show_local_cursors(cx) || replica_id != local_replica_id {
                     let cursor_position = selection.head;
+                    let mut cursor_column = cursor_position.column() as usize;
+                    let mut cursor_row = cursor_position.row();
 
-                    if layout
-                        .visible_display_row_range
-                        .contains(&cursor_position.row())
+                    if CursorShape::Block == selection.cursor_shape
+                        && !selection.range.is_empty()
+                        && !selection.reversed
                     {
-                        let cursor_row_layout = &layout.position_map.line_layouts
-                            [(cursor_position.row() - start_row) as usize]
-                            .line;
-                        let mut cursor_column = cursor_position.column() as usize;
-
-                        if CursorShape::Block == selection.cursor_shape
-                            && !selection.range.is_empty()
-                            && !selection.reversed
-                            && cursor_column > 0
-                        {
+                        if cursor_column > 0 {
                             cursor_column -= 1;
+                        } else if cursor_row > 0 {
+                            cursor_row -= 1;
+                            cursor_column =
+                                layout.position_map.snapshot.line_len(cursor_row) as usize;
                         }
+                    }
+
+                    if layout.visible_display_row_range.contains(&cursor_row) {
+                        let cursor_row_layout = &layout.position_map.line_layouts
+                            [(cursor_row - start_row) as usize]
+                            .line;
 
                         let cursor_character_x = cursor_row_layout.x_for_index(cursor_column);
                         let mut block_width =
@@ -876,7 +879,7 @@ impl EditorElement {
                                 .position_map
                                 .snapshot
                                 .chars_at(DisplayPoint::new(
-                                    cursor_position.row(),
+                                    cursor_row as u32,
                                     cursor_column as u32,
                                 ))
                                 .next()
@@ -903,8 +906,7 @@ impl EditorElement {
                         };
 
                         let x = cursor_character_x - scroll_left;
-                        let y = cursor_position.row() as f32 * layout.position_map.line_height
-                            - scroll_top;
+                        let y = cursor_row as f32 * layout.position_map.line_height - scroll_top;
                         if selection.is_newest {
                             editor.pixel_position_of_newest_cursor = Some(vec2f(
                                 bounds.origin_x() + x + block_width / 2.,

crates/editor/src/movement.rs 🔗

@@ -13,6 +13,13 @@ pub fn left(map: &DisplaySnapshot, mut point: DisplayPoint) -> DisplayPoint {
     map.clip_point(point, Bias::Left)
 }
 
+pub fn saturating_left(map: &DisplaySnapshot, mut point: DisplayPoint) -> DisplayPoint {
+    if point.column() > 0 {
+        *point.column_mut() -= 1;
+    }
+    map.clip_point(point, Bias::Left)
+}
+
 pub fn right(map: &DisplaySnapshot, mut point: DisplayPoint) -> DisplayPoint {
     let max_column = map.line_len(point.row());
     if point.column() < max_column {
@@ -24,6 +31,11 @@ pub fn right(map: &DisplaySnapshot, mut point: DisplayPoint) -> DisplayPoint {
     map.clip_point(point, Bias::Right)
 }
 
+pub fn saturating_right(map: &DisplaySnapshot, mut point: DisplayPoint) -> DisplayPoint {
+    *point.column_mut() += 1;
+    map.clip_point(point, Bias::Right)
+}
+
 pub fn up(
     map: &DisplaySnapshot,
     start: DisplayPoint,

crates/editor/src/selections_collection.rs 🔗

@@ -498,6 +498,7 @@ impl<'a> MutableSelectionsCollection<'a> {
         T: ToOffset + ToPoint + Ord + std::marker::Copy + std::fmt::Debug,
     {
         let buffer = self.buffer.read(self.cx).snapshot(self.cx);
+
         selections.sort_unstable_by_key(|s| s.start);
         // Merge overlapping selections.
         let mut i = 1;

crates/vim/src/mode_indicator.rs 🔗

@@ -87,7 +87,7 @@ impl View for ModeIndicator {
             Mode::Normal => "-- NORMAL --",
             Mode::Insert => "-- INSERT --",
             Mode::Visual { line: false } => "-- VISUAL --",
-            Mode::Visual { line: true } => "VISUAL LINE ",
+            Mode::Visual { line: true } => "VISUAL  LINE",
         };
         Label::new(text, theme.vim_mode_indicator.text.clone())
             .contained()

crates/vim/src/motion.rs 🔗

@@ -2,7 +2,7 @@ use std::sync::Arc;
 
 use editor::{
     char_kind,
-    display_map::{Clip, DisplaySnapshot, ToDisplayPoint},
+    display_map::{DisplaySnapshot, ToDisplayPoint},
     movement, Bias, CharKind, DisplayPoint, ToOffset,
 };
 use gpui::{actions, impl_actions, AppContext, WindowContext};
@@ -295,11 +295,7 @@ impl Motion {
                 SelectionGoal::None,
             ),
             EndOfParagraph => (
-                map.clip_point_with(
-                    movement::end_of_paragraph(map, point, times),
-                    Bias::Left,
-                    Clip::EndOfLine,
-                ),
+                map.clip_at_line_end(movement::end_of_paragraph(map, point, times)),
                 SelectionGoal::None,
             ),
             CurrentLine => (end_of_line(map, point), SelectionGoal::None),
@@ -387,7 +383,7 @@ impl Motion {
 
 fn left(map: &DisplaySnapshot, mut point: DisplayPoint, times: usize) -> DisplayPoint {
     for _ in 0..times {
-        point = map.move_left(point, Clip::None);
+        point = movement::saturating_left(map, point);
         if point.column() == 0 {
             break;
         }
@@ -428,7 +424,7 @@ fn up(
 
 pub(crate) fn right(map: &DisplaySnapshot, mut point: DisplayPoint, times: usize) -> DisplayPoint {
     for _ in 0..times {
-        let new_point = map.clip_point(map.move_right(point, Clip::None), Bias::Right);
+        let new_point = movement::saturating_right(map, point);
         if point == new_point {
             break;
         }

crates/vim/src/normal.rs 🔗

@@ -16,9 +16,8 @@ use crate::{
 };
 use collections::{HashMap, HashSet};
 use editor::{
-    display_map::{Clip, ToDisplayPoint},
-    scroll::autoscroll::Autoscroll,
-    Anchor, Bias, ClipboardSelection, DisplayPoint,
+    display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Anchor, Bias, ClipboardSelection,
+    DisplayPoint,
 };
 use gpui::{actions, AppContext, ViewContext, WindowContext};
 use language::{AutoindentMode, Point, SelectionGoal};
@@ -255,7 +254,7 @@ fn paste(_: &mut Workspace, _: &Paste, cx: &mut ViewContext<Workspace>) {
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
             editor.transact(cx, |editor, cx| {
-                editor.set_default_clip(Clip::None, cx);
+                editor.set_clip_at_line_ends(false, cx);
                 if let Some(item) = cx.read_from_clipboard() {
                     let mut clipboard_text = Cow::Borrowed(item.text());
                     if let Some(mut clipboard_selections) =
@@ -383,7 +382,7 @@ fn paste(_: &mut Workspace, _: &Paste, cx: &mut ViewContext<Workspace>) {
                         editor.insert(&clipboard_text, cx);
                     }
                 }
-                editor.set_default_clip(Clip::EndOfLine, cx);
+                editor.set_clip_at_line_ends(true, cx);
             });
         });
     });
@@ -393,7 +392,7 @@ pub(crate) fn normal_replace(text: Arc<str>, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
             editor.transact(cx, |editor, cx| {
-                editor.set_default_clip(Clip::None, cx);
+                editor.set_clip_at_line_ends(false, cx);
                 let (map, display_selections) = editor.selections.all_display(cx);
                 // Selections are biased right at the start. So we need to store
                 // anchors that are biased left so that we can restore the selections
@@ -426,7 +425,7 @@ pub(crate) fn normal_replace(text: Arc<str>, cx: &mut WindowContext) {
                 editor.buffer().update(cx, |buffer, cx| {
                     buffer.edit(edits, None, cx);
                 });
-                editor.set_default_clip(Clip::EndOfLine, cx);
+                editor.set_clip_at_line_ends(true, cx);
                 editor.change_selections(None, cx, |s| {
                     s.select_anchor_ranges(stable_anchors);
                 });

crates/vim/src/normal/change.rs 🔗

@@ -1,10 +1,7 @@
 use crate::{motion::Motion, object::Object, state::Mode, utils::copy_selections_content, Vim};
 use editor::{
-    char_kind,
-    display_map::{Clip, DisplaySnapshot},
-    movement,
-    scroll::autoscroll::Autoscroll,
-    CharKind, DisplayPoint,
+    char_kind, display_map::DisplaySnapshot, movement, scroll::autoscroll::Autoscroll, CharKind,
+    DisplayPoint,
 };
 use gpui::WindowContext;
 use language::Selection;
@@ -18,7 +15,7 @@ pub fn change_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &m
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
             // We are swapping to insert mode anyway. Just set the line end clipping behavior now
-            editor.set_default_clip(Clip::None, cx);
+            editor.set_clip_at_line_ends(false, cx);
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.move_with(|map, selection| {
                     motion_succeeded |= if let Motion::NextWordStart { ignore_punctuation } = motion
@@ -45,7 +42,7 @@ pub fn change_object(vim: &mut Vim, object: Object, around: bool, cx: &mut Windo
     let mut objects_found = false;
     vim.update_active_editor(cx, |editor, cx| {
         // We are swapping to insert mode anyway. Just set the line end clipping behavior now
-        editor.set_default_clip(Clip::None, cx);
+        editor.set_clip_at_line_ends(false, cx);
         editor.transact(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.move_with(|map, selection| {

crates/vim/src/normal/delete.rs 🔗

@@ -1,16 +1,12 @@
 use crate::{motion::Motion, object::Object, utils::copy_selections_content, Vim};
 use collections::{HashMap, HashSet};
-use editor::{
-    display_map::{Clip, ToDisplayPoint},
-    scroll::autoscroll::Autoscroll,
-    Bias,
-};
+use editor::{display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Bias};
 use gpui::WindowContext;
 
 pub fn delete_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
-            editor.set_default_clip(Clip::None, cx);
+            editor.set_clip_at_line_ends(false, cx);
             let mut original_columns: HashMap<_, _> = Default::default();
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.move_with(|map, selection| {
@@ -23,7 +19,7 @@ pub fn delete_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &m
             editor.insert("", cx);
 
             // Fixup cursor position after the deletion
-            editor.set_default_clip(Clip::EndOfLine, cx);
+            editor.set_clip_at_line_ends(true, cx);
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.move_with(|map, selection| {
                     let mut cursor = selection.head();
@@ -43,7 +39,7 @@ pub fn delete_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &m
 pub fn delete_object(vim: &mut Vim, object: Object, around: bool, cx: &mut WindowContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
-            editor.set_default_clip(Clip::None, cx);
+            editor.set_clip_at_line_ends(false, cx);
             // Emulates behavior in vim where if we expanded backwards to include a newline
             // the cursor gets set back to the start of the line
             let mut should_move_to_start: HashSet<_> = Default::default();
@@ -81,7 +77,7 @@ pub fn delete_object(vim: &mut Vim, object: Object, around: bool, cx: &mut Windo
             editor.insert("", cx);
 
             // Fixup cursor position after the deletion
-            editor.set_default_clip(Clip::EndOfLine, cx);
+            editor.set_clip_at_line_ends(true, cx);
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.move_with(|map, selection| {
                     let mut cursor = selection.head();

crates/vim/src/normal/yank.rs 🔗

@@ -1,12 +1,11 @@
 use crate::{motion::Motion, object::Object, utils::copy_selections_content, Vim};
 use collections::HashMap;
-use editor::display_map::Clip;
 use gpui::WindowContext;
 
 pub fn yank_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
-            editor.set_default_clip(Clip::None, cx);
+            editor.set_clip_at_line_ends(false, cx);
             let mut original_positions: HashMap<_, _> = Default::default();
             editor.change_selections(None, cx, |s| {
                 s.move_with(|map, selection| {
@@ -29,7 +28,7 @@ pub fn yank_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut
 pub fn yank_object(vim: &mut Vim, object: Object, around: bool, cx: &mut WindowContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
-            editor.set_default_clip(Clip::None, cx);
+            editor.set_clip_at_line_ends(false, cx);
             let mut original_positions: HashMap<_, _> = Default::default();
             editor.change_selections(None, cx, |s| {
                 s.move_with(|map, selection| {

crates/vim/src/object.rs 🔗

@@ -369,7 +369,7 @@ fn surrounding_markers(
                     start = Some(point)
                 } else {
                     *point.column_mut() += char.len_utf8() as u32;
-                    start = Some(point);
+                    start = Some(point)
                 }
                 break;
             }
@@ -420,11 +420,38 @@ fn surrounding_markers(
         }
     }
 
-    if let (Some(start), Some(end)) = (start, end) {
-        Some(start..end)
-    } else {
-        None
+    let (Some(mut start), Some(mut end)) = (start, end) else {
+        return None;
+    };
+
+    if !around {
+        // if a block starts with a newline, move the start to after the newline.
+        let mut was_newline = false;
+        for (char, point) in map.chars_at(start) {
+            if was_newline {
+                start = point;
+            } else if char == '\n' {
+                was_newline = true;
+                continue;
+            }
+            break;
+        }
+        // if a block ends with a newline, then whitespace, then the delimeter,
+        // move the end to after the newline.
+        let mut new_end = end;
+        for (char, point) in map.reverse_chars_at(end) {
+            if char == '\n' {
+                end = new_end;
+                break;
+            }
+            if !char.is_whitespace() {
+                break;
+            }
+            new_end = point
+        }
     }
+
+    Some(start..end)
 }
 
 #[cfg(test)]
@@ -681,6 +708,48 @@ mod test {
         }
     }
 
+    #[gpui::test]
+    async fn test_multiline_surrounding_character_objects(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state(indoc! {
+            "func empty(a string) bool {
+               if a == \"\" {
+                  return true
+               }
+               ˇreturn false
+            }"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "i", "{"]).await;
+        cx.assert_shared_state(indoc! {"
+            func empty(a string) bool {
+            «   if a == \"\" {
+                  return true
+               }
+               return false
+            ˇ»}"})
+            .await;
+        cx.set_shared_state(indoc! {
+            "func empty(a string) bool {
+                 if a == \"\" {
+                     ˇreturn true
+                 }
+                 return false
+            }"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "i", "{"]).await;
+        cx.assert_shared_state(indoc! {"
+            func empty(a string) bool {
+                 if a == \"\" {
+            «         return true
+            ˇ»     }
+                 return false
+            }"})
+            .await;
+    }
+
     #[gpui::test]
     async fn test_delete_surrounding_character_objects(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;

crates/vim/src/state.rs 🔗

@@ -1,4 +1,3 @@
-use editor::display_map::Clip;
 use gpui::keymap_matcher::KeymapContext;
 use language::CursorShape;
 use serde::{Deserialize, Serialize};
@@ -88,10 +87,10 @@ impl VimState {
             )
     }
 
-    pub fn default_clip(&self) -> Clip {
+    pub fn clip_at_line_ends(&self) -> bool {
         match self.mode {
-            Mode::Insert | Mode::Visual { .. } => Clip::None,
-            Mode::Normal => Clip::EndOfLine,
+            Mode::Insert | Mode::Visual { .. } => false,
+            Mode::Normal => true,
         }
     }
 

crates/vim/src/test/neovim_connection.rs 🔗

@@ -237,10 +237,11 @@ impl NeovimConnection {
             .join("\n");
 
         // nvim columns are 1-based, so -1.
-        let cursor_row = self.read_position("echo line('.')").await - 1;
+        let mut cursor_row = self.read_position("echo line('.')").await - 1;
         let mut cursor_col = self.read_position("echo col('.')").await - 1;
-        let selection_row = self.read_position("echo line('v')").await - 1;
+        let mut selection_row = self.read_position("echo line('v')").await - 1;
         let mut selection_col = self.read_position("echo col('v')").await - 1;
+        let total_rows = self.read_position("echo line('$')").await - 1;
 
         let nvim_mode_text = self
             .nvim
@@ -273,14 +274,20 @@ impl NeovimConnection {
                 if selection_col > cursor_col {
                     let selection_line_length =
                         self.read_position("echo strlen(getline(line('v')))").await;
-                    if selection_line_length > 0 {
+                    if selection_line_length > selection_col {
                         selection_col += 1;
+                    } else if selection_row < total_rows {
+                        selection_col = 0;
+                        selection_row += 1;
                     }
                 } else {
                     let cursor_line_length =
                         self.read_position("echo strlen(getline(line('.')))").await;
-                    if cursor_line_length > 0 {
+                    if cursor_line_length > cursor_col {
                         cursor_col += 1;
+                    } else if cursor_row < total_rows {
+                        cursor_col = 0;
+                        cursor_row += 1;
                     }
                 }
             }

crates/vim/src/vim.rs 🔗

@@ -13,7 +13,7 @@ mod visual;
 
 use anyhow::Result;
 use collections::CommandPaletteFilter;
-use editor::{display_map::Clip, Editor, EditorMode, Event};
+use editor::{movement, Editor, EditorMode, Event};
 use gpui::{
     actions, impl_actions, keymap_matcher::KeymapContext, keymap_matcher::MatchResult, AppContext,
     Subscription, ViewContext, ViewHandle, WeakViewHandle, WindowContext,
@@ -201,12 +201,12 @@ impl Vim {
                     if last_mode.is_visual() && !mode.is_visual() {
                         let mut point = selection.head();
                         if !selection.reversed {
-                            point = map.move_left(selection.head(), Clip::None);
+                            point = movement::left(map, selection.head());
                         }
                         selection.collapse_to(point, selection.goal)
                     } else if !last_mode.is_visual() && mode.is_visual() {
                         if selection.is_empty() {
-                            selection.end = map.move_right(selection.start, Clip::None);
+                            selection.end = movement::right(map, selection.start);
                         }
                     }
                 });
@@ -314,7 +314,7 @@ impl Vim {
         self.update_active_editor(cx, |editor, cx| {
             if self.enabled && editor.mode() == EditorMode::Full {
                 editor.set_cursor_shape(cursor_shape, cx);
-                editor.set_default_clip(state.default_clip(), cx);
+                editor.set_clip_at_line_ends(state.clip_at_line_ends(), cx);
                 editor.set_collapse_matches(true);
                 editor.set_input_enabled(!state.vim_controlled());
                 editor.selections.line_mode = matches!(state.mode, Mode::Visual { line: true });
@@ -331,7 +331,7 @@ impl Vim {
 
     fn unhook_vim_settings(&self, editor: &mut Editor, cx: &mut ViewContext<Editor>) {
         editor.set_cursor_shape(CursorShape::Bar, cx);
-        editor.set_default_clip(Clip::None, cx);
+        editor.set_clip_at_line_ends(false, cx);
         editor.set_input_enabled(true);
         editor.selections.line_mode = false;
 

crates/vim/src/visual.rs 🔗

@@ -2,10 +2,7 @@ use std::{borrow::Cow, sync::Arc};
 
 use collections::HashMap;
 use editor::{
-    display_map::{Clip, ToDisplayPoint},
-    movement,
-    scroll::autoscroll::Autoscroll,
-    Bias, ClipboardSelection,
+    display_map::ToDisplayPoint, movement, scroll::autoscroll::Autoscroll, Bias, ClipboardSelection,
 };
 use gpui::{actions, AppContext, ViewContext, WindowContext};
 use language::{AutoindentMode, SelectionGoal};
@@ -53,7 +50,7 @@ pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContex
                     // but in (forward) visual mode the current character is just
                     // before the end of the selection.
                     if !selection.reversed {
-                        current_head = map.move_left(current_head, Clip::None);
+                        current_head = movement::left(map, selection.end)
                     }
 
                     let Some((new_head, goal)) =
@@ -63,16 +60,16 @@ pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContex
 
                     // ensure the current character is included in the selection.
                     if !selection.reversed {
-                        selection.end = map.move_right(selection.end, Clip::None);
+                        selection.end = movement::right(map, selection.end)
                     }
 
                     // 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 = map.move_left(selection.start, Clip::None);
+                        selection.start = movement::left(map, selection.start);
                     } else if !was_reversed && selection.reversed {
-                        selection.end = map.move_right(selection.end, Clip::None);
+                        selection.end = movement::right(map, selection.end);
                     }
                 });
             });
@@ -94,7 +91,7 @@ pub fn visual_object(object: Object, cx: &mut WindowContext) {
                         // after the cursor; however in the case of a visual selection
                         // the current character is before the cursor.
                         if !selection.reversed {
-                            head = map.move_left(head, Clip::None);
+                            head = movement::left(map, head);
                         }
 
                         if let Some(range) = object.range(map, head, around) {
@@ -109,7 +106,6 @@ pub fn visual_object(object: Object, cx: &mut WindowContext) {
                                 } else {
                                     false
                                 };
-                                dbg!(expand_both_ways);
 
                                 if expand_both_ways {
                                     selection.start = range.start;
@@ -206,7 +202,7 @@ pub fn delete(_: &mut Workspace, _: &VisualDelete, cx: &mut ViewContext<Workspac
                     if line_mode {
                         let mut position = selection.head();
                         if !selection.reversed {
-                            position = map.move_left(position, Clip::None);
+                            position = movement::left(map, position);
                         }
                         original_columns.insert(selection.id, position.to_point(map).column);
                     }
@@ -224,11 +220,7 @@ pub fn delete(_: &mut Workspace, _: &VisualDelete, cx: &mut ViewContext<Workspac
                     if let Some(column) = original_columns.get(&selection.id) {
                         cursor.column = *column
                     }
-                    let cursor = map.clip_point_with(
-                        cursor.to_display_point(map),
-                        Bias::Left,
-                        Clip::EndOfLine,
-                    );
+                    let cursor = map.clip_at_line_end(cursor.to_display_point(map));
                     selection.collapse_to(cursor, selection.goal)
                 });
             });
@@ -401,6 +393,7 @@ pub(crate) fn visual_replace(text: Arc<str>, cx: &mut WindowContext) {
 #[cfg(test)]
 mod test {
     use indoc::indoc;
+    use workspace::item::Item;
 
     use crate::{
         state::Mode,
@@ -417,6 +410,7 @@ mod test {
             the lazy dog"
         })
         .await;
+        let cursor = cx.update_editor(|editor, _| editor.pixel_position_of_cursor());
 
         // entering visual mode should select the character
         // under cursor
@@ -425,6 +419,7 @@ mod test {
             fox jumps over
             the lazy dog"})
             .await;
+        cx.update_editor(|editor, _| assert_eq!(cursor, editor.pixel_position_of_cursor()));
 
         // forwards motions should extend the selection
         cx.simulate_shared_keystrokes(["w", "j"]).await;
@@ -446,6 +441,87 @@ mod test {
             fox jumps o»ver
             the lazy dog"})
             .await;
+
+        // works on empty lines
+        cx.set_shared_state(indoc! {"
+            a
+            ˇ
+            b
+            "})
+            .await;
+        let cursor = cx.update_editor(|editor, _| editor.pixel_position_of_cursor());
+        cx.simulate_shared_keystrokes(["v"]).await;
+        cx.assert_shared_state(indoc! {"
+            a
+            «
+            ˇ»b
+        "})
+            .await;
+        cx.update_editor(|editor, _| assert_eq!(cursor, editor.pixel_position_of_cursor()));
+
+        // toggles off again
+        cx.simulate_shared_keystrokes(["v"]).await;
+        cx.assert_shared_state(indoc! {"
+            a
+            ˇ
+            b
+            "})
+            .await;
+
+        // works at the end of a document
+        cx.set_shared_state(indoc! {"
+            a
+            b
+            ˇ"})
+            .await;
+
+        cx.simulate_shared_keystrokes(["v"]).await;
+        cx.assert_shared_state(indoc! {"
+            a
+            b
+            ˇ"})
+            .await;
+        assert_eq!(cx.mode(), cx.neovim_mode().await);
+    }
+
+    #[gpui::test]
+    async fn test_enter_visual_line_mode(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state(indoc! {
+            "The ˇquick brown
+            fox jumps over
+            the lazy dog"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["shift-v"]).await;
+        cx.assert_shared_state(indoc! { "The «qˇ»uick brown
+            fox jumps over
+            the lazy dog"})
+            .await;
+        assert_eq!(cx.mode(), cx.neovim_mode().await);
+        cx.simulate_shared_keystrokes(["x"]).await;
+        cx.assert_shared_state(indoc! { "fox ˇjumps over
+        the lazy dog"})
+            .await;
+
+        // it should work on empty lines
+        cx.set_shared_state(indoc! {"
+            a
+            ˇ
+            b"})
+            .await;
+        cx.simulate_shared_keystrokes(["shift-v"]).await;
+        cx.assert_shared_state(indoc! { "
+            a
+            «
+            ˇ»b"})
+            .await;
+        cx.simulate_shared_keystrokes(["x"]).await;
+        cx.assert_shared_state(indoc! { "
+            a
+            ˇb"})
+            .await;
     }
 
     #[gpui::test]

crates/vim/test_data/test_enter_visual_line_mode.json 🔗

@@ -0,0 +1,10 @@
+{"Put":{"state":"The ˇquick brown\nfox jumps over\nthe lazy dog"}}
+{"Key":"shift-v"}
+{"Get":{"state":"The «qˇ»uick brown\nfox jumps over\nthe lazy dog","mode":{"Visual":{"line":true}}}}
+{"Key":"x"}
+{"Get":{"state":"fox ˇjumps over\nthe lazy dog","mode":"Normal"}}
+{"Put":{"state":"a\nˇ\nb"}}
+{"Key":"shift-v"}
+{"Get":{"state":"a\n«\nˇ»b","mode":{"Visual":{"line":true}}}}
+{"Key":"x"}
+{"Get":{"state":"a\nˇb","mode":"Normal"}}

crates/vim/test_data/test_enter_visual_mode.json 🔗

@@ -10,3 +10,11 @@
 {"Key":"k"}
 {"Key":"b"}
 {"Get":{"state":"The «ˇquick brown\nfox jumps o»ver\nthe lazy dog","mode":{"Visual":{"line":false}}}}
+{"Put":{"state":"a\nˇ\nb\n"}}
+{"Key":"v"}
+{"Get":{"state":"a\n«\nˇ»b\n","mode":{"Visual":{"line":false}}}}
+{"Key":"v"}
+{"Get":{"state":"a\nˇ\nb\n","mode":"Normal"}}
+{"Put":{"state":"a\nb\nˇ"}}
+{"Key":"v"}
+{"Get":{"state":"a\nb\nˇ","mode":{"Visual":{"line":false}}}}

crates/vim/test_data/test_multiline_surrounding_character_objects.json 🔗

@@ -0,0 +1,10 @@
+{"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":{"line":false}}}}
+{"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":{"line":false}}}}

crates/vim/test_data/test_visual_word_object.json 🔗

@@ -43,17 +43,17 @@
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \nˇ\n\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n«\nˇ»\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
 {"Put":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\nˇ\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n"}}
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\nˇ\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n«\nˇ»\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
 {"Put":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\nˇ\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n"}}
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\nˇ\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n«\nˇ»The-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
 {"Put":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n\nThˇe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n"}}
 {"Key":"v"}
 {"Key":"i"}
@@ -118,7 +118,7 @@
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \nˇ\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n«\nˇ»","mode":{"Visual":{"line":false}}}}
 {"Put":{"state":"The quick ˇbrown   \nfox jumps over\nthe lazy dog  \n\n\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n"}}
 {"Key":"v"}
 {"Key":"i"}
@@ -158,17 +158,17 @@
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"shift-w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \nˇ\n\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n«\nˇ»\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
 {"Put":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\nˇ\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n"}}
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"shift-w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\nˇ\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n«\nˇ»\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
 {"Put":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\nˇ\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n"}}
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"shift-w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\nˇ\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n«\nˇ»The-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n","mode":{"Visual":{"line":false}}}}
 {"Put":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n\nThˇe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n\n"}}
 {"Key":"v"}
 {"Key":"i"}
@@ -233,4 +233,4 @@
 {"Key":"v"}
 {"Key":"i"}
 {"Key":"shift-w"}
-{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \nˇ\n","mode":{"Visual":{"line":false}}}}
+{"Get":{"state":"The quick brown   \nfox jumps over\nthe lazy dog  \n\n\n\nThe-quick brown \n  \n  \n  fox-jumps over\nthe lazy dog \n«\nˇ»","mode":{"Visual":{"line":false}}}}