editor: Fix block cursor offset when selecting text (#42837)

Lennart and dino created 20 hours ago

Vim visual mode and Helix selection mode both require the cursor to be
on the last character of the selection. Until now, this was implemented
by offsetting the cursor one character to the left whenever a block
cursor is used. (Since the visual modes use a block cursor.)

However, this oversees the problem that **some users might want to use
the block cursor without being in visual mode**. Meaning that the cursor
is offset by one character to the left even though Vim/Helix mode isn't
even activated.

Since the Vim mode implementation is separate from the `editor` crate
the solution is not as straightforward as just checking the current vim
mode. Therefore this PR introduces a new `Editor` struct field called
`cursor_offset_on_selection`. This field replaces the previous check 
condition and is set to `true` whenever the Vim mode is changed to a 
visual mode, and `false` otherwise.

Closes #36677 and #20121

Release Notes:

- Fixes block and hollow cursor being offset when selecting text

---------

Co-authored-by: dino <[e-mail]>

Change summary

crates/editor/src/editor.rs  |  8 ++++++++
crates/editor/src/element.rs | 17 +++++++++++------
crates/vim/src/vim.rs        |  1 +
3 files changed, 20 insertions(+), 6 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -1107,6 +1107,9 @@ pub struct Editor {
     pending_rename: Option<RenameState>,
     searchable: bool,
     cursor_shape: CursorShape,
+    /// Whether the cursor is offset one character to the left when something is
+    /// selected (needed for vim visual mode)
+    cursor_offset_on_selection: bool,
     current_line_highlight: Option<CurrentLineHighlight>,
     pub collapse_matches: bool,
     autoindent_mode: Option<AutoindentMode>,
@@ -2281,6 +2284,7 @@ impl Editor {
             cursor_shape: EditorSettings::get_global(cx)
                 .cursor_shape
                 .unwrap_or_default(),
+            cursor_offset_on_selection: false,
             current_line_highlight: None,
             autoindent_mode: Some(AutoindentMode::EachLine),
             collapse_matches: false,
@@ -3095,6 +3099,10 @@ impl Editor {
         self.cursor_shape
     }
 
+    pub fn set_cursor_offset_on_selection(&mut self, set_cursor_offset_on_selection: bool) {
+        self.cursor_offset_on_selection = set_cursor_offset_on_selection;
+    }
+
     pub fn set_current_line_highlight(
         &mut self,
         current_line_highlight: Option<CurrentLineHighlight>,

crates/editor/src/element.rs

@@ -132,6 +132,7 @@ impl SelectionLayout {
     fn new<T: ToPoint + ToDisplayPoint + Clone>(
         selection: Selection<T>,
         line_mode: bool,
+        cursor_offset: bool,
         cursor_shape: CursorShape,
         map: &DisplaySnapshot,
         is_newest: bool,
@@ -152,12 +153,9 @@ impl SelectionLayout {
         }
 
         // any vim visual mode (including line mode)
-        if (cursor_shape == CursorShape::Block || cursor_shape == CursorShape::Hollow)
-            && !range.is_empty()
-            && !selection.reversed
-        {
+        if cursor_offset && !range.is_empty() && !selection.reversed {
             if head.column() > 0 {
-                head = map.clip_point(DisplayPoint::new(head.row(), head.column() - 1), Bias::Left)
+                head = map.clip_point(DisplayPoint::new(head.row(), head.column() - 1), Bias::Left);
             } else if head.row().0 > 0 && head != map.max_point() {
                 head = map.clip_point(
                     DisplayPoint::new(
@@ -1441,6 +1439,7 @@ impl EditorElement {
                     let layout = SelectionLayout::new(
                         selection,
                         editor.selections.line_mode(),
+                        editor.cursor_offset_on_selection,
                         editor.cursor_shape,
                         &snapshot.display_snapshot,
                         is_newest,
@@ -1487,6 +1486,7 @@ impl EditorElement {
                     let drag_cursor_layout = SelectionLayout::new(
                         drop_cursor.clone(),
                         false,
+                        editor.cursor_offset_on_selection,
                         CursorShape::Bar,
                         &snapshot.display_snapshot,
                         false,
@@ -1550,6 +1550,7 @@ impl EditorElement {
                         .push(SelectionLayout::new(
                             selection.selection,
                             selection.line_mode,
+                            editor.cursor_offset_on_selection,
                             selection.cursor_shape,
                             &snapshot.display_snapshot,
                             false,
@@ -1560,6 +1561,8 @@ impl EditorElement {
 
                 selections.extend(remote_selections.into_values());
             } else if !editor.is_focused(window) && editor.show_cursor_when_unfocused {
+                let cursor_offset_on_selection = editor.cursor_offset_on_selection;
+
                 let layouts = snapshot
                     .buffer_snapshot()
                     .selections_in_range(&(start_anchor..end_anchor), true)
@@ -1567,6 +1570,7 @@ impl EditorElement {
                         SelectionLayout::new(
                             selection,
                             line_mode,
+                            cursor_offset_on_selection,
                             cursor_shape,
                             &snapshot.display_snapshot,
                             false,
@@ -3290,6 +3294,7 @@ impl EditorElement {
                 SelectionLayout::new(
                     newest,
                     editor.selections.line_mode(),
+                    editor.cursor_offset_on_selection,
                     editor.cursor_shape,
                     &snapshot.display_snapshot,
                     true,
@@ -11858,7 +11863,7 @@ mod tests {
 
         window
             .update(cx, |editor, window, cx| {
-                editor.cursor_shape = CursorShape::Block;
+                editor.cursor_offset_on_selection = true;
                 editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
                     s.select_ranges([
                         Point::new(0, 0)..Point::new(1, 0),

crates/vim/src/vim.rs

@@ -1943,6 +1943,7 @@ impl Vim {
             editor.set_collapse_matches(collapse_matches);
             editor.set_input_enabled(vim.editor_input_enabled());
             editor.set_autoindent(vim.should_autoindent());
+            editor.set_cursor_offset_on_selection(vim.mode.is_visual());
             editor
                 .selections
                 .set_line_mode(matches!(vim.mode, Mode::VisualLine));