From 40a2f1e458800195bd6b4616b591eaa30e53ce6e Mon Sep 17 00:00:00 2001 From: dino Date: Tue, 28 Oct 2025 16:14:17 +0000 Subject: [PATCH] refactor(vim): remove calls to around and whitespace 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. --- 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(-) diff --git a/crates/vim/src/helix/object.rs b/crates/vim/src/helix/object.rs index 798cd7162eb58a56c7362aa5fdcb37a33d48daa8..e1bff0fdf67c6e627462630f7136e2d2d7fa88b4 100644 --- a/crates/vim/src/helix/object.rs +++ b/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, - around: bool, + scope: &ObjectScope, ) -> Result>, 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) } diff --git a/crates/vim/src/helix/select.rs b/crates/vim/src/helix/select.rs index 844402aa5ca2e429e7c66c953ff1d84f1eb9345d..6a8493924fb3a81bd19d0fffbaaf98cf1b85f410 100644 --- a/crates/vim/src/helix/select.rs +++ b/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) diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index 1a617e36c18ffa52906cac06d4b9eddb11a91f8e..38c2ee3412506ec9e4071ead1f8f3e7d18aab175 100644 --- a/crates/vim/src/motion.rs +++ b/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 { - 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); diff --git a/crates/vim/src/object.rs b/crates/vim/src/object.rs index 5307af70d414a9738848dbaca46297089276e0d9..bfc8036f35a194624ad071e737049ef9a38b4971 100644 --- a/crates/vim/src/object.rs +++ b/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> { + 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> { - 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> { - 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> { 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, - around: bool, + scope: &ObjectScope, ) -> Option> { fn read_tag(chars: impl Iterator) -> 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> { 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> { + 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> { 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> { 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> { 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 { diff --git a/crates/vim/src/state.rs b/crates/vim/src/state.rs index 15d3867a0197df02e1dc816c6859c3e5dcbe1621..4f3b7a57d5eba1e56a529a724d32db118485ce8b 100644 --- a/crates/vim/src/state.rs +++ b/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 { diff --git a/crates/vim/src/surrounds.rs b/crates/vim/src/surrounds.rs index 2dcbfaa62949af9e000f467099b259388839d456..5ba557d7d9e811af5f64655f969fb0c005601730 100644 --- a/crates/vim/src/surrounds.rs +++ b/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,