vim: Fix visual block handling of wrapped lines (#39355)

Dino created

These changes fix an issue with vim's visual block mode when soft
wrapping is enabled. In this situation, if one was to move the cursor
either up or down, the selection would be updated to include visual
(wrapped) rows, instead of only the buffer rows. For example, take the
following contents:

```
1 | And here's a very long line that is wrapping
    at this exact point.
2 | And another very long line that is will also
    wrap at this exact point.
```

If one was to place the cursor at the start of the first line, character
`A`, trigger visual block mode with `ctrl-v` and then move down one line
with `j`, the selection would end up as (with [X] representing the
selected characters):

```
1 | [A]nd here's a very long line that is wrapping
    [a]t this exact point.
2 | [A]nd another very long line that is will also
    wrap at this exact point.
```

Instead of the expected:

```
1 | [A]nd here's a very long line that is wrapping
    at this exact point.
2 | [A]nd another very long line that is will also
    wrap at this exact point.
```

With the changes in this commit, `Vim.visual_block_motion` will now
leverage buffer rows in order to navigate to the next or previous row.

Release Notes:

- Fixed handling of soft wrapped lines in vim's visual block mode

Change summary

crates/vim/src/motion.rs                                       |  5 
crates/vim/src/visual.rs                                       | 73 +++
crates/vim/test_data/test_visual_block_wrapping_selection.json | 16 
3 files changed, 88 insertions(+), 6 deletions(-)

Detailed changes

crates/vim/src/motion.rs 🔗

@@ -1525,6 +1525,11 @@ fn wrapping_right_single(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayP
     }
 }
 
+/// Given a point, returns the start of the buffer row that is a given number of
+/// buffer rows away from the current position.
+///
+/// This moves by buffer rows instead of display rows, a distinction that is
+/// important when soft wrapping is enabled.
 pub(crate) fn start_of_relative_buffer_row(
     map: &DisplaySnapshot,
     point: DisplayPoint,

crates/vim/src/visual.rs 🔗

@@ -15,7 +15,10 @@ use workspace::searchable::Direction;
 
 use crate::{
     Vim,
-    motion::{Motion, MotionKind, first_non_whitespace, next_line_end, start_of_line},
+    motion::{
+        Motion, MotionKind, first_non_whitespace, next_line_end, start_of_line,
+        start_of_relative_buffer_row,
+    },
     object::Object,
     state::{Mark, Mode, Operator},
 };
@@ -399,12 +402,13 @@ impl Vim {
                 if row == head.row() {
                     break;
                 }
-                if tail.row() > head.row() {
-                    row.0 -= 1
-                } else {
-                    row.0 += 1
-                }
+
+                // Move to the next or previous buffer row, ensuring that
+                // wrapped lines are handled correctly.
+                let direction = if tail.row() > head.row() { -1 } else { 1 };
+                row = start_of_relative_buffer_row(map, DisplayPoint::new(row, 0), direction).row();
             }
+
             s.select(selections);
         })
     }
@@ -1533,6 +1537,63 @@ mod test {
         });
     }
 
+    #[gpui::test]
+    async fn test_visual_block_wrapping_selection(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        // Ensure that the editor is wrapping lines at 12 columns so that each
+        // of the lines ends up being wrapped.
+        cx.set_shared_wrap(12).await;
+        cx.set_shared_state(indoc! {
+            "ˇ12345678901234567890
+            12345678901234567890
+            12345678901234567890
+            "
+        })
+        .await;
+        cx.simulate_shared_keystrokes("ctrl-v j").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "«1ˇ»2345678901234567890
+            «1ˇ»2345678901234567890
+            12345678901234567890
+            "
+        });
+
+        // Test with lines taking up different amounts of display rows to ensure
+        // that, even in that case, only the buffer rows are taken into account.
+        cx.set_shared_state(indoc! {
+            "ˇ123456789012345678901234567890123456789012345678901234567890
+            1234567890123456789012345678901234567890
+            12345678901234567890
+            "
+        })
+        .await;
+        cx.simulate_shared_keystrokes("ctrl-v 2 j").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "«1ˇ»23456789012345678901234567890123456789012345678901234567890
+            «1ˇ»234567890123456789012345678901234567890
+            «1ˇ»2345678901234567890
+            "
+        });
+
+        // Same scenario as above, but using the up motion to ensure that the
+        // result is the same.
+        cx.set_shared_state(indoc! {
+            "123456789012345678901234567890123456789012345678901234567890
+            1234567890123456789012345678901234567890
+            ˇ12345678901234567890
+            "
+        })
+        .await;
+        cx.simulate_shared_keystrokes("ctrl-v 2 k").await;
+        cx.shared_state().await.assert_eq(indoc! {
+            "«1ˇ»23456789012345678901234567890123456789012345678901234567890
+            «1ˇ»234567890123456789012345678901234567890
+            «1ˇ»2345678901234567890
+            "
+        });
+    }
+
     #[gpui::test]
     async fn test_visual_object(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;

crates/vim/test_data/test_visual_block_wrapping_selection.json 🔗

@@ -0,0 +1,16 @@
+{"SetOption":{"value":"wrap"}}
+{"SetOption":{"value":"columns=12"}}
+{"Put":{"state":"ˇ12345678901234567890\n12345678901234567890\n12345678901234567890\n"}}
+{"Key":"ctrl-v"}
+{"Key":"j"}
+{"Get":{"state":"«1ˇ»2345678901234567890\n«1ˇ»2345678901234567890\n12345678901234567890\n","mode":"VisualBlock"}}
+{"Put":{"state":"ˇ123456789012345678901234567890123456789012345678901234567890\n1234567890123456789012345678901234567890\n12345678901234567890\n"}}
+{"Key":"ctrl-v"}
+{"Key":"2"}
+{"Key":"j"}
+{"Get":{"state":"«1ˇ»23456789012345678901234567890123456789012345678901234567890\n«1ˇ»234567890123456789012345678901234567890\n«1ˇ»2345678901234567890\n","mode":"VisualBlock"}}
+{"Put":{"state":"123456789012345678901234567890123456789012345678901234567890\n1234567890123456789012345678901234567890\nˇ12345678901234567890\n"}}
+{"Key":"ctrl-v"}
+{"Key":"2"}
+{"Key":"k"}
+{"Get":{"state":"«1ˇ»23456789012345678901234567890123456789012345678901234567890\n«1ˇ»234567890123456789012345678901234567890\n«1ˇ»2345678901234567890\n","mode":"VisualBlock"}}