Fix vim code working on display map chars (#10103)

Conrad Irwin created

Release Notes:

- vim: Fixed motion bugs when softwrap, folds or inlay hints were used.

Change summary

crates/editor/src/display_map.rs        | 82 +++++---------------------
crates/editor/src/element.rs            |  9 +-
crates/multi_buffer/src/anchor.rs       |  3 
crates/multi_buffer/src/multi_buffer.rs |  2 
crates/vim/src/motion.rs                |  8 +-
crates/vim/src/normal.rs                | 47 +++++++-------
crates/vim/src/normal/change.rs         | 13 ++-
crates/vim/src/normal/delete.rs         |  8 +
crates/vim/src/object.rs                | 63 +++++++++-----------
9 files changed, 95 insertions(+), 140 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -657,7 +657,7 @@ impl DisplaySnapshot {
         layout_line.closest_index_for_x(x) as u32
     }
 
-    pub fn chars_at(
+    pub fn display_chars_at(
         &self,
         mut point: DisplayPoint,
     ) -> impl Iterator<Item = (char, DisplayPoint)> + '_ {
@@ -684,62 +684,26 @@ impl DisplaySnapshot {
             })
     }
 
-    pub fn reverse_chars_at(
-        &self,
-        mut point: DisplayPoint,
-    ) -> impl Iterator<Item = (char, DisplayPoint)> + '_ {
-        point = DisplayPoint(self.block_snapshot.clip_point(point.0, Bias::Left));
-        self.reverse_text_chunks(point.row())
-            .flat_map(|chunk| chunk.chars().rev())
-            .skip_while({
-                let mut column = self.line_len(point.row());
-                if self.max_point().row() > point.row() {
-                    column += 1;
-                }
+    pub fn buffer_chars_at(&self, mut offset: usize) -> impl Iterator<Item = (char, usize)> + '_ {
+        self.buffer_snapshot.chars_at(offset).map(move |ch| {
+            let ret = (ch, offset);
+            offset += ch.len_utf8();
+            ret
+        })
+    }
 
-                move |char| {
-                    let at_point = column <= point.column();
-                    column = column.saturating_sub(char.len_utf8() as u32);
-                    !at_point
-                }
-            })
+    pub fn reverse_buffer_chars_at(
+        &self,
+        mut offset: usize,
+    ) -> impl Iterator<Item = (char, usize)> + '_ {
+        self.buffer_snapshot
+            .reversed_chars_at(offset)
             .map(move |ch| {
-                if ch == '\n' {
-                    *point.row_mut() -= 1;
-                    *point.column_mut() = self.line_len(point.row());
-                } else {
-                    *point.column_mut() = point.column().saturating_sub(ch.len_utf8() as u32);
-                }
-                (ch, point)
+                offset -= ch.len_utf8();
+                (ch, offset)
             })
     }
 
-    pub fn column_to_chars(&self, display_row: u32, target: u32) -> u32 {
-        let mut count = 0;
-        let mut column = 0;
-        for (c, _) in self.chars_at(DisplayPoint::new(display_row, 0)) {
-            if column >= target {
-                break;
-            }
-            count += 1;
-            column += c.len_utf8() as u32;
-        }
-        count
-    }
-
-    pub fn column_from_chars(&self, display_row: u32, char_count: u32) -> u32 {
-        let mut column = 0;
-
-        for (count, (c, _)) in self.chars_at(DisplayPoint::new(display_row, 0)).enumerate() {
-            if c == '\n' || count >= char_count as usize {
-                break;
-            }
-            column += c.len_utf8() as u32;
-        }
-
-        column
-    }
-
     pub fn clip_point(&self, point: DisplayPoint, bias: Bias) -> DisplayPoint {
         let mut clipped = self.block_snapshot.clip_point(point.0, bias);
         if self.clip_at_line_ends {
@@ -808,20 +772,6 @@ impl DisplaySnapshot {
         result
     }
 
-    pub fn line_indent(&self, display_row: u32) -> (u32, bool) {
-        let mut indent = 0;
-        let mut is_blank = true;
-        for (c, _) in self.chars_at(DisplayPoint::new(display_row, 0)) {
-            if c == ' ' {
-                indent += 1;
-            } else {
-                is_blank = c == '\n';
-                break;
-            }
-        }
-        (indent, is_blank)
-    }
-
     pub fn line_indent_for_buffer_row(&self, buffer_row: u32) -> (u32, bool) {
         let (buffer, range) = self
             .buffer_snapshot

crates/editor/src/element.rs 🔗

@@ -876,10 +876,8 @@ impl EditorElement {
                         block_width = em_width;
                     }
                     let block_text = if let CursorShape::Block = selection.cursor_shape {
-                        snapshot
-                            .chars_at(cursor_position)
-                            .next()
-                            .and_then(|(character, _)| {
+                        snapshot.display_chars_at(cursor_position).next().and_then(
+                            |(character, _)| {
                                 let text = if character == '\n' {
                                     SharedString::from(" ")
                                 } else {
@@ -900,7 +898,8 @@ impl EditorElement {
                                         }],
                                     )
                                     .log_err()
-                            })
+                            },
+                        )
                     } else {
                         None
                     };

crates/multi_buffer/src/anchor.rs 🔗

@@ -137,3 +137,6 @@ impl AnchorRangeExt for Range<Anchor> {
         self.start.to_point(content)..self.end.to_point(content)
     }
 }
+
+#[derive(Clone, Copy, Eq, PartialEq, Debug, Hash, Ord, PartialOrd)]
+pub struct Offset(pub usize);

crates/multi_buffer/src/multi_buffer.rs 🔗

@@ -1,6 +1,6 @@
 mod anchor;
 
-pub use anchor::{Anchor, AnchorRangeExt};
+pub use anchor::{Anchor, AnchorRangeExt, Offset};
 use anyhow::{anyhow, Result};
 use clock::ReplicaId;
 use collections::{BTreeMap, Bound, HashMap, HashSet};

crates/vim/src/motion.rs 🔗

@@ -1283,21 +1283,21 @@ pub(crate) fn first_non_whitespace(
     display_lines: bool,
     from: DisplayPoint,
 ) -> DisplayPoint {
-    let mut last_point = start_of_line(map, display_lines, from);
+    let mut start_offset = start_of_line(map, display_lines, from).to_offset(map, Bias::Left);
     let scope = map.buffer_snapshot.language_scope_at(from.to_point(map));
-    for (ch, point) in map.chars_at(last_point) {
+    for (ch, offset) in map.buffer_chars_at(start_offset) {
         if ch == '\n' {
             return from;
         }
 
-        last_point = point;
+        start_offset = offset;
 
         if char_kind(&scope, ch) != CharKind::Whitespace {
             break;
         }
     }
 
-    map.clip_point(last_point, Bias::Left)
+    start_offset.to_display_point(map)
 }
 
 pub(crate) fn start_of_line(

crates/vim/src/normal.rs 🔗

@@ -19,9 +19,9 @@ use crate::{
 };
 use collections::BTreeSet;
 use editor::scroll::Autoscroll;
-use editor::{Bias, DisplayPoint};
+use editor::Bias;
 use gpui::{actions, ViewContext, WindowContext};
-use language::SelectionGoal;
+use language::{Point, SelectionGoal};
 use log::error;
 use workspace::Workspace;
 
@@ -283,19 +283,20 @@ fn insert_line_above(_: &mut Workspace, _: &InsertLineAbove, cx: &mut ViewContex
         vim.switch_mode(Mode::Insert, false, cx);
         vim.update_active_editor(cx, |_, editor, cx| {
             editor.transact(cx, |editor, cx| {
-                let (map, old_selections) = editor.selections.all_display(cx);
-                let selection_start_rows: BTreeSet<u32> = old_selections
+                let selections = editor.selections.all::<Point>(cx);
+                let snapshot = editor.buffer().read(cx).snapshot(cx);
+
+                let selection_start_rows: BTreeSet<u32> = selections
                     .into_iter()
-                    .map(|selection| selection.start.row())
+                    .map(|selection| selection.start.row)
                     .collect();
                 let edits = selection_start_rows.into_iter().map(|row| {
-                    let (indent, _) = map.line_indent(row);
-                    let start_of_line =
-                        motion::start_of_line(&map, false, DisplayPoint::new(row, 0))
-                            .to_point(&map);
-                    let mut new_text = " ".repeat(indent as usize);
-                    new_text.push('\n');
-                    (start_of_line..start_of_line, new_text)
+                    let indent = snapshot
+                        .indent_size_for_line(row)
+                        .chars()
+                        .collect::<String>();
+                    let start_of_line = Point::new(row, 0);
+                    (start_of_line..start_of_line, indent + "\n")
                 });
                 editor.edit_with_autoindent(edits, cx);
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
@@ -317,20 +318,20 @@ fn insert_line_below(_: &mut Workspace, _: &InsertLineBelow, cx: &mut ViewContex
         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);
-                let selection_end_rows: BTreeSet<u32> = old_selections
+                let selections = editor.selections.all::<Point>(cx);
+                let snapshot = editor.buffer().read(cx).snapshot(cx);
+
+                let selection_end_rows: BTreeSet<u32> = selections
                     .into_iter()
-                    .map(|selection| selection.end.row())
+                    .map(|selection| selection.end.row)
                     .collect();
                 let edits = selection_end_rows.into_iter().map(|row| {
-                    let (indent, _) = map.line_indent(row);
-                    let end_of_line =
-                        motion::end_of_line(&map, false, DisplayPoint::new(row, 0), 1)
-                            .to_point(&map);
-
-                    let mut new_text = "\n".to_string();
-                    new_text.push_str(&" ".repeat(indent as usize));
-                    (end_of_line..end_of_line, new_text)
+                    let indent = snapshot
+                        .indent_size_for_line(row)
+                        .chars()
+                        .collect::<String>();
+                    let end_of_line = Point::new(row, snapshot.line_len(row));
+                    (end_of_line..end_of_line, "\n".to_string() + &indent)
                 });
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                     s.maybe_move_cursors_with(|map, cursor, goal| {

crates/vim/src/normal/change.rs 🔗

@@ -6,7 +6,10 @@ use crate::{
     Vim,
 };
 use editor::{
-    display_map::DisplaySnapshot, movement::TextLayoutDetails, scroll::Autoscroll, DisplayPoint,
+    display_map::{DisplaySnapshot, ToDisplayPoint},
+    movement::TextLayoutDetails,
+    scroll::Autoscroll,
+    Bias, DisplayPoint,
 };
 use gpui::WindowContext;
 use language::{char_kind, CharKind, Selection};
@@ -56,15 +59,17 @@ pub fn change_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &m
                             &text_layout_details,
                         );
                         if let Motion::CurrentLine = motion {
+                            let mut start_offset = selection.start.to_offset(map, Bias::Left);
                             let scope = map
                                 .buffer_snapshot
                                 .language_scope_at(selection.start.to_point(&map));
-                            for (ch, _) in map.chars_at(selection.start) {
+                            for (ch, offset) in map.buffer_chars_at(start_offset) {
                                 if ch == '\n' || char_kind(&scope, ch) != CharKind::Whitespace {
                                     break;
                                 }
-                                *selection.start.column_mut() += 1;
+                                start_offset = offset + ch.len_utf8();
                             }
+                            selection.start = start_offset.to_display_point(map);
                         }
                         result
                     };
@@ -126,7 +131,7 @@ fn expand_changed_word_selection(
             .buffer_snapshot
             .language_scope_at(selection.start.to_point(map));
         let in_word = map
-            .chars_at(selection.head())
+            .buffer_chars_at(selection.head().to_offset(map, Bias::Left))
             .next()
             .map(|(c, _)| char_kind(&scope, c) != CharKind::Whitespace)
             .unwrap_or_default();

crates/vim/src/normal/delete.rs 🔗

@@ -84,13 +84,15 @@ pub fn delete_object(vim: &mut Vim, object: Object, around: bool, cx: &mut Windo
                                 selection.start = (start - '\n'.len_utf8()).to_display_point(map);
                             }
                         };
+                    let range = selection.start.to_offset(map, Bias::Left)
+                        ..selection.end.to_offset(map, Bias::Right);
                     let contains_only_newlines = map
-                        .chars_at(selection.start)
-                        .take_while(|(_, p)| p < &selection.end)
+                        .buffer_chars_at(range.start)
+                        .take_while(|(_, p)| p < &range.end)
                         .all(|(char, _)| char == '\n')
                         && !offset_range.is_empty();
                     let end_at_newline = map
-                        .chars_at(selection.end)
+                        .buffer_chars_at(range.end)
                         .next()
                         .map(|(c, _)| c == '\n')
                         .unwrap_or(false);

crates/vim/src/object.rs 🔗

@@ -347,11 +347,10 @@ fn around_word(
     relative_to: DisplayPoint,
     ignore_punctuation: bool,
 ) -> Option<Range<DisplayPoint>> {
-    let scope = map
-        .buffer_snapshot
-        .language_scope_at(relative_to.to_point(map));
+    let offset = relative_to.to_offset(map, Bias::Left);
+    let scope = map.buffer_snapshot.language_scope_at(offset);
     let in_word = map
-        .chars_at(relative_to)
+        .buffer_chars_at(offset)
         .next()
         .map(|(c, _)| char_kind(&scope, c) != CharKind::Whitespace)
         .unwrap_or(false);
@@ -565,51 +564,52 @@ fn sentence(
     around: bool,
 ) -> Option<Range<DisplayPoint>> {
     let mut start = None;
-    let mut previous_end = relative_to;
+    let relative_offset = relative_to.to_offset(map, Bias::Left);
+    let mut previous_end = relative_offset;
 
-    let mut chars = map.chars_at(relative_to).peekable();
+    let mut chars = map.buffer_chars_at(previous_end).peekable();
 
     // Search backwards for the previous sentence end or current sentence start. Include the character under relative_to
-    for (char, point) in chars
+    for (char, offset) in chars
         .peek()
         .cloned()
         .into_iter()
-        .chain(map.reverse_chars_at(relative_to))
+        .chain(map.reverse_buffer_chars_at(previous_end))
     {
-        if is_sentence_end(map, point) {
+        if is_sentence_end(map, offset) {
             break;
         }
 
         if is_possible_sentence_start(char) {
-            start = Some(point);
+            start = Some(offset);
         }
 
-        previous_end = point;
+        previous_end = offset;
     }
 
     // Search forward for the end of the current sentence or if we are between sentences, the start of the next one
-    let mut end = relative_to;
-    for (char, point) in chars {
+    let mut end = relative_offset;
+    for (char, offset) in chars {
         if start.is_none() && is_possible_sentence_start(char) {
             if around {
-                start = Some(point);
+                start = Some(offset);
                 continue;
             } else {
-                end = point;
+                end = offset;
                 break;
             }
         }
 
-        end = point;
-        *end.column_mut() += char.len_utf8() as u32;
-        end = map.clip_point(end, Bias::Left);
+        if char != '\n' {
+            end = offset + char.len_utf8();
+        }
 
         if is_sentence_end(map, end) {
             break;
         }
     }
 
-    let mut range = start.unwrap_or(previous_end)..end;
+    let mut range = start.unwrap_or(previous_end).to_display_point(map)..end.to_display_point(map);
     if around {
         range = expand_to_include_whitespace(map, range, false);
     }
@@ -624,8 +624,8 @@ fn is_possible_sentence_start(character: char) -> bool {
 const SENTENCE_END_PUNCTUATION: &[char] = &['.', '!', '?'];
 const SENTENCE_END_FILLERS: &[char] = &[')', ']', '"', '\''];
 const SENTENCE_END_WHITESPACE: &[char] = &[' ', '\t', '\n'];
-fn is_sentence_end(map: &DisplaySnapshot, point: DisplayPoint) -> bool {
-    let mut next_chars = map.chars_at(point).peekable();
+fn is_sentence_end(map: &DisplaySnapshot, offset: usize) -> bool {
+    let mut next_chars = map.buffer_chars_at(offset).peekable();
     if let Some((char, _)) = next_chars.next() {
         // We are at a double newline. This position is a sentence end.
         if char == '\n' && next_chars.peek().map(|(c, _)| c == &'\n').unwrap_or(false) {
@@ -638,7 +638,7 @@ fn is_sentence_end(map: &DisplaySnapshot, point: DisplayPoint) -> bool {
         }
     }
 
-    for (char, _) in map.reverse_chars_at(point) {
+    for (char, _) in map.reverse_buffer_chars_at(offset) {
         if SENTENCE_END_PUNCTUATION.contains(&char) {
             return true;
         }
@@ -655,26 +655,21 @@ fn is_sentence_end(map: &DisplaySnapshot, point: DisplayPoint) -> bool {
 /// whitespace to the end first and falls back to the start if there was none.
 fn expand_to_include_whitespace(
     map: &DisplaySnapshot,
-    mut range: Range<DisplayPoint>,
+    range: Range<DisplayPoint>,
     stop_at_newline: bool,
 ) -> Range<DisplayPoint> {
+    let mut range = range.start.to_offset(map, Bias::Left)..range.end.to_offset(map, Bias::Right);
     let mut whitespace_included = false;
 
-    let mut chars = map.chars_at(range.end).peekable();
-    while let Some((char, point)) = chars.next() {
+    let mut chars = map.buffer_chars_at(range.end).peekable();
+    while let Some((char, offset)) = chars.next() {
         if char == '\n' && stop_at_newline {
             break;
         }
 
         if char.is_whitespace() {
-            // Set end to the next display_point or the character position after the current display_point
-            range.end = chars.peek().map(|(_, point)| *point).unwrap_or_else(|| {
-                let mut end = point;
-                *end.column_mut() += char.len_utf8() as u32;
-                map.clip_point(end, Bias::Left)
-            });
-
             if char != '\n' {
+                range.end = offset + char.len_utf8();
                 whitespace_included = true;
             }
         } else {
@@ -684,7 +679,7 @@ fn expand_to_include_whitespace(
     }
 
     if !whitespace_included {
-        for (char, point) in map.reverse_chars_at(range.start) {
+        for (char, point) in map.reverse_buffer_chars_at(range.start) {
             if char == '\n' && stop_at_newline {
                 break;
             }
@@ -697,7 +692,7 @@ fn expand_to_include_whitespace(
         }
     }
 
-    range
+    range.start.to_display_point(map)..range.end.to_display_point(map)
 }
 
 /// If not `around` (i.e. inner), returns a range that surrounds the paragraph