refactor(vim): remove calls to around and whitespace

dino created 1 week ago

Remove `ObjectScope.around` and `ObjectScope.inside`, ensuring that
pretty much all object related functions are actually leveraging
`ObjectScope` instead of the `around` and `whitespace` booleans.

In some rare cases the `around` boolean was kept because it made more
sense in the function's context to do so, as it'll likely never need the
`whitespace` argument, but this is something we can refactor as we work
more on the codebase.

Change summary

crates/vim/src/helix/object.rs |  11 +
crates/vim/src/helix/select.rs |   2 
crates/vim/src/motion.rs       |   6 
crates/vim/src/object.rs       | 204 +++++++++++++----------------------
crates/vim/src/state.rs        |  19 ---
crates/vim/src/surrounds.rs    |   3 
6 files changed, 89 insertions(+), 156 deletions(-)

Detailed changes

crates/vim/src/helix/object.rs

@@ -10,6 +10,7 @@ use text::Selection;
 use crate::{
     helix::boundary::{FuzzyBoundary, ImmediateBoundary},
     object::Object as VimObject,
+    state::ObjectScope,
 };
 
 /// A text object from helix or an extra one
@@ -43,11 +44,17 @@ impl VimObject {
         self,
         map: &DisplaySnapshot,
         selection: Selection<DisplayPoint>,
-        around: bool,
+        scope: &ObjectScope,
     ) -> Result<Option<Range<DisplayPoint>>, VimToHelixError> {
         let cursor = cursor_range(&selection, map);
         if let Some(helix_object) = self.to_helix_object() {
-            Ok(helix_object.range(map, cursor, around))
+            // TODO!: Does it make sense to update the trait so as to work on
+            // `ObjectScope` instead of the `around` bool?
+            Ok(helix_object.range(
+                map,
+                cursor,
+                matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed),
+            ))
         } else {
             Err(VimToHelixError)
         }

crates/vim/src/helix/select.rs

@@ -18,7 +18,7 @@ impl Vim {
             editor.change_selections(Default::default(), window, cx, |s| {
                 s.move_with(|map, selection| {
                     let Some(range) = object
-                        .helix_range(map, selection.clone(), scope.around())
+                        .helix_range(map, selection.clone(), &scope)
                         .unwrap_or({
                             let vim_range = object.range(map, selection.clone(), &scope, None);
                             vim_range.filter(|r| r.start <= cursor_range(selection, map).start)

crates/vim/src/motion.rs

@@ -16,7 +16,7 @@ use workspace::searchable::Direction;
 use crate::{
     Vim,
     normal::mark,
-    state::{Mode, Operator},
+    state::{Mode, ObjectScope, Operator},
     surrounds::SurroundsType,
 };
 
@@ -2336,8 +2336,8 @@ fn end_of_document(
 }
 
 fn matching_tag(map: &DisplaySnapshot, head: DisplayPoint) -> Option<DisplayPoint> {
-    let inner = crate::object::surrounding_html_tag(map, head, head..head, false)?;
-    let outer = crate::object::surrounding_html_tag(map, head, head..head, true)?;
+    let inner = crate::object::surrounding_html_tag(map, head, head..head, &ObjectScope::Inside)?;
+    let outer = crate::object::surrounding_html_tag(map, head, head..head, &ObjectScope::Around)?;
 
     if head > outer.start && head < inner.start {
         let mut offset = inner.end.to_offset(map, Bias::Left);

crates/vim/src/object.rs

@@ -204,9 +204,10 @@ impl DelimiterRange {
 fn find_mini_delimiters(
     map: &DisplaySnapshot,
     display_point: DisplayPoint,
-    around: bool,
+    scope: &ObjectScope,
     is_valid_delimiter: &DelimiterPredicate,
 ) -> Option<Range<DisplayPoint>> {
+    let around = matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed);
     let point = map.clip_at_line_end(display_point).to_point(map);
     let offset = point.to_offset(&map.buffer_snapshot());
 
@@ -278,17 +279,17 @@ fn is_bracket_delimiter(buffer: &BufferSnapshot, start: usize, _end: usize) -> b
 fn find_mini_quotes(
     map: &DisplaySnapshot,
     display_point: DisplayPoint,
-    around: bool,
+    scope: &ObjectScope,
 ) -> Option<Range<DisplayPoint>> {
-    find_mini_delimiters(map, display_point, around, &is_quote_delimiter)
+    find_mini_delimiters(map, display_point, scope, &is_quote_delimiter)
 }
 
 fn find_mini_brackets(
     map: &DisplaySnapshot,
     display_point: DisplayPoint,
-    around: bool,
+    scope: &ObjectScope,
 ) -> Option<Range<DisplayPoint>> {
-    find_mini_delimiters(map, display_point, around, &is_bracket_delimiter)
+    find_mini_delimiters(map, display_point, scope, &is_bracket_delimiter)
 }
 
 actions!(
@@ -554,41 +555,27 @@ impl Object {
     ) -> Option<Range<DisplayPoint>> {
         let relative_to = selection.head();
         match self {
-            Object::Word { ignore_punctuation } => {
-                if scope.around() {
+            Object::Word { ignore_punctuation } => match scope {
+                ObjectScope::Around | ObjectScope::AroundTrimmed => {
                     around_word(map, relative_to, ignore_punctuation)
-                } else {
-                    in_word(map, relative_to, ignore_punctuation)
                 }
-            }
-            Object::Subword { ignore_punctuation } => {
-                if scope.around() {
+                ObjectScope::Inside => in_word(map, relative_to, ignore_punctuation),
+            },
+            Object::Subword { ignore_punctuation } => match scope {
+                ObjectScope::Around | ObjectScope::AroundTrimmed => {
                     around_subword(map, relative_to, ignore_punctuation)
-                } else {
-                    in_subword(map, relative_to, ignore_punctuation)
                 }
-            }
-            Object::Sentence => sentence(map, relative_to, scope.around()),
+                ObjectScope::Inside => in_subword(map, relative_to, ignore_punctuation),
+            },
+            Object::Sentence => sentence(map, relative_to, scope),
             //change others later
-            Object::Paragraph => paragraph(map, relative_to, scope.around(), times.unwrap_or(1)),
-            Object::Quotes => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '\'',
-                '\'',
-            ),
-            Object::BackQuotes => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '`',
-                '`',
-            ),
+            Object::Paragraph => paragraph(map, relative_to, scope, times.unwrap_or(1)),
+            Object::Quotes => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '\'', '\'')
+            }
+            Object::BackQuotes => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '`', '`')
+            }
             Object::AnyQuotes => {
                 let quote_types = ['\'', '"', '`'];
                 let cursor_offset = relative_to.to_offset(map, Bias::Left);
@@ -602,8 +589,7 @@ impl Object {
                     if let Some(range) = surrounding_markers(
                         map,
                         relative_to,
-                        scope.around(),
-                        scope.whitespace(),
+                        scope,
                         self.is_multiline(),
                         quote,
                         quote,
@@ -632,8 +618,7 @@ impl Object {
                         surrounding_markers(
                             map,
                             relative_to,
-                            scope.around(),
-                            scope.whitespace(),
+                            scope,
                             self.is_multiline(),
                             quote,
                             quote,
@@ -651,38 +636,20 @@ impl Object {
                         }
                     })
             }
-            Object::MiniQuotes => find_mini_quotes(map, relative_to, scope.around()),
-            Object::DoubleQuotes => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '"',
-                '"',
-            ),
-            Object::VerticalBars => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '|',
-                '|',
-            ),
-            Object::Parentheses => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '(',
-                ')',
-            ),
+            Object::MiniQuotes => find_mini_quotes(map, relative_to, scope),
+            Object::DoubleQuotes => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '"', '"')
+            }
+            Object::VerticalBars => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '|', '|')
+            }
+            Object::Parentheses => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '(', ')')
+            }
             Object::Tag => {
                 let head = selection.head();
                 let range = selection.range();
-                surrounding_html_tag(map, head, range, scope.around())
+                surrounding_html_tag(map, head, range, scope)
             }
             Object::AnyBrackets => {
                 let bracket_pairs = [('(', ')'), ('[', ']'), ('{', '}'), ('<', '>')];
@@ -696,8 +663,7 @@ impl Object {
                     if let Some(range) = surrounding_markers(
                         map,
                         relative_to,
-                        scope.around(),
-                        scope.whitespace(),
+                        scope,
                         self.is_multiline(),
                         open,
                         close,
@@ -726,8 +692,7 @@ impl Object {
                         surrounding_markers(
                             map,
                             relative_to,
-                            scope.around(),
-                            scope.whitespace(),
+                            scope,
                             self.is_multiline(),
                             open,
                             close,
@@ -745,65 +710,42 @@ impl Object {
                         }
                     })
             }
-            Object::MiniBrackets => find_mini_brackets(map, relative_to, scope.around()),
-            Object::SquareBrackets => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '[',
-                ']',
-            ),
-            Object::CurlyBrackets => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '{',
-                '}',
-            ),
-            Object::AngleBrackets => surrounding_markers(
-                map,
-                relative_to,
-                scope.around(),
-                scope.whitespace(),
-                self.is_multiline(),
-                '<',
-                '>',
-            ),
+            Object::MiniBrackets => find_mini_brackets(map, relative_to, scope),
+            Object::SquareBrackets => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '[', ']')
+            }
+            Object::CurlyBrackets => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '{', '}')
+            }
+            Object::AngleBrackets => {
+                surrounding_markers(map, relative_to, scope, self.is_multiline(), '<', '>')
+            }
             Object::Method => text_object(
                 map,
                 relative_to,
-                if scope.around() {
-                    TextObject::AroundFunction
-                } else {
-                    TextObject::InsideFunction
+                match scope {
+                    ObjectScope::Around | ObjectScope::AroundTrimmed => TextObject::AroundFunction,
+                    ObjectScope::Inside => TextObject::InsideFunction,
                 },
             ),
             Object::Comment => text_object(
                 map,
                 relative_to,
-                if scope.around() {
-                    TextObject::AroundComment
-                } else {
-                    TextObject::InsideComment
+                match scope {
+                    ObjectScope::Around | ObjectScope::AroundTrimmed => TextObject::AroundComment,
+                    ObjectScope::Inside => TextObject::InsideComment,
                 },
             ),
             Object::Class => text_object(
                 map,
                 relative_to,
-                if scope.around() {
-                    TextObject::AroundClass
-                } else {
-                    TextObject::InsideClass
+                match scope {
+                    ObjectScope::Around | ObjectScope::AroundTrimmed => TextObject::AroundClass,
+                    ObjectScope::Inside => TextObject::InsideClass,
                 },
             ),
-            Object::Argument => argument(map, relative_to, scope.around()),
-            Object::IndentObj { include_below } => {
-                indent(map, relative_to, scope.around(), include_below)
-            }
+            Object::Argument => argument(map, relative_to, scope),
+            Object::IndentObj { include_below } => indent(map, relative_to, scope, include_below),
             Object::EntireFile => entire_file(map),
         }
     }
@@ -914,7 +856,7 @@ pub fn surrounding_html_tag(
     map: &DisplaySnapshot,
     head: DisplayPoint,
     range: Range<DisplayPoint>,
-    around: bool,
+    scope: &ObjectScope,
 ) -> Option<Range<DisplayPoint>> {
     fn read_tag(chars: impl Iterator<Item = char>) -> String {
         chars
@@ -966,7 +908,8 @@ pub fn surrounding_html_tag(
                         && range.end.to_offset(map, Bias::Left) <= last_child.start_byte() + 1
                 };
                 if open_tag.is_some() && open_tag == close_tag && is_valid {
-                    let range = if around {
+                    let range = if matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed)
+                    {
                         first_child.byte_range().start..last_child.byte_range().end
                     } else {
                         first_child.byte_range().end..last_child.byte_range().start
@@ -1167,7 +1110,7 @@ fn text_object(
 fn argument(
     map: &DisplaySnapshot,
     relative_to: DisplayPoint,
-    around: bool,
+    scope: &ObjectScope,
 ) -> Option<Range<DisplayPoint>> {
     let snapshot = &map.buffer_snapshot();
     let offset = relative_to.to_offset(map, Bias::Left);
@@ -1303,7 +1246,9 @@ fn argument(
         Some(start..end)
     }
 
-    let result = comma_delimited_range_at(buffer, excerpt.map_offset_to_buffer(offset), around)?;
+    let include_comma = matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed);
+    let result =
+        comma_delimited_range_at(buffer, excerpt.map_offset_to_buffer(offset), include_comma)?;
 
     if excerpt.contains_buffer_range(result.clone()) {
         let result = excerpt.map_range_from_buffer(result);
@@ -1316,9 +1261,10 @@ fn argument(
 fn indent(
     map: &DisplaySnapshot,
     relative_to: DisplayPoint,
-    around: bool,
+    scope: &ObjectScope,
     include_below: bool,
 ) -> Option<Range<DisplayPoint>> {
+    let around = matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed);
     let point = relative_to.to_point(map);
     let row = point.row;
 
@@ -1368,13 +1314,13 @@ fn indent(
 fn sentence(
     map: &DisplaySnapshot,
     relative_to: DisplayPoint,
-    around: bool,
+    scope: &ObjectScope,
 ) -> Option<Range<DisplayPoint>> {
     let mut start = None;
     let relative_offset = relative_to.to_offset(map, Bias::Left);
     let mut previous_end = relative_offset;
-
     let mut chars = map.buffer_chars_at(previous_end).peekable();
+    let around = matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed);
 
     // Search backwards for the previous sentence end or current sentence start. Include the character under relative_to
     for (char, offset) in chars
@@ -1520,7 +1466,7 @@ pub fn expand_to_include_whitespace(
 fn paragraph(
     map: &DisplaySnapshot,
     relative_to: DisplayPoint,
-    around: bool,
+    scope: &ObjectScope,
     times: usize,
 ) -> Option<Range<DisplayPoint>> {
     let mut paragraph_start = start_of_paragraph(map, relative_to);
@@ -1534,7 +1480,7 @@ fn paragraph(
             .buffer_snapshot()
             .is_line_blank(MultiBufferRow(point.row));
 
-        if around {
+        if matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed) {
             if paragraph_ends_with_eof {
                 if current_line_is_empty {
                     return None;
@@ -1615,13 +1561,13 @@ pub fn end_of_paragraph(map: &DisplaySnapshot, display_point: DisplayPoint) -> D
 pub fn surrounding_markers(
     map: &DisplaySnapshot,
     relative_to: DisplayPoint,
-    around: bool,
-    whitespace: bool,
+    scope: &ObjectScope,
     search_across_lines: bool,
     open_marker: char,
     close_marker: char,
 ) -> Option<Range<DisplayPoint>> {
     let point = relative_to.to_offset(map, Bias::Left);
+    let around = matches!(scope, ObjectScope::Around | ObjectScope::AroundTrimmed);
 
     let mut matched_closes = 0;
     let mut opening = None;
@@ -1730,7 +1676,7 @@ pub fn surrounding_markers(
 
                 // Only update closing range's `end` value if whitespace is
                 // meant to be included.
-                if whitespace {
+                if *scope == ObjectScope::Around {
                     closing.end = range.end;
                 }
             } else {
@@ -1743,7 +1689,7 @@ pub fn surrounding_markers(
                 if ch.is_whitespace() && ch != '\n' {
                     // Only update closing range's `start` value if whitespace
                     // is meant to be included.
-                    if whitespace {
+                    if *scope == ObjectScope::Around {
                         opening.start = range.start
                     }
                 } else {

crates/vim/src/state.rs

@@ -175,25 +175,6 @@ pub(crate) enum ObjectScope {
 }
 
 impl ObjectScope {
-    // TODO!: This is meant to be removed after everything has been migrated to
-    // work with `ObjectScope` directly.
-    pub(crate) fn around(&self) -> bool {
-        match self {
-            ObjectScope::Inside => false,
-            ObjectScope::Around => true,
-            ObjectScope::AroundTrimmed => true,
-        }
-    }
-
-    // TODO!: This is meant to be removed after everything has been migrated to
-    // work with `ObjectScope` directly.
-    pub(crate) fn whitespace(&self) -> bool {
-        match self {
-            ObjectScope::Inside | ObjectScope::AroundTrimmed => false,
-            ObjectScope::Around => true,
-        }
-    }
-
     /// Create the `ObjectScope` from a `PushObject` action, taking into account
     /// its `around` and `whitespace` values.
     pub(crate) fn from_action(action: &PushObject) -> Self {

crates/vim/src/surrounds.rs

@@ -542,8 +542,7 @@ impl Vim {
                             if let Some(range) = surrounding_markers(
                                 &display_map,
                                 relative_to,
-                                true,
-                                true,
+                                &ObjectScope::Around,
                                 false,
                                 open,
                                 close,