From 9cbde74274950b1259087c425a6c0cb22ac4a5c6 Mon Sep 17 00:00:00 2001
From: Hans <weartistt@gmail.com>
Date: Thu, 4 Apr 2024 14:35:26 +0800
Subject: [PATCH] Refactor selection expansion logic into a separate method
 (#10117)

Release Notes:

- N/A

This commit introduces a new method `range` to calculate the target
range for selection expansion based on the current selection, movement
times, and other parameters. The `expand_selection` method is refactored
to use this new `range` method, simplifying the logic for expanding a
selection and making the code more modular and reusable. The `range`
method encapsulates the logic for calculating the new selection range,
including handling linewise selection and adjustments for surrounding
newlines, making it easier to understand and maintain the selection
expansion functionality.

---------

Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
---
 crates/vim/src/motion.rs | 38 ++++++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs
index 08235c03e7c8ab1afbaa444e3a310dfcece6b3c9..500eef28f3fb0bb505054f3a3d0de08a5459ad2b 100644
--- a/crates/vim/src/motion.rs
+++ b/crates/vim/src/motion.rs
@@ -8,6 +8,7 @@ use editor::{
 use gpui::{actions, impl_actions, px, ViewContext, WindowContext};
 use language::{char_kind, CharKind, Point, Selection, SelectionGoal};
 use serde::Deserialize;
+use std::ops::Range;
 use workspace::Workspace;
 
 use crate::{
@@ -707,15 +708,15 @@ impl Motion {
         (new_point != point || infallible).then_some((new_point, goal))
     }
 
-    // Expands a selection using self motion for an operator
-    pub fn expand_selection(
+    // Get the range value after self is applied to the specified selection.
+    pub fn range(
         &self,
         map: &DisplaySnapshot,
-        selection: &mut Selection<DisplayPoint>,
+        selection: Selection<DisplayPoint>,
         times: Option<usize>,
         expand_to_surrounding_newline: bool,
         text_layout_details: &TextLayoutDetails,
-    ) -> bool {
+    ) -> Option<Range<DisplayPoint>> {
         if let Some((new_head, goal)) = self.move_point(
             map,
             selection.head(),
@@ -723,6 +724,7 @@ impl Motion {
             times,
             &text_layout_details,
         ) {
+            let mut selection = selection.clone();
             selection.set_head(new_head, goal);
 
             if self.linewise() {
@@ -734,7 +736,7 @@ impl Motion {
                         *selection.end.column_mut() = 0;
                         selection.end = map.clip_point(selection.end, Bias::Right);
                         // Don't reset the end here
-                        return true;
+                        return Some(selection.start..selection.end);
                     } else if selection.start.row() > 0 {
                         *selection.start.row_mut() -= 1;
                         *selection.start.column_mut() = map.line_len(selection.start.row());
@@ -742,7 +744,7 @@ impl Motion {
                     }
                 }
 
-                (_, selection.end) = map.next_line_boundary(selection.end.to_point(map));
+                selection.end = map.next_line_boundary(selection.end.to_point(map)).1;
             } else {
                 // Another special case: When using the "w" motion in combination with an
                 // operator and the last word moved over is at the end of a line, the end of
@@ -783,6 +785,30 @@ impl Motion {
                     *selection.end.column_mut() += 1;
                 }
             }
+            Some(selection.start..selection.end)
+        } else {
+            None
+        }
+    }
+
+    // Expands a selection using self for an operator
+    pub fn expand_selection(
+        &self,
+        map: &DisplaySnapshot,
+        selection: &mut Selection<DisplayPoint>,
+        times: Option<usize>,
+        expand_to_surrounding_newline: bool,
+        text_layout_details: &TextLayoutDetails,
+    ) -> bool {
+        if let Some(range) = self.range(
+            map,
+            selection.clone(),
+            times,
+            expand_to_surrounding_newline,
+            text_layout_details,
+        ) {
+            selection.start = range.start;
+            selection.end = range.end;
             true
         } else {
             false