editor: Defer the effects of change_selections to end of transact (#31731)

In quite a few places the selection is changed multiple times in a
transaction. For example, `backspace` might do it 3 times:

* `select_autoclose_pair`
* selection of the ranges to delete
* `insert` of empty string also updates selection

Before this change, each of these selection changes appended to
selection history and did a bunch of work that's only relevant to
selections the user actually sees. So for each backspace,
`editor::UndoSelection` would need to be invoked 3-4 times before the
cursor actually moves. It still needs to be run twice after this change,
but that is a separate issue.

Signature help even had a `backspace_pressed: bool` as an incomplete
workaround, to avoid it flickering due to the selection switching
between being a range and being cursor-like.

The original motivation for this change is work I'm doing on not
re-querying completions when the language server provides a response
that has `is_incomplete: false`. Whether the menu is still visible is
determined by the cursor position, and this was complicated by it seeing
`backspace` temporarily moving the head of the selection 1 character to
the left.

This change also removes some redundant uses of
`push_to_selection_history`.

Not super stoked with the name `DeferredSelectionEffectsState`. Naming
is hard.

Release Notes:

- N/A

crates/editor/src/editor.rs

@@ -936,6 +936,8 @@ pub struct Editor {
     select_next_state: Option<SelectNextState>,
     select_prev_state: Option<SelectNextState>,
     selection_history: SelectionHistory,
+    defer_selection_effects: bool,
+    deferred_selection_effects_state: Option<DeferredSelectionEffectsState>,
     autoclose_regions: Vec<AutocloseRegion>,
     snippet_stack: InvalidationStack<SnippetState>,
     select_syntax_node_history: SelectSyntaxNodeHistory,
@@ -1195,6 +1197,14 @@ impl Default for SelectionHistoryMode {
     }
 }
 
+struct DeferredSelectionEffectsState {
+    changed: bool,
+    show_completions: bool,
+    autoscroll: Option<Autoscroll>,
+    old_cursor_position: Anchor,
+    history_entry: SelectionHistoryEntry,
+}
+
 #[derive(Default)]
 struct SelectionHistory {
     #[allow(clippy::type_complexity)]
@@ -1791,6 +1801,8 @@ impl Editor {
             select_next_state: None,
             select_prev_state: None,
             selection_history: SelectionHistory::default(),
+            defer_selection_effects: false,
+            deferred_selection_effects_state: None,
             autoclose_regions: Vec::new(),
             snippet_stack: InvalidationStack::default(),
             select_syntax_node_history: SelectSyntaxNodeHistory::default(),
@@ -2954,6 +2966,9 @@ impl Editor {
         Subscription::join(other_subscription, this_subscription)
     }
 
+    /// Changes selections using the provided mutation function. Changes to `self.selections` occur
+    /// immediately, but when run within `transact` or `with_selection_effects_deferred` other
+    /// effects of selection change occur at the end of the transaction.
     pub fn change_selections<R>(
         &mut self,
         autoscroll: Option<Autoscroll>,
@@ -2961,39 +2976,105 @@ impl Editor {
         cx: &mut Context<Self>,
         change: impl FnOnce(&mut MutableSelectionsCollection<'_>) -> R,
     ) -> R {
-        self.change_selections_inner(autoscroll, true, window, cx, change)
+        self.change_selections_inner(true, autoscroll, window, cx, change)
     }
 
-    fn change_selections_inner<R>(
+    pub(crate) fn change_selections_without_showing_completions<R>(
         &mut self,
         autoscroll: Option<Autoscroll>,
-        request_completions: bool,
         window: &mut Window,
         cx: &mut Context<Self>,
         change: impl FnOnce(&mut MutableSelectionsCollection<'_>) -> R,
     ) -> R {
-        let old_cursor_position = self.selections.newest_anchor().head();
+        self.change_selections_inner(false, autoscroll, window, cx, change)
-        self.push_to_selection_history();
+    }
 
+    fn change_selections_inner<R>(
+        &mut self,
+        show_completions: bool,
+        autoscroll: Option<Autoscroll>,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+        change: impl FnOnce(&mut MutableSelectionsCollection<'_>) -> R,
+    ) -> R {
+        if let Some(state) = &mut self.deferred_selection_effects_state {
+            state.autoscroll = autoscroll.or(state.autoscroll);
+            state.show_completions = show_completions;
+            let (changed, result) = self.selections.change_with(cx, change);
+            state.changed |= changed;
+            return result;
+        }
+        let mut state = DeferredSelectionEffectsState {
+            changed: false,
+            show_completions,
+            autoscroll,
+            old_cursor_position: self.selections.newest_anchor().head(),
+            history_entry: SelectionHistoryEntry {
+                selections: self.selections.disjoint_anchors(),
+                select_next_state: self.select_next_state.clone(),
+                select_prev_state: self.select_prev_state.clone(),
+                add_selections_state: self.add_selections_state.clone(),
+            },
+        };
         let (changed, result) = self.selections.change_with(cx, change);
+        state.changed = state.changed || changed;
+        if self.defer_selection_effects {
+            self.deferred_selection_effects_state = Some(state);
+        } else {
+            self.apply_selection_effects(state, window, cx);
+        }
+        result
+    }
 
-        if changed {
+    /// Defers the effects of selection change, so that the effects of multiple calls to
-            if let Some(autoscroll) = autoscroll {
+    /// `change_selections` are applied at the end. This way these intermediate states aren't added
+    /// to selection history and the state of popovers based on selection position aren't
+    /// erroneously updated.
+    pub fn with_selection_effects_deferred<R>(
+        &mut self,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+        update: impl FnOnce(&mut Self, &mut Window, &mut Context<Self>) -> R,
+    ) -> R {
+        let already_deferred = self.defer_selection_effects;
+        self.defer_selection_effects = true;
+        let result = update(self, window, cx);
+        if !already_deferred {
+            self.defer_selection_effects = false;
+            if let Some(state) = self.deferred_selection_effects_state.take() {
+                self.apply_selection_effects(state, window, cx);
+            }
+        }
+        result
+    }
+
+    fn apply_selection_effects(
+        &mut self,
+        state: DeferredSelectionEffectsState,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        if state.changed {
+            self.selection_history.push(state.history_entry);
+
+            if let Some(autoscroll) = state.autoscroll {
                 self.request_autoscroll(autoscroll, cx);
             }
-            self.selections_did_change(true, &old_cursor_position, request_completions, window, cx);
 
-            if self.should_open_signature_help_automatically(
+            let old_cursor_position = &state.old_cursor_position;
+
+            self.selections_did_change(
+                true,
                 &old_cursor_position,
-                self.signature_help_state.backspace_pressed(),
+                state.show_completions,
+                window,
                 cx,
-            ) {
+            );
+
+            if self.should_open_signature_help_automatically(&old_cursor_position, cx) {
                 self.show_signature_help(&ShowSignatureHelp, window, cx);
             }
-            self.signature_help_state.set_backspace_pressed(false);
         }
-
-        result
     }
 
     pub fn edit<I, S, T>(&mut self, edits: I, cx: &mut Context<Self>)
@@ -3877,9 +3958,12 @@ impl Editor {
             }
 
             let had_active_inline_completion = this.has_active_inline_completion();
-            this.change_selections_inner(Some(Autoscroll::fit()), false, window, cx, |s| {
+            this.change_selections_without_showing_completions(
-                s.select(new_selections)
+                Some(Autoscroll::fit()),
-            });
+                window,
+                cx,
+                |s| s.select(new_selections),
+            );
 
             if !bracket_inserted {
                 if let Some(on_type_format_task) =
@@ -9033,7 +9117,6 @@ impl Editor {
                 }
             }
 
-            this.signature_help_state.set_backspace_pressed(true);
             this.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
                 s.select(selections)
             });
@@ -12755,7 +12838,6 @@ impl Editor {
     ) -> Result<()> {
         self.hide_mouse_cursor(&HideMouseCursorOrigin::MovementAction);
 
-        self.push_to_selection_history();
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
 
         self.select_next_match_internal(&display_map, false, None, window, cx)?;
@@ -12808,7 +12890,6 @@ impl Editor {
         cx: &mut Context<Self>,
     ) -> Result<()> {
         self.hide_mouse_cursor(&HideMouseCursorOrigin::MovementAction);
-        self.push_to_selection_history();
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
         self.select_next_match_internal(
             &display_map,
@@ -12827,7 +12908,6 @@ impl Editor {
         cx: &mut Context<Self>,
     ) -> Result<()> {
         self.hide_mouse_cursor(&HideMouseCursorOrigin::MovementAction);
-        self.push_to_selection_history();
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
         let buffer = &display_map.buffer_snapshot;
         let mut selections = self.selections.all::<usize>(cx);
@@ -15697,24 +15777,17 @@ impl Editor {
         self.selections_did_change(false, &old_cursor_position, true, window, cx);
     }
 
-    fn push_to_selection_history(&mut self) {
-        self.selection_history.push(SelectionHistoryEntry {
-            selections: self.selections.disjoint_anchors(),
-            select_next_state: self.select_next_state.clone(),
-            select_prev_state: self.select_prev_state.clone(),
-            add_selections_state: self.add_selections_state.clone(),
-        });
-    }
-
     pub fn transact(
         &mut self,
         window: &mut Window,
         cx: &mut Context<Self>,
         update: impl FnOnce(&mut Self, &mut Window, &mut Context<Self>),
     ) -> Option<TransactionId> {
-        self.start_transaction_at(Instant::now(), window, cx);
+        self.with_selection_effects_deferred(window, cx, |this, window, cx| {
-        update(self, window, cx);
+            this.start_transaction_at(Instant::now(), window, cx);
-        self.end_transaction_at(Instant::now(), cx)
+            update(this, window, cx);
+            this.end_transaction_at(Instant::now(), cx)
+        })
     }
 
     pub fn start_transaction_at(

crates/editor/src/jsx_tag_auto_close.rs

@@ -600,7 +600,7 @@ pub(crate) fn handle_from(
                     })
                     .collect::<Vec<_>>();
                 this.update_in(cx, |this, window, cx| {
-                    this.change_selections_inner(None, false, window, cx, |s| {
+                    this.change_selections_without_showing_completions(None, window, cx, |s| {
                         s.select(base_selections);
                     });
                 })

crates/editor/src/signature_help.rs

@@ -74,8 +74,6 @@ impl Editor {
     pub(super) fn should_open_signature_help_automatically(
         &mut self,
         old_cursor_position: &Anchor,
-        backspace_pressed: bool,
-
         cx: &mut Context<Self>,
     ) -> bool {
         if !(self.signature_help_state.is_shown() || self.auto_signature_help_enabled(cx)) {
@@ -84,9 +82,7 @@ impl Editor {
         let newest_selection = self.selections.newest::<usize>(cx);
         let head = newest_selection.head();
 
-        // There are two cases where the head and tail of a selection are different: selecting multiple ranges and using backspace.
+        if !newest_selection.is_empty() && head != newest_selection.tail() {
-        // If we don’t exclude the backspace case, signature_help will blink every time backspace is pressed, so we need to prevent this.
-        if !newest_selection.is_empty() && !backspace_pressed && head != newest_selection.tail() {
             self.signature_help_state
                 .hide(SignatureHelpHiddenBy::Selection);
             return false;
@@ -232,7 +228,6 @@ pub struct SignatureHelpState {
     task: Option<Task<()>>,
     popover: Option<SignatureHelpPopover>,
     hidden_by: Option<SignatureHelpHiddenBy>,
-    backspace_pressed: bool,
 }
 
 impl SignatureHelpState {
@@ -254,14 +249,6 @@ impl SignatureHelpState {
         self.popover.as_mut()
     }
 
-    pub fn backspace_pressed(&self) -> bool {
-        self.backspace_pressed
-    }
-
-    pub fn set_backspace_pressed(&mut self, backspace_pressed: bool) {
-        self.backspace_pressed = backspace_pressed;
-    }
-
     pub fn set_popover(&mut self, popover: SignatureHelpPopover) {
         self.popover = Some(popover);
         self.hidden_by = None;