Make vim visual block work better

Conrad Irwin created 2 years ago

Change summary

crates/editor/src/display_map.rs                       | 11 
crates/editor/src/element.rs                           |  2 
crates/editor/src/movement.rs                          |  2 
crates/editor/src/selections_collection.rs             |  6 
crates/text/src/selection.rs                           |  1 
crates/vim/src/motion.rs                               |  1 
crates/vim/src/test.rs                                 |  1 
crates/vim/src/visual.rs                               | 73 ++++++++---
crates/vim/test_data/test_visual_block_issue_2123.json |  5 
crates/vim/test_data/test_wrapped_motions.json         | 15 ++
10 files changed, 85 insertions(+), 32 deletions(-)

Detailed changes

crates/editor/src/display_map.rs

@@ -509,11 +509,12 @@ impl DisplaySnapshot {
     pub fn highlighted_chunks<'a>(
         &'a self,
         display_rows: Range<u32>,
+        language_aware: bool,
         style: &'a EditorStyle,
     ) -> impl Iterator<Item = HighlightedChunk<'a>> {
         self.chunks(
             display_rows,
-            true,
+            language_aware,
             Some(style.theme.hint),
             Some(style.theme.suggestion),
         )
@@ -562,7 +563,7 @@ impl DisplaySnapshot {
         })
     }
 
-    fn layout_line_for_row(
+    pub fn lay_out_line_for_row(
         &self,
         display_row: u32,
         TextLayoutDetails {
@@ -575,7 +576,7 @@ impl DisplaySnapshot {
         let mut line = String::new();
 
         let range = display_row..display_row + 1;
-        for chunk in self.highlighted_chunks(range, editor_style) {
+        for chunk in self.highlighted_chunks(range, false, editor_style) {
             line.push_str(chunk.chunk);
 
             let text_style = if let Some(style) = chunk.style {
@@ -607,7 +608,7 @@ impl DisplaySnapshot {
         display_point: DisplayPoint,
         text_layout_details: &TextLayoutDetails,
     ) -> f32 {
-        let layout_line = self.layout_line_for_row(display_point.row(), text_layout_details);
+        let layout_line = self.lay_out_line_for_row(display_point.row(), text_layout_details);
         layout_line.x_for_index(display_point.column() as usize)
     }
 
@@ -617,7 +618,7 @@ impl DisplaySnapshot {
         x_coordinate: f32,
         text_layout_details: &TextLayoutDetails,
     ) -> u32 {
-        let layout_line = self.layout_line_for_row(display_row, text_layout_details);
+        let layout_line = self.lay_out_line_for_row(display_row, text_layout_details);
         layout_line.closest_index_for_x(x_coordinate) as u32
     }
 

crates/editor/src/element.rs

@@ -1583,7 +1583,7 @@ impl EditorElement {
                 .collect()
         } else {
             let style = &self.style;
-            let chunks = snapshot.highlighted_chunks(rows.clone(), style);
+            let chunks = snapshot.highlighted_chunks(rows.clone(), true, style);
 
             LineWithInvisibles::from_chunks(
                 chunks,

crates/editor/src/movement.rs

@@ -107,7 +107,6 @@ pub fn up_by_rows(
         SelectionGoal::HorizontalPosition(x) => x,
         SelectionGoal::WrappedHorizontalPosition((_, x)) => x,
         SelectionGoal::HorizontalRange { end, .. } => end,
-        SelectionGoal::WrappedHorizontalRange { end: (_, end), .. } => end,
         _ => map.x_for_point(start, text_layout_details),
     };
 
@@ -144,7 +143,6 @@ pub fn down_by_rows(
         SelectionGoal::HorizontalPosition(x) => x,
         SelectionGoal::WrappedHorizontalPosition((_, x)) => x,
         SelectionGoal::HorizontalRange { end, .. } => end,
-        SelectionGoal::WrappedHorizontalRange { end: (_, end), .. } => end,
         _ => map.x_for_point(start, text_layout_details),
     };
 

crates/editor/src/selections_collection.rs

@@ -313,10 +313,12 @@ impl SelectionsCollection {
         let is_empty = positions.start == positions.end;
         let line_len = display_map.line_len(row);
 
-        let start_col = display_map.column_for_x(row, positions.start, text_layout_details);
+        let layed_out_line = display_map.lay_out_line_for_row(row, &text_layout_details);
+
+        let start_col = layed_out_line.closest_index_for_x(positions.start) as u32;
         if start_col < line_len || (is_empty && start_col == line_len) {
             let start = DisplayPoint::new(row, start_col);
-            let end_col = display_map.column_for_x(row, positions.end, text_layout_details);
+            let end_col = layed_out_line.closest_index_for_x(positions.end) as u32;
             let end = DisplayPoint::new(row, end_col);
 
             Some(Selection {

crates/text/src/selection.rs

@@ -8,7 +8,6 @@ pub enum SelectionGoal {
     HorizontalPosition(f32),
     HorizontalRange { start: f32, end: f32 },
     WrappedHorizontalPosition((u32, f32)),
-    WrappedHorizontalRange { start: (u32, f32), end: (u32, f32) },
 }
 
 #[derive(Clone, Debug, PartialEq)]

crates/vim/src/motion.rs

@@ -568,7 +568,6 @@ fn up_down_buffer_rows(
 
     let (goal_wrap, goal_x) = match goal {
         SelectionGoal::WrappedHorizontalPosition((row, x)) => (row, x),
-        SelectionGoal::WrappedHorizontalRange { end: (row, x), .. } => (row, x),
         SelectionGoal::HorizontalRange { end, .. } => (select_nth_wrapped_row, end),
         SelectionGoal::HorizontalPosition(x) => (select_nth_wrapped_row, x),
         _ => {

crates/vim/src/test.rs

@@ -653,6 +653,7 @@ 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;
 

crates/vim/src/visual.rs

@@ -145,16 +145,18 @@ pub fn visual_block_motion(
         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);
+        dbg!(head, tail);
+        dbg!(s.newest_anchor().goal);
 
         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 + 10.0),
+            SelectionGoal::HorizontalPosition(start) if preserve_goal => (start, start),
             _ => (
                 map.x_for_point(tail, &text_layout_details),
                 map.x_for_point(head, &text_layout_details),
             ),
         };
-        let goal = SelectionGoal::HorizontalRange { start, end };
+        let mut goal = SelectionGoal::HorizontalRange { start, end };
 
         let was_reversed = tail.column() > head.column();
         if !was_reversed && !preserve_goal {
@@ -179,35 +181,44 @@ pub fn visual_block_motion(
         let positions = if is_reversed {
             map.x_for_point(head, &text_layout_details)..map.x_for_point(tail, &text_layout_details)
         } else if head.column() == tail.column() {
-            map.x_for_point(head, &text_layout_details)
-                ..map.x_for_point(head, &text_layout_details) + 10.0
+            let head_forward = movement::saturating_right(map, head);
+            map.x_for_point(head, &text_layout_details)..map.x_for_point(head, &text_layout_details)
         } else {
             map.x_for_point(tail, &text_layout_details)..map.x_for_point(head, &text_layout_details)
         };
 
+        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,
-                    map.column_for_x(row, positions.start, &text_layout_details),
-                ),
-                Bias::Left,
+            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 end = map.clip_point(
-                DisplayPoint::new(
-                    row,
-                    map.column_for_x(row, positions.end, &text_layout_details),
-                ),
-                Bias::Left,
+            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
-                <= map.x_for_point(
-                    DisplayPoint::new(row, map.line_len(row)),
-                    &text_layout_details,
-                )
+                <=
+                //map.x_for_point(DisplayPoint::new(row, map.line_len(row)), &text_layout_details)
+                layed_out_line.width()
             {
                 let selection = Selection {
                     id: s.new_selection_id(),
@@ -915,6 +926,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_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"}}