Add multi selection support to UnwrapSyntaxNode (#35991)

Gregor created 3 months ago

Closes #35932
Closes #35933

I only intended to fix multi select in this, I accidentally drive-by
fixed the VIM issue as well. `replace_text_in_range` which I was using
before has two, to me unexpected, side-effects:
- it no-ops when input is disabled, which is the case in VIM's
Insert/Visual modes
- it takes the current selection into account, and does not just operate
on the given range (which I erroneously assumed before)

Now the code is using `buffer.edit` instead, which seems more lower
level, and does not have those side-effects. I was enthused to see that
it accepts a vec of edits, so I didn't have to calculate offsets for
following edits... until I also wanted to set selections, where I do
need to do it by hand. I'm still wondering if there is a simpler way to
do it, but for now it at least passes my muster

Release Notes:

- Added multiple selection support to UnwrapSyntaxNode action
- Fixed UnwrapSyntaxNode not working in VIM Insert/Visual modes

Change summary

crates/editor/src/editor.rs       | 85 +++++++++++++++++---------------
crates/editor/src/editor_tests.rs | 17 +-----
2 files changed, 50 insertions(+), 52 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -14834,15 +14834,18 @@ impl Editor {
         self.hide_mouse_cursor(HideMouseCursorOrigin::MovementAction, cx);
 
         let buffer = self.buffer.read(cx).snapshot(cx);
-        let old_selections: Box<[_]> = self.selections.all::<usize>(cx).into();
+        let selections = self
+            .selections
+            .all::<usize>(cx)
+            .into_iter()
+            // subtracting the offset requires sorting
+            .sorted_by_key(|i| i.start);
 
-        let edits = old_selections
-            .iter()
-            // only consider the first selection for now
-            .take(1)
-            .map(|selection| {
+        let full_edits = selections
+            .into_iter()
+            .filter_map(|selection| {
                 // Only requires two branches once if-let-chains stabilize (#53667)
-                let selection_range = if !selection.is_empty() {
+                let child = if !selection.is_empty() {
                     selection.range()
                 } else if let Some((_, ancestor_range)) =
                     buffer.syntax_ancestor(selection.start..selection.end)
@@ -14855,48 +14858,52 @@ impl Editor {
                     selection.range()
                 };
 
-                let mut new_range = selection_range.clone();
-                while let Some((_, ancestor_range)) = buffer.syntax_ancestor(new_range.clone()) {
-                    new_range = match ancestor_range {
+                let mut parent = child.clone();
+                while let Some((_, ancestor_range)) = buffer.syntax_ancestor(parent.clone()) {
+                    parent = match ancestor_range {
                         MultiOrSingleBufferOffsetRange::Single(range) => range,
                         MultiOrSingleBufferOffsetRange::Multi(range) => range,
                     };
-                    if new_range.start < selection_range.start
-                        || new_range.end > selection_range.end
-                    {
+                    if parent.start < child.start || parent.end > child.end {
                         break;
                     }
                 }
 
-                (selection, selection_range, new_range)
+                if parent == child {
+                    return None;
+                }
+                let text = buffer.text_for_range(child.clone()).collect::<String>();
+                Some((selection.id, parent, text))
             })
             .collect::<Vec<_>>();
 
-        self.transact(window, cx, |editor, window, cx| {
-            for (_, child, parent) in &edits {
-                let text = buffer.text_for_range(child.clone()).collect::<String>();
-                editor.replace_text_in_range(Some(parent.clone()), &text, window, cx);
-            }
-
-            editor.change_selections(
-                SelectionEffects::scroll(Autoscroll::fit()),
-                window,
-                cx,
-                |s| {
-                    s.select(
-                        edits
-                            .iter()
-                            .map(|(s, old, new)| Selection {
-                                id: s.id,
-                                start: new.start,
-                                end: new.start + old.len(),
-                                goal: SelectionGoal::None,
-                                reversed: s.reversed,
-                            })
-                            .collect(),
-                    );
-                },
-            );
+        self.transact(window, cx, |this, window, cx| {
+            this.buffer.update(cx, |buffer, cx| {
+                buffer.edit(
+                    full_edits
+                        .iter()
+                        .map(|(_, p, t)| (p.clone(), t.clone()))
+                        .collect::<Vec<_>>(),
+                    None,
+                    cx,
+                );
+            });
+            this.change_selections(Default::default(), window, cx, |s| {
+                let mut offset = 0;
+                let mut selections = vec![];
+                for (id, parent, text) in full_edits {
+                    let start = parent.start - offset;
+                    offset += parent.len() - text.len();
+                    selections.push(Selection {
+                        id: id,
+                        start,
+                        end: start + text.len(),
+                        reversed: false,
+                        goal: Default::default(),
+                    });
+                }
+                s.select(selections);
+            });
         });
     }
 

crates/editor/src/editor_tests.rs

@@ -8015,7 +8015,7 @@ async fn test_select_larger_smaller_syntax_node_for_string(cx: &mut TestAppConte
 }
 
 #[gpui::test]
-async fn test_unwrap_syntax_node(cx: &mut gpui::TestAppContext) {
+async fn test_unwrap_syntax_nodes(cx: &mut gpui::TestAppContext) {
     init_test(cx, |_| {});
 
     let mut cx = EditorTestContext::new(cx).await;
@@ -8029,21 +8029,12 @@ async fn test_unwrap_syntax_node(cx: &mut gpui::TestAppContext) {
         buffer.set_language(Some(language), cx);
     });
 
-    cx.set_state(
-        &r#"
-            use mod1::mod2::{«mod3ˇ», mod4};
-        "#
-        .unindent(),
-    );
+    cx.set_state(indoc! { r#"use mod1::{mod2::{«mod3ˇ», mod4}, mod5::{mod6, «mod7ˇ»}};"# });
     cx.update_editor(|editor, window, cx| {
         editor.unwrap_syntax_node(&UnwrapSyntaxNode, window, cx);
     });
-    cx.assert_editor_state(
-        &r#"
-            use mod1::mod2::«mod3ˇ»;
-        "#
-        .unindent(),
-    );
+
+    cx.assert_editor_state(indoc! { r#"use mod1::{mod2::«mod3ˇ», mod5::«mod7ˇ»};"# });
 }
 
 #[gpui::test]