Unify visual line_mode and non line_mode operators

assets/keymaps/vim.json

@@ -9,7 +9,7 @@
                 }
             ],
             "h": "vim::Left",
-            "backspace": "editor::Backspace", // "vim::Left",
+            "backspace": "vim::Left",
             "j": "vim::Down",
             "k": "vim::Up",
             "l": "vim::Right",
@@ -75,11 +75,19 @@
             "shift-O": "vim::InsertLineAbove",
             "v": [
                 "vim::SwitchMode",
-                "Visual"
+                {
+                    "Visual": {
+                        "line": false
+                    }
+                }
             ],
             "shift-V": [
                 "vim::SwitchMode",
-                "VisualLine"
+                {
+                    "Visual": {
+                        "line": true
+                    }
+                }
             ],
             "p": "vim::Paste",
             "u": "editor::Undo",
@@ -131,15 +139,6 @@
             "y": "vim::VisualYank"
         }
     },
-    {
-        "context": "Editor && vim_mode == visual_line",
-        "bindings": {
-            "c": "vim::VisualLineChange",
-            "d": "vim::VisualLineDelete",
-            "x": "vim::VisualLineDelete",
-            "y": "vim::VisualLineYank"
-        }
-    },
     {
         "context": "Editor && vim_mode == insert",
         "bindings": {

crates/editor/src/display_map.rs

@@ -279,16 +279,21 @@ impl DisplaySnapshot {
         }
     }
 
-    pub fn expand_to_line(&self, mut range: Range<Point>) -> Range<Point> {
+    pub fn expand_to_line(&self, range: Range<Point>) -> Range<Point> {
-        (range.start, _) = self.prev_line_boundary(range.start);
+        let mut new_start = self.prev_line_boundary(range.start).0;
-        (range.end, _) = self.next_line_boundary(range.end);
+        let mut new_end = self.next_line_boundary(range.end).0;
 
-        if range.is_empty() && range.start.row > 0 {
+        if new_start.row == range.start.row && new_end.row == range.end.row {
-            range.start.row -= 1;
+            if new_end.row < self.buffer_snapshot.max_point().row {
-            range.start.column = self.buffer_snapshot.line_len(range.start.row);
+                new_end.row += 1;
+                new_end.column = 0;
+            } else if new_start.row > 0 {
+                new_start.row -= 1;
+                new_start.column = self.buffer_snapshot.line_len(new_start.row);
+            }
         }
 
-        range
+        new_start..new_end
     }
 
     fn point_to_display_point(&self, point: Point, bias: Bias) -> DisplayPoint {

crates/editor/src/editor.rs

@@ -1866,7 +1866,10 @@ impl Editor {
                     let snapshot = buffer.read(cx);
                     old_selections
                         .iter()
-                        .map(|s| (s.id, s.goal, snapshot.anchor_after(s.end)))
+                        .map(|s| {
+                            let anchor = snapshot.anchor_after(s.end);
+                            s.map(|_| anchor.clone())
+                        })
                         .collect::<Vec<_>>()
                 };
                 buffer.edit_with_autoindent(
@@ -1878,25 +1881,8 @@ impl Editor {
                 anchors
             });
 
-            let selections = {
-                let snapshot = this.buffer.read(cx).read(cx);
-                selection_anchors
-                    .into_iter()
-                    .map(|(id, goal, position)| {
-                        let position = position.to_offset(&snapshot);
-                        Selection {
-                            id,
-                            start: position,
-                            end: position,
-                            goal,
-                            reversed: false,
-                        }
-                    })
-                    .collect()
-            };
-
             this.change_selections(Some(Autoscroll::Fit), cx, |s| {
-                s.select(selections);
+                s.select_anchors(selection_anchors);
             })
         });
     }

crates/editor/src/selections_collection.rs

@@ -22,6 +22,13 @@ pub struct PendingSelection {
     pub mode: SelectMode,
 }
 
+#[derive(Clone)]
+pub enum LineMode {
+    None,
+    WithNewline,
+    WithoutNewline,
+}
+
 #[derive(Clone)]
 pub struct SelectionsCollection {
     display_map: ModelHandle<DisplayMap>,

crates/vim/src/editor_events.rs

@@ -58,7 +58,7 @@ fn editor_released(EditorReleased(editor): &EditorReleased, cx: &mut MutableAppC
 fn editor_local_selections_changed(newest_empty: bool, cx: &mut MutableAppContext) {
     Vim::update(cx, |vim, cx| {
         if vim.state.mode == Mode::Normal && !newest_empty {
-            vim.switch_mode(Mode::Visual, cx)
+            vim.switch_mode(Mode::Visual { line: false }, cx)
         }
     })
 }

crates/vim/src/motion.rs

@@ -111,8 +111,7 @@ fn motion(motion: Motion, cx: &mut MutableAppContext) {
     });
     match Vim::read(cx).state.mode {
         Mode::Normal => normal_motion(motion, cx),
-        Mode::Visual => visual_motion(motion, cx),
+        Mode::Visual { .. } => visual_motion(motion, cx),
-        Mode::VisualLine => visual_motion(motion, cx),
         Mode::Insert => {
             // Shouldn't execute a motion in insert mode. Ignoring
         }

crates/vim/src/normal.rs

@@ -777,14 +777,8 @@ mod test {
                 |
                 The quick"},
         );
-        cx.assert(
+        // Indoc disallows trailing whitspace.
-            indoc! {"
+        cx.assert("   | \nThe quick", "   | \nThe quick");
-                    |
-                The quick"},
-            indoc! {"
-                    |
-                The quick"},
-        );
     }
 
     #[gpui::test]

crates/vim/src/state.rs

@@ -6,8 +6,7 @@ use serde::Deserialize;
 pub enum Mode {
     Normal,
     Insert,
-    Visual,
+    Visual { line: bool },
-    VisualLine,
 }
 
 impl Default for Mode {
@@ -38,7 +37,7 @@ pub struct VimState {
 impl VimState {
     pub fn cursor_shape(&self) -> CursorShape {
         match self.mode {
-            Mode::Normal | Mode::Visual | Mode::VisualLine => CursorShape::Block,
+            Mode::Normal | Mode::Visual { .. } => CursorShape::Block,
             Mode::Insert => CursorShape::Bar,
         }
     }
@@ -49,13 +48,13 @@ impl VimState {
 
     pub fn clip_at_line_end(&self) -> bool {
         match self.mode {
-            Mode::Insert | Mode::Visual | Mode::VisualLine => false,
+            Mode::Insert | Mode::Visual { .. } => false,
             _ => true,
         }
     }
 
     pub fn empty_selections_only(&self) -> bool {
-        self.mode != Mode::Visual && self.mode != Mode::VisualLine
+        !matches!(self.mode, Mode::Visual { .. })
     }
 
     pub fn keymap_context_layer(&self) -> Context {
@@ -64,8 +63,7 @@ impl VimState {
             "vim_mode".to_string(),
             match self.mode {
                 Mode::Normal => "normal",
-                Mode::Visual => "visual",
+                Mode::Visual { .. } => "visual",
-                Mode::VisualLine => "visual_line",
                 Mode::Insert => "insert",
             }
             .to_string(),

crates/vim/src/utils.rs

@@ -1,9 +1,8 @@
 use editor::{ClipboardSelection, Editor};
 use gpui::{ClipboardItem, MutableAppContext};
-use language::Point;
 
 pub fn copy_selections_content(editor: &mut Editor, linewise: bool, cx: &mut MutableAppContext) {
-    let selections = editor.selections.all::<Point>(cx);
+    let selections = editor.selections.all_adjusted(cx);
     let buffer = editor.buffer().read(cx).snapshot(cx);
     let mut text = String::new();
     let mut clipboard_selections = Vec::with_capacity(selections.len());

crates/vim/src/vim.rs

@@ -10,7 +10,7 @@ mod utils;
 mod visual;
 
 use collections::HashMap;
-use editor::{CursorShape, Editor, Input};
+use editor::{Bias, CursorShape, Editor, Input};
 use gpui::{impl_actions, MutableAppContext, Subscription, ViewContext, WeakViewHandle};
 use serde::Deserialize;
 
@@ -43,7 +43,8 @@ pub fn init(cx: &mut MutableAppContext) {
     );
     cx.add_action(|_: &mut Editor, _: &Input, cx| {
         if Vim::read(cx).active_operator().is_some() {
-            cx.defer(|_, cx| Vim::update(cx, |vim, cx| vim.clear_operator(cx)))
+            // Defer without updating editor
+            MutableAppContext::defer(cx, |cx| Vim::update(cx, |vim, cx| vim.clear_operator(cx)))
         } else {
             cx.propagate_action()
         }
@@ -138,7 +139,8 @@ impl Vim {
                         editor.set_cursor_shape(cursor_shape, cx);
                         editor.set_clip_at_line_ends(state.clip_at_line_end(), cx);
                         editor.set_input_enabled(!state.vim_controlled());
-                        editor.selections.line_mode = state.mode == Mode::VisualLine;
+                        editor.selections.line_mode =
+                            matches!(state.mode, Mode::Visual { line: true });
                         let context_layer = state.keymap_context_layer();
                         editor.set_keymap_context_layer::<Self>(context_layer);
                     } else {
@@ -149,13 +151,17 @@ impl Vim {
                         editor.remove_keymap_context_layer::<Self>();
                     }
 
-                    if state.empty_selections_only() {
                     editor.change_selections(None, cx, |s| {
-                            s.move_with(|_, selection| {
+                        s.move_with(|map, selection| {
+                            selection.set_head(
+                                map.clip_point(selection.head(), Bias::Left),
+                                selection.goal,
+                            );
+                            if state.empty_selections_only() {
                                 selection.collapse_to(selection.head(), selection.goal)
+                            }
                         });
                     })
-                    }
                 });
             }
         }
@@ -190,9 +196,9 @@ mod test {
         assert_eq!(cx.mode(), Mode::Normal);
         cx.simulate_keystrokes(["h", "h", "h", "l"]);
         assert_eq!(cx.editor_text(), "hjkl".to_owned());
-        cx.assert_editor_state("hj|kl");
+        cx.assert_editor_state("h|jkl");
         cx.simulate_keystrokes(["i", "T", "e", "s", "t"]);
-        cx.assert_editor_state("hjTest|kl");
+        cx.assert_editor_state("hTest|jkl");
 
         // Disabling and enabling resets to normal mode
         assert_eq!(cx.mode(), Mode::Insert);

crates/vim/src/vim_test_context.rs

@@ -12,7 +12,7 @@ use util::{
     set_eq,
     test::{marked_text, marked_text_ranges_by, SetEqError},
 };
-use workspace::{AppState, WorkspaceHandle};
+use workspace::{pane, AppState, WorkspaceHandle};
 
 use crate::{state::Operator, *};
 
@@ -26,6 +26,7 @@ impl<'a> VimTestContext<'a> {
     pub async fn new(cx: &'a mut gpui::TestAppContext, enabled: bool) -> VimTestContext<'a> {
         cx.update(|cx| {
             editor::init(cx);
+            pane::init(cx);
             crate::init(cx);
 
             settings::KeymapFileContent::load("keymaps/vim.json", cx).unwrap();
@@ -269,9 +270,12 @@ impl<'a> VimTestContext<'a> {
                 panic!(
                     indoc! {"
                         Editor has extra selection
-                        Extra Selection Location: {}
+                        Extra Selection Location:
-                        Asserted selections: {}
+                        {}
-                        Actual selections: {}"},
+                        Asserted selections:
+                        {}
+                        Actual selections:
+                        {}"},
                     location_text, asserted_selections, actual_selections,
                 );
             }
@@ -279,9 +283,12 @@ impl<'a> VimTestContext<'a> {
                 panic!(
                     indoc! {"
                         Editor is missing empty selection
-                        Missing Selection Location: {}
+                        Missing Selection Location:
-                        Asserted selections: {}
+                        {}
-                        Actual selections: {}"},
+                        Asserted selections:
+                        {}
+                        Actual selections:
+                        {}"},
                     location_text, asserted_selections, actual_selections,
                 );
             }

crates/vim/src/visual.rs

@@ -1,29 +1,17 @@
 use collections::HashMap;
-use editor::{Autoscroll, Bias};
+use editor::{display_map::ToDisplayPoint, Autoscroll, Bias};
 use gpui::{actions, MutableAppContext, ViewContext};
+use language::SelectionGoal;
 use workspace::Workspace;
 
 use crate::{motion::Motion, state::Mode, utils::copy_selections_content, Vim};
 
-actions!(
+actions!(vim, [VisualDelete, VisualChange, VisualYank,]);
-    vim,
-    [
-        VisualDelete,
-        VisualLineDelete,
-        VisualChange,
-        VisualLineChange,
-        VisualYank,
-        VisualLineYank,
-    ]
-);
 
 pub fn init(cx: &mut MutableAppContext) {
     cx.add_action(change);
-    cx.add_action(change_line);
     cx.add_action(delete);
-    cx.add_action(delete_line);
     cx.add_action(yank);
-    cx.add_action(yank_line);
 }
 
 pub fn visual_motion(motion: Motion, cx: &mut MutableAppContext) {
@@ -32,7 +20,6 @@ pub fn visual_motion(motion: Motion, cx: &mut MutableAppContext) {
             editor.change_selections(Some(Autoscroll::Fit), cx, |s| {
                 s.move_with(|map, selection| {
                     let (new_head, goal) = motion.move_point(map, selection.head(), selection.goal);
-                    let new_head = map.clip_at_line_end(new_head);
                     let was_reversed = selection.reversed;
                     selection.set_head(new_head, goal);
 
@@ -57,7 +44,12 @@ pub fn change(_: &mut Workspace, _: &VisualChange, cx: &mut ViewContext<Workspac
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
             editor.set_clip_at_line_ends(false, cx);
-            editor.change_selections(Some(Autoscroll::Fit), cx, |s| {
+            // Compute edits and resulting anchor selections. If in line mode, adjust
+            // the anchor location and additional newline
+            let mut edits = Vec::new();
+            let mut new_selections = Vec::new();
+            let line_mode = editor.selections.line_mode;
+            editor.change_selections(None, cx, |s| {
                 s.move_with(|map, selection| {
                     if !selection.reversed {
                         // Head is at the end of the selection. Adjust the end position to
@@ -65,24 +57,33 @@ pub fn change(_: &mut Workspace, _: &VisualChange, cx: &mut ViewContext<Workspac
                         *selection.end.column_mut() = selection.end.column() + 1;
                         selection.end = map.clip_point(selection.end, Bias::Left);
                     }
-                });
-            });
-            copy_selections_content(editor, false, cx);
-            editor.insert("", cx);
-        });
-        vim.switch_mode(Mode::Insert, cx);
-    });
-}
 
-pub fn change_line(_: &mut Workspace, _: &VisualLineChange, cx: &mut ViewContext<Workspace>) {
+                    if line_mode {
-    Vim::update(cx, |vim, cx| {
+                        let range = selection.map(|p| p.to_point(map)).range();
-        vim.update_active_editor(cx, |editor, cx| {
+                        let expanded_range = map.expand_to_line(range);
-            editor.set_clip_at_line_ends(false, cx);
+                        // If we are at the last line, the anchor needs to be after the newline so that
+                        // it is on a line of its own. Otherwise, the anchor may be after the newline
+                        let anchor = if expanded_range.end == map.buffer_snapshot.max_point() {
+                            map.buffer_snapshot.anchor_after(expanded_range.end)
+                        } else {
+                            map.buffer_snapshot.anchor_before(expanded_range.start)
+                        };
 
-            let adjusted = editor.selections.all_adjusted(cx);
+                        edits.push((expanded_range, "\n"));
-            editor.change_selections(None, cx, |s| s.select(adjusted));
+                        new_selections.push(selection.map(|_| anchor.clone()));
-            copy_selections_content(editor, true, cx);
+                    } else {
-            editor.insert("", cx);
+                        let range = selection.map(|p| p.to_point(map)).range();
+                        let anchor = map.buffer_snapshot.anchor_after(range.end);
+                        edits.push((range, ""));
+                        new_selections.push(selection.map(|_| anchor.clone()));
+                    }
+                });
+            });
+            copy_selections_content(editor, editor.selections.line_mode, cx);
+            editor.edit_with_autoindent(edits, cx);
+            editor.change_selections(Some(Autoscroll::Fit), cx, |s| {
+                s.select_anchors(new_selections);
+            });
         });
         vim.switch_mode(Mode::Insert, cx);
     });
@@ -92,9 +93,14 @@ pub fn delete(_: &mut Workspace, _: &VisualDelete, cx: &mut ViewContext<Workspac
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
             editor.set_clip_at_line_ends(false, cx);
+            let mut original_columns: HashMap<_, _> = Default::default();
+            let line_mode = editor.selections.line_mode;
             editor.change_selections(Some(Autoscroll::Fit), cx, |s| {
                 s.move_with(|map, selection| {
-                    if !selection.reversed {
+                    if line_mode {
+                        original_columns
+                            .insert(selection.id, selection.head().to_point(&map).column);
+                    } else if !selection.reversed {
                         // Head is at the end of the selection. Adjust the end position to
                         // to include the character under the cursor.
                         *selection.end.column_mut() = selection.end.column() + 1;
@@ -102,60 +108,19 @@ pub fn delete(_: &mut Workspace, _: &VisualDelete, cx: &mut ViewContext<Workspac
                     }
                 });
             });
-            copy_selections_content(editor, false, cx);
+            copy_selections_content(editor, line_mode, cx);
             editor.insert("", cx);
 
             // Fixup cursor position after the deletion
             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();
+                    let mut cursor = selection.head().to_point(map);
-                    cursor = map.clip_point(cursor, Bias::Left);
-                    selection.collapse_to(cursor, selection.goal)
-                });
-            });
-        });
-        vim.switch_mode(Mode::Normal, cx);
-    });
-}
 
-pub fn delete_line(_: &mut Workspace, _: &VisualLineDelete, cx: &mut ViewContext<Workspace>) {
-    Vim::update(cx, |vim, cx| {
-        vim.update_active_editor(cx, |editor, 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| {
-                    original_columns.insert(selection.id, selection.head().column());
-                    selection.start = map.prev_line_boundary(selection.start.to_point(map)).1;
-
-                    if selection.end.row() < map.max_point().row() {
-                        *selection.end.row_mut() += 1;
-                        *selection.end.column_mut() = 0;
-                        selection.end = map.clip_point(selection.end, Bias::Right);
-                        // Don't reset the end here
-                        return;
-                    } else if selection.start.row() > 0 {
-                        *selection.start.row_mut() -= 1;
-                        *selection.start.column_mut() = map.line_len(selection.start.row());
-                        selection.start = map.clip_point(selection.start, Bias::Left);
-                    }
-
-                    selection.end = map.next_line_boundary(selection.end.to_point(map)).1;
-                });
-            });
-            copy_selections_content(editor, true, cx);
-            editor.insert("", cx);
-
-            // Fixup cursor position after the deletion
-            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();
                     if let Some(column) = original_columns.get(&selection.id) {
-                        *cursor.column_mut() = *column
+                        cursor.column = *column
                     }
-                    cursor = map.clip_point(cursor, Bias::Left);
+                    let cursor = map.clip_point(cursor.to_display_point(map), Bias::Left);
                     selection.collapse_to(cursor, selection.goal)
                 });
             });
@@ -168,9 +133,10 @@ pub fn yank(_: &mut Workspace, _: &VisualYank, cx: &mut ViewContext<Workspace>)
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
             editor.set_clip_at_line_ends(false, cx);
-            editor.change_selections(Some(Autoscroll::Fit), cx, |s| {
+            let line_mode = editor.selections.line_mode;
+            editor.change_selections(None, cx, |s| {
                 s.move_with(|map, selection| {
-                    if !selection.reversed {
+                    if !line_mode && !selection.reversed {
                         // Head is at the end of the selection. Adjust the end position to
                         // to include the character under the cursor.
                         *selection.end.column_mut() = selection.end.column() + 1;
@@ -178,19 +144,12 @@ pub fn yank(_: &mut Workspace, _: &VisualYank, cx: &mut ViewContext<Workspace>)
                     }
                 });
             });
-            copy_selections_content(editor, false, cx);
+            copy_selections_content(editor, line_mode, cx);
+            editor.change_selections(None, cx, |s| {
+                s.move_with(|_, selection| {
+                    selection.collapse_to(selection.start, SelectionGoal::None)
                 });
-        vim.switch_mode(Mode::Normal, cx);
             });
-}
-
-pub fn yank_line(_: &mut Workspace, _: &VisualLineYank, cx: &mut ViewContext<Workspace>) {
-    Vim::update(cx, |vim, cx| {
-        vim.update_active_editor(cx, |editor, cx| {
-            editor.set_clip_at_line_ends(false, cx);
-            let adjusted = editor.selections.all_adjusted(cx);
-            editor.change_selections(None, cx, |s| s.select(adjusted));
-            copy_selections_content(editor, true, cx);
         });
         vim.switch_mode(Mode::Normal, cx);
     });
@@ -205,7 +164,9 @@ mod test {
     #[gpui::test]
     async fn test_enter_visual_mode(cx: &mut gpui::TestAppContext) {
         let cx = VimTestContext::new(cx, true).await;
-        let mut cx = cx.binding(["v", "w", "j"]).mode_after(Mode::Visual);
+        let mut cx = cx
+            .binding(["v", "w", "j"])
+            .mode_after(Mode::Visual { line: false });
         cx.assert(
             indoc! {"
                 The |quick brown
@@ -236,7 +197,9 @@ mod test {
                 fox jumps [over
                 }the lazy dog"},
         );
-        let mut cx = cx.binding(["v", "b", "k"]).mode_after(Mode::Visual);
+        let mut cx = cx
+            .binding(["v", "b", "k"])
+            .mode_after(Mode::Visual { line: false });
         cx.assert(
             indoc! {"
                 The |quick brown
@@ -576,7 +539,7 @@ mod test {
         );
         cx.assert_clipboard_content(Some(indoc! {"
             quick brown
-            fox jumps ov"}));
+            fox jumps o"}));
         cx.assert(
             indoc! {"
                 The quick brown
@@ -608,7 +571,7 @@ mod test {
                 fox jumps over
                 the lazy dog"},
             indoc! {"
-                The |quick brown
+                |The quick brown
                 fox jumps over
                 the lazy dog"},
         );
@@ -620,8 +583,8 @@ mod test {
                 the |lazy dog"},
             indoc! {"
                 The quick brown
-                fox jumps over
+                |fox jumps over
-                the |lazy dog"},
+                the lazy dog"},
         );
         cx.assert_clipboard_content(Some(indoc! {"
             fox jumps over
@@ -632,8 +595,8 @@ mod test {
                 fox jumps |over
                 the lazy dog"},
             indoc! {"
-                The quick brown
+                The |quick brown
-                fox jumps |over
+                fox jumps over
                 the lazy dog"},
         );
         cx.assert_clipboard_content(Some(indoc! {"