pixel columns (#3052)

@ForLoveOfCats and I found a few speedups that make this acceptably fast
(able to update ~10k selections in <100ms), so the remaining work here
is to fix the tests, and then ship!

Release notes:
- Updated up/down to work based on pixel positions
([#1966](https://github.com/zed-industries/community/issues/1966))
([#759](https://github.com/zed-industries/community/issues/759))
- vim: Fixed off-by-one in visual block mode
([2123](https://github.com/zed-industries/community/issues/2123))

crates/vim/src/motion.rs

@@ -1,9 +1,7 @@
-use std::cmp;
-
 use editor::{
     char_kind,
     display_map::{DisplaySnapshot, FoldPoint, ToDisplayPoint},
-    movement::{self, find_boundary, find_preceding_boundary, FindRange},
+    movement::{self, find_boundary, find_preceding_boundary, FindRange, TextLayoutDetails},
     Bias, CharKind, DisplayPoint, ToOffset,
 };
 use gpui::{actions, impl_actions, AppContext, WindowContext};
@@ -361,6 +359,7 @@ impl Motion {
         point: DisplayPoint,
         goal: SelectionGoal,
         maybe_times: Option<usize>,
+        text_layout_details: &TextLayoutDetails,
     ) -> Option<(DisplayPoint, SelectionGoal)> {
         let times = maybe_times.unwrap_or(1);
         use Motion::*;
@@ -370,16 +369,16 @@ impl Motion {
             Backspace => (backspace(map, point, times), SelectionGoal::None),
             Down {
                 display_lines: false,
-            } => down(map, point, goal, times),
+            } => up_down_buffer_rows(map, point, goal, times as isize, &text_layout_details),
             Down {
                 display_lines: true,
-            } => down_display(map, point, goal, times),
+            } => down_display(map, point, goal, times, &text_layout_details),
             Up {
                 display_lines: false,
-            } => up(map, point, goal, times),
+            } => up_down_buffer_rows(map, point, goal, 0 - times as isize, &text_layout_details),
             Up {
                 display_lines: true,
-            } => up_display(map, point, goal, times),
+            } => up_display(map, point, goal, times, &text_layout_details),
             Right => (right(map, point, times), SelectionGoal::None),
             NextWordStart { ignore_punctuation } => (
                 next_word_start(map, point, *ignore_punctuation, times),
@@ -442,10 +441,15 @@ impl Motion {
         selection: &mut Selection<DisplayPoint>,
         times: Option<usize>,
         expand_to_surrounding_newline: bool,
+        text_layout_details: &TextLayoutDetails,
     ) -> bool {
-        if let Some((new_head, goal)) =
-            self.move_point(map, selection.head(), selection.goal, times)
-        {
+        if let Some((new_head, goal)) = self.move_point(
+            map,
+            selection.head(),
+            selection.goal,
+            times,
+            &text_layout_details,
+        ) {
             selection.set_head(new_head, goal);
 
             if self.linewise() {
@@ -530,35 +534,85 @@ fn backspace(map: &DisplaySnapshot, mut point: DisplayPoint, times: usize) -> Di
     point
 }
 
-fn down(
+pub(crate) fn start_of_relative_buffer_row(
+    map: &DisplaySnapshot,
+    point: DisplayPoint,
+    times: isize,
+) -> DisplayPoint {
+    let start = map.display_point_to_fold_point(point, Bias::Left);
+    let target = start.row() as isize + times;
+    let new_row = (target.max(0) as u32).min(map.fold_snapshot.max_point().row());
+
+    map.clip_point(
+        map.fold_point_to_display_point(
+            map.fold_snapshot
+                .clip_point(FoldPoint::new(new_row, 0), Bias::Right),
+        ),
+        Bias::Right,
+    )
+}
+
+fn up_down_buffer_rows(
     map: &DisplaySnapshot,
     point: DisplayPoint,
     mut goal: SelectionGoal,
-    times: usize,
+    times: isize,
+    text_layout_details: &TextLayoutDetails,
 ) -> (DisplayPoint, SelectionGoal) {
     let start = map.display_point_to_fold_point(point, Bias::Left);
+    let begin_folded_line = map.fold_point_to_display_point(
+        map.fold_snapshot
+            .clip_point(FoldPoint::new(start.row(), 0), Bias::Left),
+    );
+    let select_nth_wrapped_row = point.row() - begin_folded_line.row();
 
-    let goal_column = match goal {
-        SelectionGoal::Column(column) => column,
-        SelectionGoal::ColumnRange { end, .. } => end,
+    let (goal_wrap, goal_x) = match goal {
+        SelectionGoal::WrappedHorizontalPosition((row, x)) => (row, x),
+        SelectionGoal::HorizontalRange { end, .. } => (select_nth_wrapped_row, end),
+        SelectionGoal::HorizontalPosition(x) => (select_nth_wrapped_row, x),
         _ => {
-            goal = SelectionGoal::Column(start.column());
-            start.column()
+            let x = map.x_for_point(point, text_layout_details);
+            goal = SelectionGoal::WrappedHorizontalPosition((select_nth_wrapped_row, x));
+            (select_nth_wrapped_row, x)
         }
     };
 
-    let new_row = cmp::min(
-        start.row() + times as u32,
-        map.fold_snapshot.max_point().row(),
-    );
-    let new_col = cmp::min(goal_column, map.fold_snapshot.line_len(new_row));
-    let point = map.fold_point_to_display_point(
+    let target = start.row() as isize + times;
+    let new_row = (target.max(0) as u32).min(map.fold_snapshot.max_point().row());
+
+    let mut begin_folded_line = map.fold_point_to_display_point(
         map.fold_snapshot
-            .clip_point(FoldPoint::new(new_row, new_col), Bias::Left),
+            .clip_point(FoldPoint::new(new_row, 0), Bias::Left),
     );
 
-    // clip twice to "clip at end of line"
-    (map.clip_point(point, Bias::Left), goal)
+    let mut i = 0;
+    while i < goal_wrap && begin_folded_line.row() < map.max_point().row() {
+        let next_folded_line = DisplayPoint::new(begin_folded_line.row() + 1, 0);
+        if map
+            .display_point_to_fold_point(next_folded_line, Bias::Right)
+            .row()
+            == new_row
+        {
+            i += 1;
+            begin_folded_line = next_folded_line;
+        } else {
+            break;
+        }
+    }
+
+    let new_col = if i == goal_wrap {
+        map.column_for_x(begin_folded_line.row(), goal_x, text_layout_details)
+    } else {
+        map.line_len(begin_folded_line.row())
+    };
+
+    (
+        map.clip_point(
+            DisplayPoint::new(begin_folded_line.row(), new_col),
+            Bias::Left,
+        ),
+        goal,
+    )
 }
 
 fn down_display(
@@ -566,49 +620,24 @@ fn down_display(
     mut point: DisplayPoint,
     mut goal: SelectionGoal,
     times: usize,
+    text_layout_details: &TextLayoutDetails,
 ) -> (DisplayPoint, SelectionGoal) {
     for _ in 0..times {
-        (point, goal) = movement::down(map, point, goal, true);
+        (point, goal) = movement::down(map, point, goal, true, text_layout_details);
     }
 
     (point, goal)
 }
 
-pub(crate) fn up(
-    map: &DisplaySnapshot,
-    point: DisplayPoint,
-    mut goal: SelectionGoal,
-    times: usize,
-) -> (DisplayPoint, SelectionGoal) {
-    let start = map.display_point_to_fold_point(point, Bias::Left);
-
-    let goal_column = match goal {
-        SelectionGoal::Column(column) => column,
-        SelectionGoal::ColumnRange { end, .. } => end,
-        _ => {
-            goal = SelectionGoal::Column(start.column());
-            start.column()
-        }
-    };
-
-    let new_row = start.row().saturating_sub(times as u32);
-    let new_col = cmp::min(goal_column, map.fold_snapshot.line_len(new_row));
-    let point = map.fold_point_to_display_point(
-        map.fold_snapshot
-            .clip_point(FoldPoint::new(new_row, new_col), Bias::Left),
-    );
-
-    (map.clip_point(point, Bias::Left), goal)
-}
-
 fn up_display(
     map: &DisplaySnapshot,
     mut point: DisplayPoint,
     mut goal: SelectionGoal,
     times: usize,
+    text_layout_details: &TextLayoutDetails,
 ) -> (DisplayPoint, SelectionGoal) {
     for _ in 0..times {
-        (point, goal) = movement::up(map, point, goal, true);
+        (point, goal) = movement::up(map, point, goal, true, &text_layout_details);
     }
 
     (point, goal)
@@ -707,7 +736,7 @@ fn previous_word_start(
     point
 }
 
-fn first_non_whitespace(
+pub(crate) fn first_non_whitespace(
     map: &DisplaySnapshot,
     display_lines: bool,
     from: DisplayPoint,
@@ -886,13 +915,17 @@ fn find_backward(
 }
 
 fn next_line_start(map: &DisplaySnapshot, point: DisplayPoint, times: usize) -> DisplayPoint {
-    let correct_line = down(map, point, SelectionGoal::None, times).0;
+    let correct_line = start_of_relative_buffer_row(map, point, times as isize);
     first_non_whitespace(map, false, correct_line)
 }
 
-fn next_line_end(map: &DisplaySnapshot, mut point: DisplayPoint, times: usize) -> DisplayPoint {
+pub(crate) fn next_line_end(
+    map: &DisplaySnapshot,
+    mut point: DisplayPoint,
+    times: usize,
+) -> DisplayPoint {
     if times > 1 {
-        point = down(map, point, SelectionGoal::None, times - 1).0;
+        point = start_of_relative_buffer_row(map, point, times as isize - 1);
     }
     end_of_line(map, false, point)
 }

crates/vim/src/normal.rs

@@ -12,7 +12,7 @@ mod yank;
 use std::sync::Arc;
 
 use crate::{
-    motion::{self, Motion},
+    motion::{self, first_non_whitespace, next_line_end, right, Motion},
     object::Object,
     state::{Mode, Operator},
     Vim,
@@ -179,10 +179,11 @@ pub(crate) fn move_cursor(
     cx: &mut WindowContext,
 ) {
     vim.update_active_editor(cx, |editor, cx| {
+        let text_layout_details = editor.text_layout_details(cx);
         editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
             s.move_cursors_with(|map, cursor, goal| {
                 motion
-                    .move_point(map, cursor, goal, times)
+                    .move_point(map, cursor, goal, times, &text_layout_details)
                     .unwrap_or((cursor, goal))
             })
         })
@@ -195,9 +196,7 @@ fn insert_after(_: &mut Workspace, _: &InsertAfter, cx: &mut ViewContext<Workspa
         vim.switch_mode(Mode::Insert, false, cx);
         vim.update_active_editor(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
-                s.maybe_move_cursors_with(|map, cursor, goal| {
-                    Motion::Right.move_point(map, cursor, goal, None)
-                });
+                s.move_cursors_with(|map, cursor, _| (right(map, cursor, 1), SelectionGoal::None));
             });
         });
     });
@@ -220,11 +219,11 @@ fn insert_first_non_whitespace(
         vim.switch_mode(Mode::Insert, false, cx);
         vim.update_active_editor(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
-                s.maybe_move_cursors_with(|map, cursor, goal| {
-                    Motion::FirstNonWhitespace {
-                        display_lines: false,
-                    }
-                    .move_point(map, cursor, goal, None)
+                s.move_cursors_with(|map, cursor, _| {
+                    (
+                        first_non_whitespace(map, false, cursor),
+                        SelectionGoal::None,
+                    )
                 });
             });
         });
@@ -237,8 +236,8 @@ fn insert_end_of_line(_: &mut Workspace, _: &InsertEndOfLine, cx: &mut ViewConte
         vim.switch_mode(Mode::Insert, false, cx);
         vim.update_active_editor(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
-                s.maybe_move_cursors_with(|map, cursor, goal| {
-                    Motion::CurrentLine.move_point(map, cursor, goal, None)
+                s.move_cursors_with(|map, cursor, _| {
+                    (next_line_end(map, cursor, 1), SelectionGoal::None)
                 });
             });
         });
@@ -268,7 +267,7 @@ fn insert_line_above(_: &mut Workspace, _: &InsertLineAbove, cx: &mut ViewContex
                 editor.edit_with_autoindent(edits, cx);
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                     s.move_cursors_with(|map, cursor, _| {
-                        let previous_line = motion::up(map, cursor, SelectionGoal::None, 1).0;
+                        let previous_line = motion::start_of_relative_buffer_row(map, cursor, -1);
                         let insert_point = motion::end_of_line(map, false, previous_line);
                         (insert_point, SelectionGoal::None)
                     });
@@ -283,6 +282,7 @@ fn insert_line_below(_: &mut Workspace, _: &InsertLineBelow, cx: &mut ViewContex
         vim.start_recording(cx);
         vim.switch_mode(Mode::Insert, false, cx);
         vim.update_active_editor(cx, |editor, cx| {
+            let text_layout_details = editor.text_layout_details(cx);
             editor.transact(cx, |editor, cx| {
                 let (map, old_selections) = editor.selections.all_display(cx);
 
@@ -301,7 +301,13 @@ fn insert_line_below(_: &mut Workspace, _: &InsertLineBelow, cx: &mut ViewContex
                 });
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                     s.maybe_move_cursors_with(|map, cursor, goal| {
-                        Motion::CurrentLine.move_point(map, cursor, goal, None)
+                        Motion::CurrentLine.move_point(
+                            map,
+                            cursor,
+                            goal,
+                            None,
+                            &text_layout_details,
+                        )
                     });
                 });
                 editor.edit_with_autoindent(edits, cx);
@@ -399,12 +405,26 @@ mod test {
 
     #[gpui::test]
     async fn test_j(cx: &mut gpui::TestAppContext) {
-        let mut cx = NeovimBackedTestContext::new(cx).await.binding(["j"]);
-        cx.assert_all(indoc! {"
-            ˇThe qˇuick broˇwn
-            ˇfox jumps"
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state(indoc! {"
+                    aaˇaa
+                    😃😃"
         })
         .await;
+        cx.simulate_shared_keystrokes(["j"]).await;
+        cx.assert_shared_state(indoc! {"
+                    aaaa
+                    😃ˇ😃"
+        })
+        .await;
+
+        for marked_position in cx.each_marked_position(indoc! {"
+                    ˇThe qˇuick broˇwn
+                    ˇfox jumps"
+        }) {
+            cx.assert_neovim_compatible(&marked_position, ["j"]).await;
+        }
     }
 
     #[gpui::test]

crates/vim/src/normal/change.rs

@@ -2,7 +2,7 @@ use crate::{motion::Motion, object::Object, state::Mode, utils::copy_selections_
 use editor::{
     char_kind,
     display_map::DisplaySnapshot,
-    movement::{self, FindRange},
+    movement::{self, FindRange, TextLayoutDetails},
     scroll::autoscroll::Autoscroll,
     CharKind, DisplayPoint,
 };
@@ -20,6 +20,7 @@ pub fn change_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &m
             | Motion::StartOfLine { .. }
     );
     vim.update_active_editor(cx, |editor, cx| {
+        let text_layout_details = editor.text_layout_details(cx);
         editor.transact(cx, |editor, cx| {
             // We are swapping to insert mode anyway. Just set the line end clipping behavior now
             editor.set_clip_at_line_ends(false, cx);
@@ -27,9 +28,15 @@ pub fn change_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &m
                 s.move_with(|map, selection| {
                     motion_succeeded |= if let Motion::NextWordStart { ignore_punctuation } = motion
                     {
-                        expand_changed_word_selection(map, selection, times, ignore_punctuation)
+                        expand_changed_word_selection(
+                            map,
+                            selection,
+                            times,
+                            ignore_punctuation,
+                            &text_layout_details,
+                        )
                     } else {
-                        motion.expand_selection(map, selection, times, false)
+                        motion.expand_selection(map, selection, times, false, &text_layout_details)
                     };
                 });
             });
@@ -81,6 +88,7 @@ fn expand_changed_word_selection(
     selection: &mut Selection<DisplayPoint>,
     times: Option<usize>,
     ignore_punctuation: bool,
+    text_layout_details: &TextLayoutDetails,
 ) -> bool {
     if times.is_none() || times.unwrap() == 1 {
         let scope = map
@@ -103,11 +111,22 @@ fn expand_changed_word_selection(
                 });
             true
         } else {
-            Motion::NextWordStart { ignore_punctuation }
-                .expand_selection(map, selection, None, false)
+            Motion::NextWordStart { ignore_punctuation }.expand_selection(
+                map,
+                selection,
+                None,
+                false,
+                &text_layout_details,
+            )
         }
     } else {
-        Motion::NextWordStart { ignore_punctuation }.expand_selection(map, selection, times, false)
+        Motion::NextWordStart { ignore_punctuation }.expand_selection(
+            map,
+            selection,
+            times,
+            false,
+            &text_layout_details,
+        )
     }
 }
 

crates/vim/src/normal/delete.rs

@@ -7,6 +7,7 @@ use language::Point;
 pub fn delete_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     vim.stop_recording();
     vim.update_active_editor(cx, |editor, cx| {
+        let text_layout_details = editor.text_layout_details(cx);
         editor.transact(cx, |editor, cx| {
             editor.set_clip_at_line_ends(false, cx);
             let mut original_columns: HashMap<_, _> = Default::default();
@@ -14,7 +15,7 @@ pub fn delete_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &m
                 s.move_with(|map, selection| {
                     let original_head = selection.head();
                     original_columns.insert(selection.id, original_head.column());
-                    motion.expand_selection(map, selection, times, true);
+                    motion.expand_selection(map, selection, times, true, &text_layout_details);
 
                     // Motion::NextWordStart on an empty line should delete it.
                     if let Motion::NextWordStart {

crates/vim/src/normal/increment.rs

@@ -255,8 +255,18 @@ mod test {
             4
             5"})
             .await;
-        cx.simulate_shared_keystrokes(["shift-g", "ctrl-v", "g", "g", "g", "ctrl-x"])
+
+        cx.simulate_shared_keystrokes(["shift-g", "ctrl-v", "g", "g"])
             .await;
+        cx.assert_shared_state(indoc! {"
+            «1ˇ»
+            «2ˇ»
+            «3ˇ»  2
+            «4ˇ»
+            «5ˇ»"})
+            .await;
+
+        cx.simulate_shared_keystrokes(["g", "ctrl-x"]).await;
         cx.assert_shared_state(indoc! {"
             ˇ0
             0

crates/vim/src/normal/paste.rs

@@ -30,6 +30,7 @@ fn paste(_: &mut Workspace, action: &Paste, cx: &mut ViewContext<Workspace>) {
     Vim::update(cx, |vim, cx| {
         vim.record_current_action(cx);
         vim.update_active_editor(cx, |editor, cx| {
+            let text_layout_details = editor.text_layout_details(cx);
             editor.transact(cx, |editor, cx| {
                 editor.set_clip_at_line_ends(false, cx);
 
@@ -168,8 +169,14 @@ fn paste(_: &mut Workspace, action: &Paste, cx: &mut ViewContext<Workspace>) {
                             let mut cursor = anchor.to_display_point(map);
                             if *line_mode {
                                 if !before {
-                                    cursor =
-                                        movement::down(map, cursor, SelectionGoal::None, false).0;
+                                    cursor = movement::down(
+                                        map,
+                                        cursor,
+                                        SelectionGoal::None,
+                                        false,
+                                        &text_layout_details,
+                                    )
+                                    .0;
                                 }
                                 cursor = movement::indented_line_beginning(map, cursor, true);
                             } else if !is_multiline {

crates/vim/src/normal/substitute.rs

@@ -32,10 +32,17 @@ pub fn substitute(vim: &mut Vim, count: Option<usize>, line_mode: bool, cx: &mut
     vim.update_active_editor(cx, |editor, cx| {
         editor.set_clip_at_line_ends(false, cx);
         editor.transact(cx, |editor, cx| {
+            let text_layout_details = editor.text_layout_details(cx);
             editor.change_selections(None, cx, |s| {
                 s.move_with(|map, selection| {
                     if selection.start == selection.end {
-                        Motion::Right.expand_selection(map, selection, count, true);
+                        Motion::Right.expand_selection(
+                            map,
+                            selection,
+                            count,
+                            true,
+                            &text_layout_details,
+                        );
                     }
                     if line_mode {
                         // in Visual mode when the selection contains the newline at the end
@@ -43,7 +50,13 @@ pub fn substitute(vim: &mut Vim, count: Option<usize>, line_mode: bool, cx: &mut
                         if !selection.is_empty() && selection.end.column() == 0 {
                             selection.end = movement::left(map, selection.end);
                         }
-                        Motion::CurrentLine.expand_selection(map, selection, None, false);
+                        Motion::CurrentLine.expand_selection(
+                            map,
+                            selection,
+                            None,
+                            false,
+                            &text_layout_details,
+                        );
                         if let Some((point, _)) = (Motion::FirstNonWhitespace {
                             display_lines: false,
                         })
@@ -52,6 +65,7 @@ pub fn substitute(vim: &mut Vim, count: Option<usize>, line_mode: bool, cx: &mut
                             selection.start,
                             selection.goal,
                             None,
+                            &text_layout_details,
                         ) {
                             selection.start = point;
                         }

crates/vim/src/normal/yank.rs

@@ -4,6 +4,7 @@ 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| {
+        let text_layout_details = editor.text_layout_details(cx);
         editor.transact(cx, |editor, cx| {
             editor.set_clip_at_line_ends(false, cx);
             let mut original_positions: HashMap<_, _> = Default::default();
@@ -11,7 +12,7 @@ pub fn yank_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut
                 s.move_with(|map, selection| {
                     let original_position = (selection.head(), selection.goal);
                     original_positions.insert(selection.id, original_position);
-                    motion.expand_selection(map, selection, times, true);
+                    motion.expand_selection(map, selection, times, true, &text_layout_details);
                 });
             });
             copy_selections_content(editor, motion.linewise(), cx);

crates/vim/src/test.rs

@@ -653,6 +653,63 @@ async fn test_selection_goal(cx: &mut gpui::TestAppContext) {
         .await;
 }
 
+#[gpui::test]
+async fn test_wrapped_motions(cx: &mut gpui::TestAppContext) {
+    let mut cx = NeovimBackedTestContext::new(cx).await;
+
+    cx.set_shared_wrap(12).await;
+
+    cx.set_shared_state(indoc! {"
+                aaˇaa
+                😃😃"
+    })
+    .await;
+    cx.simulate_shared_keystrokes(["j"]).await;
+    cx.assert_shared_state(indoc! {"
+                aaaa
+                😃ˇ😃"
+    })
+    .await;
+
+    cx.set_shared_state(indoc! {"
+                123456789012aaˇaa
+                123456789012😃😃"
+    })
+    .await;
+    cx.simulate_shared_keystrokes(["j"]).await;
+    cx.assert_shared_state(indoc! {"
+        123456789012aaaa
+        123456789012😃ˇ😃"
+    })
+    .await;
+
+    cx.set_shared_state(indoc! {"
+                123456789012aaˇaa
+                123456789012😃😃"
+    })
+    .await;
+    cx.simulate_shared_keystrokes(["j"]).await;
+    cx.assert_shared_state(indoc! {"
+        123456789012aaaa
+        123456789012😃ˇ😃"
+    })
+    .await;
+
+    cx.set_shared_state(indoc! {"
+        123456789012aaaaˇaaaaaaaa123456789012
+        wow
+        123456789012😃😃😃😃😃😃123456789012"
+    })
+    .await;
+    cx.simulate_shared_keystrokes(["j", "j"]).await;
+    cx.assert_shared_state(indoc! {"
+        123456789012aaaaaaaaaaaa123456789012
+        wow
+        123456789012😃😃ˇ😃😃😃😃123456789012"
+    })
+    .await;
+}
+
 #[gpui::test]
 async fn test_paragraphs_dont_wrap(cx: &mut gpui::TestAppContext) {
     let mut cx = NeovimBackedTestContext::new(cx).await;

crates/vim/src/vim.rs

@@ -590,7 +590,7 @@ impl Setting for VimModeSetting {
 fn local_selections_changed(newest: Selection<usize>, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
         if vim.enabled && vim.state().mode == Mode::Normal && !newest.is_empty() {
-            if matches!(newest.goal, SelectionGoal::ColumnRange { .. }) {
+            if matches!(newest.goal, SelectionGoal::HorizontalRange { .. }) {
                 vim.switch_mode(Mode::VisualBlock, false, cx);
             } else {
                 vim.switch_mode(Mode::Visual, false, cx)

crates/vim/src/visual.rs

@@ -57,6 +57,7 @@ pub fn init(cx: &mut AppContext) {
 pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
+            let text_layout_details = editor.text_layout_details(cx);
             if vim.state().mode == Mode::VisualBlock
                 && !matches!(
                     motion,
@@ -67,7 +68,7 @@ pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContex
             {
                 let is_up_or_down = matches!(motion, Motion::Up { .. } | Motion::Down { .. });
                 visual_block_motion(is_up_or_down, editor, cx, |map, point, goal| {
-                    motion.move_point(map, point, goal, times)
+                    motion.move_point(map, point, goal, times, &text_layout_details)
                 })
             } else {
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
@@ -89,9 +90,13 @@ pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContex
                             current_head = movement::left(map, selection.end)
                         }
 
-                        let Some((new_head, goal)) =
-                            motion.move_point(map, current_head, selection.goal, times)
-                        else {
+                        let Some((new_head, goal)) = motion.move_point(
+                            map,
+                            current_head,
+                            selection.goal,
+                            times,
+                            &text_layout_details,
+                        ) else {
                             return;
                         };
 
@@ -135,19 +140,23 @@ pub fn visual_block_motion(
         SelectionGoal,
     ) -> Option<(DisplayPoint, SelectionGoal)>,
 ) {
+    let text_layout_details = editor.text_layout_details(cx);
     editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
         let map = &s.display_map();
         let mut head = s.newest_anchor().head().to_display_point(map);
         let mut tail = s.oldest_anchor().tail().to_display_point(map);
 
-        let (start, end) = match s.newest_anchor().goal {
-            SelectionGoal::ColumnRange { start, end } if preserve_goal => (start, end),
-            SelectionGoal::Column(start) if preserve_goal => (start, start + 1),
-            _ => (tail.column(), head.column()),
-        };
-        let goal = SelectionGoal::ColumnRange { start, end };
+        let mut head_x = map.x_for_point(head, &text_layout_details);
+        let mut tail_x = map.x_for_point(tail, &text_layout_details);
 
-        let was_reversed = tail.column() > head.column();
+        let (start, end) = match s.newest_anchor().goal {
+            SelectionGoal::HorizontalRange { start, end } if preserve_goal => (start, end),
+            SelectionGoal::HorizontalPosition(start) if preserve_goal => (start, start),
+            _ => (tail_x, head_x),
+        };
+        let mut goal = SelectionGoal::HorizontalRange { start, end };
+
+        let was_reversed = tail_x > head_x;
         if !was_reversed && !preserve_goal {
             head = movement::saturating_left(map, head);
         }
@@ -156,32 +165,56 @@ pub fn visual_block_motion(
             return;
         };
         head = new_head;
+        head_x = map.x_for_point(head, &text_layout_details);
 
-        let is_reversed = tail.column() > head.column();
+        let is_reversed = tail_x > head_x;
         if was_reversed && !is_reversed {
-            tail = movement::left(map, tail)
+            tail = movement::saturating_left(map, tail);
+            tail_x = map.x_for_point(tail, &text_layout_details);
         } else if !was_reversed && is_reversed {
-            tail = movement::right(map, tail)
+            tail = movement::saturating_right(map, tail);
+            tail_x = map.x_for_point(tail, &text_layout_details);
         }
         if !is_reversed && !preserve_goal {
-            head = movement::saturating_right(map, head)
+            head = movement::saturating_right(map, head);
+            head_x = map.x_for_point(head, &text_layout_details);
         }
 
-        let columns = if is_reversed {
-            head.column()..tail.column()
-        } else if head.column() == tail.column() {
-            head.column()..(head.column() + 1)
+        let positions = if is_reversed {
+            head_x..tail_x
         } else {
-            tail.column()..head.column()
+            tail_x..head_x
         };
 
+        if !preserve_goal {
+            goal = SelectionGoal::HorizontalRange {
+                start: positions.start,
+                end: positions.end,
+            };
+        }
+
         let mut selections = Vec::new();
         let mut row = tail.row();
 
         loop {
-            let start = map.clip_point(DisplayPoint::new(row, columns.start), Bias::Left);
-            let end = map.clip_point(DisplayPoint::new(row, columns.end), Bias::Left);
-            if columns.start <= map.line_len(row) {
+            let layed_out_line = map.lay_out_line_for_row(row, &text_layout_details);
+            let start = DisplayPoint::new(
+                row,
+                layed_out_line.closest_index_for_x(positions.start) as u32,
+            );
+            let mut end = DisplayPoint::new(
+                row,
+                layed_out_line.closest_index_for_x(positions.end) as u32,
+            );
+            if end <= start {
+                if start.column() == map.line_len(start.row()) {
+                    end = start;
+                } else {
+                    end = movement::saturating_right(map, start);
+                }
+            }
+
+            if positions.start <= layed_out_line.width() {
                 let selection = Selection {
                     id: s.new_selection_id(),
                     start: start.to_point(map),
@@ -888,6 +921,28 @@ mod test {
         .await;
     }
 
+    #[gpui::test]
+    async fn test_visual_block_issue_2123(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(["ctrl-v", "right", "down"])
+            .await;
+        cx.assert_shared_state(indoc! {
+            "The «quˇ»ick brown
+            fox «juˇ»mps over
+            the lazy dog
+            "
+        })
+        .await;
+    }
+
     #[gpui::test]
     async fn test_visual_block_insert(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;

crates/vim/test_data/test_increment_steps.json

@@ -9,6 +9,7 @@
 {"Key":"ctrl-v"}
 {"Key":"g"}
 {"Key":"g"}
+{"Get":{"state":"«1ˇ»\n«2ˇ»\n«3ˇ»  2\n«4ˇ»\n«5ˇ»","mode":"VisualBlock"}}
 {"Key":"g"}
 {"Key":"ctrl-x"}
 {"Get":{"state":"ˇ0\n0\n0  2\n0\n0","mode":"Normal"}}

crates/vim/test_data/test_j.json

@@ -1,3 +1,6 @@
+{"Put":{"state":"aaˇaa\n😃😃"}}
+{"Key":"j"}
+{"Get":{"state":"aaaa\n😃ˇ😃","mode":"Normal"}}
 {"Put":{"state":"ˇThe quick brown\nfox jumps"}}
 {"Key":"j"}
 {"Get":{"state":"The quick brown\nˇfox jumps","mode":"Normal"}}

crates/vim/test_data/test_visual_block_issue_2123.json

@@ -0,0 +1,5 @@
+{"Put":{"state":"The ˇquick brown\nfox jumps over\nthe lazy dog\n"}}
+{"Key":"ctrl-v"}
+{"Key":"right"}
+{"Key":"down"}
+{"Get":{"state":"The «quˇ»ick brown\nfox «juˇ»mps over\nthe lazy dog\n","mode":"VisualBlock"}}

crates/vim/test_data/test_wrapped_motions.json

@@ -0,0 +1,15 @@
+{"SetOption":{"value":"wrap"}}
+{"SetOption":{"value":"columns=12"}}
+{"Put":{"state":"aaˇaa\n😃😃"}}
+{"Key":"j"}
+{"Get":{"state":"aaaa\n😃ˇ😃","mode":"Normal"}}
+{"Put":{"state":"123456789012aaˇaa\n123456789012😃😃"}}
+{"Key":"j"}
+{"Get":{"state":"123456789012aaaa\n123456789012😃ˇ😃","mode":"Normal"}}
+{"Put":{"state":"123456789012aaˇaa\n123456789012😃😃"}}
+{"Key":"j"}
+{"Get":{"state":"123456789012aaaa\n123456789012😃ˇ😃","mode":"Normal"}}
+{"Put":{"state":"123456789012aaaaˇaaaaaaaa123456789012\nwow\n123456789012😃😃😃😃😃😃123456789012"}}
+{"Key":"j"}
+{"Key":"j"}
+{"Get":{"state":"123456789012aaaaaaaaaaaa123456789012\nwow\n123456789012😃😃ˇ😃😃😃😃123456789012","mode":"Normal"}}