From 5a2a85a7db29d79c406be015ff2b287e20b02548 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 2 Apr 2024 22:16:52 -0600 Subject: [PATCH] Fix vim code working on display map chars (#10103) Release Notes: - vim: Fixed motion bugs when softwrap, folds or inlay hints were used. --- 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(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index e8a67a9456..03ff628b8f 100644 --- a/crates/editor/src/display_map.rs +++ b/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 + '_ { @@ -684,62 +684,26 @@ impl DisplaySnapshot { }) } - pub fn reverse_chars_at( + pub fn buffer_chars_at(&self, mut offset: usize) -> impl Iterator + '_ { + self.buffer_snapshot.chars_at(offset).map(move |ch| { + let ret = (ch, offset); + offset += ch.len_utf8(); + ret + }) + } + + pub fn reverse_buffer_chars_at( &self, - mut point: DisplayPoint, - ) -> impl Iterator + '_ { - 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; - } - - move |char| { - let at_point = column <= point.column(); - column = column.saturating_sub(char.len_utf8() as u32); - !at_point - } - }) + mut offset: usize, + ) -> impl Iterator + '_ { + 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 diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 811818314e..9a1dacf44d 100644 --- a/crates/editor/src/element.rs +++ b/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 }; diff --git a/crates/multi_buffer/src/anchor.rs b/crates/multi_buffer/src/anchor.rs index ee0862b957..b13a1fe2e6 100644 --- a/crates/multi_buffer/src/anchor.rs +++ b/crates/multi_buffer/src/anchor.rs @@ -137,3 +137,6 @@ impl AnchorRangeExt for Range { self.start.to_point(content)..self.end.to_point(content) } } + +#[derive(Clone, Copy, Eq, PartialEq, Debug, Hash, Ord, PartialOrd)] +pub struct Offset(pub usize); diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 1426c40b67..a6bc1251a0 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/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}; diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 3b5e3296ce..08235c03e7 100644 --- a/crates/vim/src/motion.rs +++ b/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( diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index b286fc75b9..c73021885f 100644 --- a/crates/vim/src/normal.rs +++ b/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 = old_selections + let selections = editor.selections.all::(cx); + let snapshot = editor.buffer().read(cx).snapshot(cx); + + let selection_start_rows: BTreeSet = 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::(); + 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 = old_selections + let selections = editor.selections.all::(cx); + let snapshot = editor.buffer().read(cx).snapshot(cx); + + let selection_end_rows: BTreeSet = 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::(); + 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| { diff --git a/crates/vim/src/normal/change.rs b/crates/vim/src/normal/change.rs index 7d9d6aa3a2..3c0a507bf9 100644 --- a/crates/vim/src/normal/change.rs +++ b/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, 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(); diff --git a/crates/vim/src/normal/delete.rs b/crates/vim/src/normal/delete.rs index b81c0b16a5..9c539ca490 100644 --- a/crates/vim/src/normal/delete.rs +++ b/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); diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index 6eb2506c9d..f6d5ff1499 100644 --- a/crates/vim/src/object.rs +++ b/crates/vim/src/object.rs @@ -347,11 +347,10 @@ fn around_word( relative_to: DisplayPoint, ignore_punctuation: bool, ) -> Option> { - 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> { 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, + range: Range, stop_at_newline: bool, ) -> Range { + 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