From 3aa4e0c90b82db0a9cf542e56c2efae22500d8fc Mon Sep 17 00:00:00 2001 From: Andrew Marek Date: Tue, 6 Feb 2024 19:25:56 -0800 Subject: [PATCH] Fix Vim 'e' Behavior When Boundary Is Last Point on Line (#7424) This was originally just to fix https://github.com/zed-industries/zed/issues/4354, which I did by just returning the previous offset in `find_boundary`.. but `find_boundary` is used in the "insert mode" / normal editor too, so returning the previous boundary breaks existing functionality in that case. I was considering a new `find_boundary` function just for some of the vim motions like this, but I thought that this is straightforward enough and future Vim functions might need similar logic too. Release Notes: - Fixed https://github.com/zed-industries/zed/issues/4354 --- crates/editor/src/movement.rs | 35 ++++++++++++++++--- crates/vim/src/motion.rs | 30 ++++++++-------- .../test_next_word_end_newline_last_char.json | 3 ++ 3 files changed, 48 insertions(+), 20 deletions(-) create mode 100644 crates/vim/test_data/test_next_word_end_newline_last_char.json diff --git a/crates/editor/src/movement.rs b/crates/editor/src/movement.rs index 4aacc7e4e7..81c6c8c21f 100644 --- a/crates/editor/src/movement.rs +++ b/crates/editor/src/movement.rs @@ -395,14 +395,17 @@ pub fn find_preceding_boundary( /// Scans for a boundary following the given start point until a boundary is found, indicated by the /// given predicate returning true. The predicate is called with the character to the left and right /// of the candidate boundary location, and will be called with `\n` characters indicating the start -/// or end of a line. -pub fn find_boundary( +/// or end of a line. The function supports optionally returning the point just before the boundary +/// is found via return_point_before_boundary. +pub fn find_boundary_point( map: &DisplaySnapshot, from: DisplayPoint, find_range: FindRange, mut is_boundary: impl FnMut(char, char) -> bool, + return_point_before_boundary: bool, ) -> DisplayPoint { let mut offset = from.to_offset(&map, Bias::Right); + let mut prev_offset = offset; let mut prev_ch = None; for ch in map.buffer_snapshot.chars_at(offset) { @@ -411,16 +414,38 @@ pub fn find_boundary( } if let Some(prev_ch) = prev_ch { if is_boundary(prev_ch, ch) { - break; + if return_point_before_boundary { + return map.clip_point(prev_offset.to_display_point(map), Bias::Right); + } else { + break; + } } } - + prev_offset = offset; offset += ch.len_utf8(); prev_ch = Some(ch); } map.clip_point(offset.to_display_point(map), Bias::Right) } +pub fn find_boundary( + map: &DisplaySnapshot, + from: DisplayPoint, + find_range: FindRange, + is_boundary: impl FnMut(char, char) -> bool, +) -> DisplayPoint { + return find_boundary_point(map, from, find_range, is_boundary, false); +} + +pub fn find_boundary_exclusive( + map: &DisplaySnapshot, + from: DisplayPoint, + find_range: FindRange, + is_boundary: impl FnMut(char, char) -> bool, +) -> DisplayPoint { + return find_boundary_point(map, from, find_range, is_boundary, true); +} + /// Returns an iterator over the characters following a given offset in the [`DisplaySnapshot`]. /// The returned value also contains a range of the start/end of a returned character in /// the [`DisplaySnapshot`]. The offsets are relative to the start of a buffer. @@ -763,7 +788,7 @@ mod tests { &snapshot, display_points[0], FindRange::MultiLine, - is_boundary + is_boundary, ), display_points[1] ); diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 67abd5836c..99472dd1c8 100644 --- a/crates/vim/src/motion.rs +++ b/crates/vim/src/motion.rs @@ -798,23 +798,14 @@ fn next_word_end( *point.row_mut() += 1; *point.column_mut() = 0; } - point = movement::find_boundary(map, point, FindRange::MultiLine, |left, right| { - let left_kind = coerce_punctuation(char_kind(&scope, left), ignore_punctuation); - let right_kind = coerce_punctuation(char_kind(&scope, right), ignore_punctuation); - left_kind != right_kind && left_kind != CharKind::Whitespace - }); + point = + movement::find_boundary_exclusive(map, point, FindRange::MultiLine, |left, right| { + let left_kind = coerce_punctuation(char_kind(&scope, left), ignore_punctuation); + let right_kind = coerce_punctuation(char_kind(&scope, right), ignore_punctuation); - // find_boundary clips, so if the character after the next character is a newline or at the end of the document, we know - // we have backtracked already - if !map - .chars_at(point) - .nth(1) - .map(|(c, _)| c == '\n') - .unwrap_or(true) - { - *point.column_mut() = point.column().saturating_sub(1); - } + left_kind != right_kind && left_kind != CharKind::Whitespace + }); point = map.clip_point(point, Bias::Left); } point @@ -1285,6 +1276,15 @@ mod test { cx.assert_shared_state("one two thˇree four").await; } + #[gpui::test] + async fn test_next_word_end_newline_last_char(cx: &mut gpui::TestAppContext) { + let mut cx = NeovimBackedTestContext::new(cx).await; + let initial_state = indoc! {r"something(ˇfoo)"}; + cx.set_shared_state(initial_state).await; + cx.simulate_shared_keystrokes(["}"]).await; + cx.assert_shared_state(indoc! {r"something(fooˇ)"}).await; + } + #[gpui::test] async fn test_next_line_start(cx: &mut gpui::TestAppContext) { let mut cx = NeovimBackedTestContext::new(cx).await; diff --git a/crates/vim/test_data/test_next_word_end_newline_last_char.json b/crates/vim/test_data/test_next_word_end_newline_last_char.json new file mode 100644 index 0000000000..9dac2979f5 --- /dev/null +++ b/crates/vim/test_data/test_next_word_end_newline_last_char.json @@ -0,0 +1,3 @@ +{"Put":{"state":"something(ˇfoo)"}} +{"Key":"}"} +{"Get":{"state":"something(fooˇ)","mode":"Normal"}}