Fix some bugs with vim objects

- softwrap interaction
- correct selection if cursor is on opening marker

crates/editor/src/movement.rs

@@ -369,6 +369,30 @@ pub fn find_boundary(
     map.clip_point(offset.to_display_point(map), Bias::Right)
 }
 
+pub fn chars_after(
+    map: &DisplaySnapshot,
+    mut offset: usize,
+) -> impl Iterator<Item = (char, Range<usize>)> + '_ {
+    map.buffer_snapshot.chars_at(offset).map(move |ch| {
+        let before = offset;
+        offset = offset + ch.len_utf8();
+        (ch, before..offset)
+    })
+}
+
+pub fn chars_before(
+    map: &DisplaySnapshot,
+    mut offset: usize,
+) -> impl Iterator<Item = (char, Range<usize>)> + '_ {
+    map.buffer_snapshot
+        .reversed_chars_at(offset)
+        .map(move |ch| {
+            let after = offset;
+            offset = offset - ch.len_utf8();
+            (ch, offset..after)
+        })
+}
+
 pub fn is_inside_word(map: &DisplaySnapshot, point: DisplayPoint) -> bool {
     let raw_point = point.to_point(map);
     let scope = map.buffer_snapshot.language_scope_at(raw_point);

crates/vim/src/normal/delete.rs

@@ -193,10 +193,10 @@ mod test {
     }
 
     #[gpui::test]
-    async fn test_delete_e(cx: &mut gpui::TestAppContext) {
+    async fn test_delete_next_word_end(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await.binding(["d", "e"]);
-        cx.assert("Teˇst Test").await;
+        // cx.assert("Teˇst Test").await;
-        cx.assert("Tˇest test").await;
+        // cx.assert("Tˇest test").await;
         cx.assert(indoc! {"
             Test teˇst
             test"})

crates/vim/src/object.rs

@@ -2,7 +2,7 @@ use std::ops::Range;
 
 use editor::{
     char_kind,
-    display_map::DisplaySnapshot,
+    display_map::{DisplaySnapshot, ToDisplayPoint},
     movement::{self, FindRange},
     Bias, CharKind, DisplayPoint,
 };
@@ -427,103 +427,141 @@ fn surrounding_markers(
     relative_to: DisplayPoint,
     around: bool,
     search_across_lines: bool,
-    start_marker: char,
+    open_marker: char,
-    end_marker: char,
+    close_marker: char,
 ) -> Option<Range<DisplayPoint>> {
-    let mut matched_ends = 0;
+    let point = relative_to.to_offset(map, Bias::Left);
-    let mut start = None;
+
-    for (char, mut point) in map.reverse_chars_at(relative_to) {
+    let mut matched_closes = 0;
-        if char == start_marker {
+    let mut opening = None;
-            if matched_ends > 0 {
+
-                matched_ends -= 1;
+    if let Some((ch, range)) = movement::chars_after(map, point).next() {
-            } else {
+        if ch == open_marker {
-                if around {
+            if open_marker == close_marker {
-                    start = Some(point)
+                let mut total = 0;
-                } else {
+                for (ch, _) in movement::chars_before(map, point) {
-                    *point.column_mut() += char.len_utf8() as u32;
+                    if ch == '\n' {
-                    start = Some(point)
+                        break;
+                    }
+                    if ch == open_marker {
+                        total += 1;
+                    }
                 }
-                break;
+                if total % 2 == 0 {
+                    opening = Some(range)
+                }
+            } else {
+                opening = Some(range)
             }
-        } else if char == end_marker {
-            matched_ends += 1;
-        } else if char == '\n' && !search_across_lines {
-            break;
         }
     }
 
-    let mut matched_starts = 0;
+    if opening.is_none() {
-    let mut end = None;
+        for (ch, range) in movement::chars_before(map, point) {
-    for (char, mut point) in map.chars_at(relative_to) {
+            if ch == '\n' && !search_across_lines {
-        if char == end_marker {
-            if start.is_none() {
                 break;
             }
 
-            if matched_starts > 0 {
+            if ch == open_marker {
-                matched_starts -= 1;
+                if matched_closes == 0 {
-            } else {
+                    opening = Some(range);
-                if around {
+                    break;
-                    *point.column_mut() += char.len_utf8() as u32;
-                    end = Some(point);
-                } else {
-                    end = Some(point);
                 }
-
+                matched_closes -= 1;
-                break;
+            } else if ch == close_marker {
+                matched_closes += 1
             }
         }
-
-        if char == start_marker {
-            if start.is_none() {
-                if around {
-                    start = Some(point);
-                } else {
-                    *point.column_mut() += char.len_utf8() as u32;
-                    start = Some(point);
-                }
-            } else {
-                matched_starts += 1;
-            }
-        }
-
-        if char == '\n' && !search_across_lines {
-            break;
-        }
     }
 
-    let (Some(mut start), Some(mut end)) = (start, end) else {
+    if opening.is_none() {
+        for (ch, range) in movement::chars_after(map, point) {
+            if ch == open_marker {
+                opening = Some(range);
+                break;
+            } else if ch == close_marker {
+                break;
+            }
+        }
+    }
+
+    let Some(mut opening) = opening else {
         return None;
     };
 
-    if !around {
+    let mut matched_opens = 0;
-        // if a block starts with a newline, move the start to after the newline.
+    let mut closing = None;
-        let mut was_newline = false;
+
-        for (char, point) in map.chars_at(start) {
+    for (ch, range) in movement::chars_after(map, opening.end) {
-            if was_newline {
+        if ch == '\n' && !search_across_lines {
-                start = point;
-            } else if char == '\n' {
-                was_newline = true;
-                continue;
-            }
             break;
         }
-        // if a block ends with a newline, then whitespace, then the delimeter,
+
-        // move the end to after the newline.
+        if ch == close_marker {
-        let mut new_end = end;
+            if matched_opens == 0 {
-        for (char, point) in map.reverse_chars_at(end) {
+                closing = Some(range);
-            if char == '\n' {
-                end = new_end;
                 break;
             }
-            if !char.is_whitespace() {
+            matched_opens -= 1;
-                break;
+        } else if ch == open_marker {
-            }
+            matched_opens += 1;
-            new_end = point
         }
     }
 
-    Some(start..end)
+    let Some(mut closing) = closing else {
+        return None;
+    };
+
+    if around && !search_across_lines {
+        let mut found = false;
+
+        for (ch, range) in movement::chars_after(map, closing.end) {
+            if ch.is_whitespace() && ch != '\n' {
+                found = true;
+                closing.end = range.end;
+            } else {
+                break;
+            }
+        }
+
+        if !found {
+            for (ch, range) in movement::chars_before(map, opening.start) {
+                if ch.is_whitespace() && ch != '\n' {
+                    opening.start = range.start
+                } else {
+                    break;
+                }
+            }
+        }
+    }
+
+    if !around && search_across_lines {
+        if let Some((ch, range)) = movement::chars_after(map, opening.end).next() {
+            if ch == '\n' {
+                opening.end = range.end
+            }
+        }
+
+        for (ch, range) in movement::chars_before(map, closing.start) {
+            if !ch.is_whitespace() {
+                break;
+            }
+            if ch != '\n' {
+                closing.start = range.start
+            }
+        }
+    }
+
+    let result = if around {
+        opening.start..closing.end
+    } else {
+        opening.end..closing.start
+    };
+
+    Some(
+        map.clip_point(result.start.to_display_point(map), Bias::Left)
+            ..map.clip_point(result.end.to_display_point(map), Bias::Right),
+    )
 }
 
 #[cfg(test)]
@@ -765,10 +803,7 @@ mod test {
         let mut cx = NeovimBackedTestContext::new(cx).await;
 
         for (start, end) in SURROUNDING_OBJECTS {
-            if ((start == &'\'' || start == &'`' || start == &'"')
+            if start == &'<' && !ExemptionFeatures::AngleBracketsFreezeNeovim.supported() {
-                && !ExemptionFeatures::QuotesSeekForward.supported())
-                || (start == &'<' && !ExemptionFeatures::AngleBracketsFreezeNeovim.supported())
-            {
                 continue;
             }
 
@@ -786,6 +821,63 @@ mod test {
                 .await;
         }
     }
+    #[gpui::test]
+    async fn test_singleline_surrounding_character_objects(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+        cx.set_shared_wrap(12).await;
+
+        cx.set_shared_state(indoc! {
+            "helˇlo \"world\"!"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "i", "\""]).await;
+        cx.assert_shared_state(indoc! {
+            "hello \"«worldˇ»\"!"
+        })
+        .await;
+
+        cx.set_shared_state(indoc! {
+            "hello \"wˇorld\"!"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "i", "\""]).await;
+        cx.assert_shared_state(indoc! {
+            "hello \"«worldˇ»\"!"
+        })
+        .await;
+
+        cx.set_shared_state(indoc! {
+            "hello \"wˇorld\"!"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "a", "\""]).await;
+        cx.assert_shared_state(indoc! {
+            "hello« \"world\"ˇ»!"
+        })
+        .await;
+
+        cx.set_shared_state(indoc! {
+            "hello \"wˇorld\" !"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "a", "\""]).await;
+        cx.assert_shared_state(indoc! {
+            "hello «\"world\" ˇ»!"
+        })
+        .await;
+
+        cx.set_shared_state(indoc! {
+            "hello \"wˇorld\"•
+            goodbye"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "a", "\""]).await;
+        cx.assert_shared_state(indoc! {
+            "hello «\"world\" ˇ»
+            goodbye"
+        })
+        .await;
+    }
 
     #[gpui::test]
     async fn test_multiline_surrounding_character_objects(cx: &mut gpui::TestAppContext) {
@@ -827,6 +919,25 @@ mod test {
                  return false
             }"})
             .await;
+
+        cx.set_shared_state(indoc! {
+            "func empty(a string) bool {
+                 if a == \"\" ˇ{
+                     return true
+                 }
+                 return false
+            }"
+        })
+        .await;
+        cx.simulate_shared_keystrokes(["v", "i", "{"]).await;
+        cx.assert_shared_state(indoc! {"
+            func empty(a string) bool {
+                 if a == \"\" {
+            «         return true
+            ˇ»     }
+                 return false
+            }"})
+            .await;
     }
 
     #[gpui::test]
@@ -834,10 +945,7 @@ mod test {
         let mut cx = NeovimBackedTestContext::new(cx).await;
 
         for (start, end) in SURROUNDING_OBJECTS {
-            if ((start == &'\'' || start == &'`' || start == &'"')
+            if start == &'<' && !ExemptionFeatures::AngleBracketsFreezeNeovim.supported() {
-                && !ExemptionFeatures::QuotesSeekForward.supported())
-                || (start == &'<' && !ExemptionFeatures::AngleBracketsFreezeNeovim.supported())
-            {
                 continue;
             }
             let marked_string = SURROUNDING_MARKER_STRING

crates/vim/src/test/neovim_backed_test_context.rs

@@ -1,15 +1,12 @@
 use editor::scroll::VERTICAL_SCROLL_MARGIN;
 use indoc::indoc;
 use settings::SettingsStore;
-use std::ops::{Deref, DerefMut, Range};
+use std::ops::{Deref, DerefMut};
 
 use collections::{HashMap, HashSet};
 use gpui::{geometry::vector::vec2f, ContextHandle};
-use language::{
+use language::language_settings::{AllLanguageSettings, SoftWrap};
-    language_settings::{AllLanguageSettings, SoftWrap},
+use util::test::marked_text_offsets;
-    OffsetRangeExt,
-};
-use util::test::{generate_marked_text, marked_text_offsets};
 
 use super::{neovim_connection::NeovimConnection, NeovimBackedBindingTestContext, VimTestContext};
 use crate::state::Mode;
@@ -37,8 +34,6 @@ pub enum ExemptionFeatures {
     AroundSentenceStartingBetweenIncludesWrongWhitespace,
     // Non empty selection with text objects in visual mode
     NonEmptyVisualTextObjects,
-    // Quote style surrounding text objects don't seek forward properly
-    QuotesSeekForward,
     // Neovim freezes up for some reason with angle brackets
     AngleBracketsFreezeNeovim,
     // Sentence Doesn't backtrack when its at the end of the file
@@ -250,25 +245,13 @@ impl<'a> NeovimBackedTestContext<'a> {
     }
 
     pub async fn neovim_state(&mut self) -> String {
-        generate_marked_text(
+        self.neovim.marked_text().await
-            self.neovim.text().await.as_str(),
-            &self.neovim_selections().await[..],
-            true,
-        )
     }
 
     pub async fn neovim_mode(&mut self) -> Mode {
         self.neovim.mode().await.unwrap()
     }
 
-    async fn neovim_selections(&mut self) -> Vec<Range<usize>> {
-        let neovim_selections = self.neovim.selections().await;
-        neovim_selections
-            .into_iter()
-            .map(|selection| selection.to_offset(&self.buffer_snapshot()))
-            .collect()
-    }
-
     pub async fn assert_state_matches(&mut self) {
         self.is_dirty = false;
         let neovim = self.neovim_state().await;

crates/vim/src/test/neovim_connection.rs

@@ -1,9 +1,9 @@
+use std::path::PathBuf;
 #[cfg(feature = "neovim")]
 use std::{
     cmp,
-    ops::{Deref, DerefMut},
+    ops::{Deref, DerefMut, Range},
 };
-use std::{ops::Range, path::PathBuf};
 
 #[cfg(feature = "neovim")]
 use async_compat::Compat;
@@ -12,6 +12,7 @@ use async_trait::async_trait;
 #[cfg(feature = "neovim")]
 use gpui::keymap_matcher::Keystroke;
 
+#[cfg(feature = "neovim")]
 use language::Point;
 
 #[cfg(feature = "neovim")]
@@ -296,7 +297,7 @@ impl NeovimConnection {
     }
 
     #[cfg(feature = "neovim")]
-    pub async fn state(&mut self) -> (Option<Mode>, String, Vec<Range<Point>>) {
+    pub async fn state(&mut self) -> (Option<Mode>, String) {
         let nvim_buffer = self
             .nvim
             .get_current_buf()
@@ -405,37 +406,33 @@ impl NeovimConnection {
                 .push(Point::new(selection_row, selection_col)..Point::new(cursor_row, cursor_col)),
         }
 
+        let ranges = encode_ranges(&text, &selections);
         let state = NeovimData::Get {
             mode,
-            state: encode_ranges(&text, &selections),
+            state: ranges.clone(),
         };
 
         if self.data.back() != Some(&state) {
             self.data.push_back(state.clone());
         }
 
-        (mode, text, selections)
+        (mode, ranges)
     }
 
     #[cfg(not(feature = "neovim"))]
-    pub async fn state(&mut self) -> (Option<Mode>, String, Vec<Range<Point>>) {
+    pub async fn state(&mut self) -> (Option<Mode>, String) {
-        if let Some(NeovimData::Get { state: text, mode }) = self.data.front() {
+        if let Some(NeovimData::Get { state: raw, mode }) = self.data.front() {
-            let (text, ranges) = parse_state(text);
+            (*mode, raw.to_string())
-            (*mode, text, ranges)
         } else {
             panic!("operation does not match recorded script. re-record with --features=neovim");
         }
     }
 
-    pub async fn selections(&mut self) -> Vec<Range<Point>> {
-        self.state().await.2
-    }
-
     pub async fn mode(&mut self) -> Option<Mode> {
         self.state().await.0
     }
 
-    pub async fn text(&mut self) -> String {
+    pub async fn marked_text(&mut self) -> String {
         self.state().await.1
     }
 
@@ -527,6 +524,7 @@ impl Handler for NvimHandler {
     }
 }
 
+#[cfg(feature = "neovim")]
 fn parse_state(marked_text: &str) -> (String, Vec<Range<Point>>) {
     let (text, ranges) = util::test::marked_text_ranges(marked_text, true);
     let point_ranges = ranges

crates/vim/src/visual.rs

@@ -1,5 +1,5 @@
 use anyhow::Result;
-use std::{cmp, sync::Arc};
+use std::sync::Arc;
 
 use collections::HashMap;
 use editor::{
@@ -263,21 +263,13 @@ pub fn visual_object(object: Object, cx: &mut WindowContext) {
 
                         if let Some(range) = object.range(map, head, around) {
                             if !range.is_empty() {
-                                let expand_both_ways =
+                                let expand_both_ways = object.always_expands_both_ways()
-                                    if object.always_expands_both_ways() || selection.is_empty() {
+                                    || selection.is_empty()
-                                        true
+                                    || movement::right(map, selection.start) == selection.end;
-                                        // contains only one character
-                                    } else if let Some((_, start)) =
-                                        map.reverse_chars_at(selection.end).next()
-                                    {
-                                        selection.start == start
-                                    } else {
-                                        false
-                                    };
 
                                 if expand_both_ways {
-                                    selection.start = cmp::min(selection.start, range.start);
+                                    selection.start = range.start;
-                                    selection.end = cmp::max(selection.end, range.end);
+                                    selection.end = range.end;
                                 } else if selection.reversed {
                                     selection.start = range.start;
                                 } else {

crates/vim/test_data/test_delete_e.json

@@ -1,11 +1,3 @@
-{"Put":{"state":"Teˇst Test"}}
-{"Key":"d"}
-{"Key":"e"}
-{"Get":{"state":"Teˇ Test","mode":"Normal"}}
-{"Put":{"state":"Tˇest test"}}
-{"Key":"d"}
-{"Key":"e"}
-{"Get":{"state":"Tˇ test","mode":"Normal"}}
 {"Put":{"state":"Test teˇst\ntest"}}
 {"Key":"d"}
 {"Key":"e"}

crates/vim/test_data/test_delete_next_word_end.json

@@ -0,0 +1,12 @@
+{"Put":{"state":"Test teˇst\ntest"}}
+{"Key":"d"}
+{"Key":"e"}
+{"Get":{"state":"Test tˇe\ntest","mode":"Normal"}}
+{"Put":{"state":"Test tesˇt\ntest"}}
+{"Key":"d"}
+{"Key":"e"}
+{"Get":{"state":"Test teˇs","mode":"Normal"}}
+{"Put":{"state":"Test teˇst-test test"}}
+{"Key":"d"}
+{"Key":"shift-e"}
+{"Get":{"state":"Test teˇ test","mode":"Normal"}}

crates/vim/test_data/test_multiline_surrounding_character_objects.json

@@ -8,3 +8,8 @@
 {"Key":"i"}
 {"Key":"{"}
 {"Get":{"state":"func empty(a string) bool {\n     if a == \"\" {\n«         return true\nˇ»     }\n     return false\n}","mode":"Visual"}}
+{"Put":{"state":"func empty(a string) bool {\n     if a == \"\" {\n         ˇreturn true\n     }\n     return false\n}"}}
+{"Key":"v"}
+{"Key":"i"}
+{"Key":"{"}
+{"Get":{"state":"func empty(a string) bool {\n     if a == \"\" {\n«         return true\nˇ»     }\n     return false\n}","mode":"Visual"}}

crates/vim/test_data/test_singleline_surrounding_character_objects.json

@@ -0,0 +1,27 @@
+{"SetOption":{"value":"wrap"}}
+{"SetOption":{"value":"columns=12"}}
+{"Put":{"state":"helˇlo \"world\"!"}}
+{"Key":"v"}
+{"Key":"i"}
+{"Key":"\""}
+{"Get":{"state":"hello \"«worldˇ»\"!","mode":"Visual"}}
+{"Put":{"state":"hello \"wˇorld\"!"}}
+{"Key":"v"}
+{"Key":"i"}
+{"Key":"\""}
+{"Get":{"state":"hello \"«worldˇ»\"!","mode":"Visual"}}
+{"Put":{"state":"hello \"wˇorld\"!"}}
+{"Key":"v"}
+{"Key":"a"}
+{"Key":"\""}
+{"Get":{"state":"hello« \"world\"ˇ»!","mode":"Visual"}}
+{"Put":{"state":"hello \"wˇorld\" !"}}
+{"Key":"v"}
+{"Key":"a"}
+{"Key":"\""}
+{"Get":{"state":"hello «\"world\" ˇ»!","mode":"Visual"}}
+{"Put":{"state":"hello \"wˇorld\"•\ngoodbye"}}
+{"Key":"v"}
+{"Key":"a"}
+{"Key":"\""}
+{"Get":{"state":"hello «\"world\" ˇ»\ngoodbye","mode":"Visual"}}