Always call set_selections in mouse selection methods

Max Brunsfeld , Antonio Scandurra , and Nathan Sobo created

Co-Authored-By: Antonio Scandurra <me@as-cii.com>
Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

crates/diagnostics/src/diagnostics.rs |   2 
crates/editor/src/editor.rs           | 153 ++++++++++++++--------------
2 files changed, 79 insertions(+), 76 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -490,7 +490,7 @@ impl ProjectDiagnosticsEditor {
                     }
                 }
             }
-            editor.update_selections(selections, None, None, cx);
+            editor.update_selections(selections, None, cx);
             Some(())
         });
 

crates/editor/src/editor.rs 🔗

@@ -443,6 +443,7 @@ pub struct EditorSnapshot {
     scroll_top_anchor: Option<Anchor>,
 }
 
+#[derive(Clone)]
 pub struct PendingSelection {
     selection: Selection<Anchor>,
     mode: SelectMode,
@@ -1164,7 +1165,7 @@ impl Editor {
 
         let position = position.to_offset(&display_map, Bias::Left);
         let tail_anchor = display_map.buffer_snapshot.anchor_before(tail);
-        let pending = self.pending_selection.as_mut().unwrap();
+        let mut pending = self.pending_selection.clone().unwrap();
 
         if position >= tail {
             pending.selection.start = tail_anchor.clone();
@@ -1179,6 +1180,8 @@ impl Editor {
             }
             _ => {}
         }
+
+        self.set_selections(self.selections.clone(), Some(pending), cx);
     }
 
     fn begin_selection(
@@ -1242,20 +1245,23 @@ impl Editor {
             goal: SelectionGoal::None,
         };
 
-        let mut selections = Vec::new();
+        let mut selections;
         if add {
-            selections = self.local_selections(cx);
-            // Remove the newest selection since it was only added as part of this multi-click.
+            selections = self.selections.clone();
+            // Remove the newest selection if it was added due to a previous mouse up
+            // within this multi-click.
             if click_count > 1 {
-                selections.retain(|selection| selection.id != newest_selection.id);
+                selections = self
+                    .selections
+                    .iter()
+                    .filter(|selection| selection.id != newest_selection.id)
+                    .cloned()
+                    .collect();
             }
+        } else {
+            selections = Arc::from([]);
         }
-        self.update_selections::<usize>(
-            selections,
-            Some(PendingSelection { selection, mode }),
-            None,
-            cx,
-        );
+        self.set_selections(selections, Some(PendingSelection { selection, mode }), cx);
 
         cx.notify();
     }
@@ -1298,14 +1304,14 @@ impl Editor {
         if let Some(tail) = self.columnar_selection_tail.as_ref() {
             let tail = tail.to_display_point(&display_map);
             self.select_columns(tail, position, overshoot, &display_map, cx);
-        } else if let Some(PendingSelection { selection, mode }) = self.pending_selection.as_mut() {
+        } else if let Some(mut pending) = self.pending_selection.clone() {
             let buffer = self.buffer.read(cx).snapshot(cx);
             let head;
             let tail;
-            match mode {
+            match &pending.mode {
                 SelectMode::Character => {
                     head = position.to_point(&display_map);
-                    tail = selection.tail().to_point(&buffer);
+                    tail = pending.selection.tail().to_point(&buffer);
                 }
                 SelectMode::Word(original_range) => {
                     let original_display_range = original_range.start.to_display_point(&display_map)
@@ -1361,14 +1367,15 @@ impl Editor {
             };
 
             if head < tail {
-                selection.start = buffer.anchor_before(head);
-                selection.end = buffer.anchor_before(tail);
-                selection.reversed = true;
+                pending.selection.start = buffer.anchor_before(head);
+                pending.selection.end = buffer.anchor_before(tail);
+                pending.selection.reversed = true;
             } else {
-                selection.start = buffer.anchor_before(tail);
-                selection.end = buffer.anchor_before(head);
-                selection.reversed = false;
+                pending.selection.start = buffer.anchor_before(tail);
+                pending.selection.end = buffer.anchor_before(head);
+                pending.selection.reversed = false;
             }
+            self.set_selections(self.selections.clone(), Some(pending), cx);
         } else {
             log::error!("update_selection dispatched with no pending selection");
             return;
@@ -1382,7 +1389,7 @@ impl Editor {
         self.columnar_selection_tail.take();
         if self.pending_selection.is_some() {
             let selections = self.local_selections::<usize>(cx);
-            self.update_selections(selections, None, None, cx);
+            self.update_selections(selections, None, cx);
         }
     }
 
@@ -1422,7 +1429,7 @@ impl Editor {
             })
             .collect::<Vec<_>>();
 
-        self.update_selections(selections, None, None, cx);
+        self.update_selections(selections, None, cx);
         cx.notify();
     }
 
@@ -1456,7 +1463,7 @@ impl Editor {
                 goal: selection.goal,
             };
             if self.local_selections::<Point>(cx).is_empty() {
-                self.update_selections(vec![selection], None, Some(Autoscroll::Fit), cx);
+                self.update_selections(vec![selection], Some(Autoscroll::Fit), cx);
             }
         } else {
             let buffer = self.buffer.read(cx).snapshot(cx);
@@ -1470,7 +1477,7 @@ impl Editor {
                 oldest_selection.start = oldest_selection.head().clone();
                 oldest_selection.end = oldest_selection.head().clone();
             }
-            self.update_selections(vec![oldest_selection], None, Some(Autoscroll::Fit), cx);
+            self.update_selections(vec![oldest_selection], Some(Autoscroll::Fit), cx);
         }
     }
 
@@ -1543,7 +1550,7 @@ impl Editor {
                 }
             })
             .collect::<Vec<_>>();
-        self.update_selections(selections, None, autoscroll, cx);
+        self.update_selections(selections, autoscroll, cx);
     }
 
     #[cfg(any(test, feature = "test-support"))]
@@ -1572,7 +1579,7 @@ impl Editor {
                 }
             })
             .collect();
-        self.update_selections(selections, None, None, cx);
+        self.update_selections(selections, None, cx);
     }
 
     pub fn handle_input(&mut self, action: &Input, cx: &mut ViewContext<Self>) {
@@ -1746,7 +1753,7 @@ impl Editor {
                 })
                 .collect()
         };
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
         self.end_transaction(cx);
     }
 
@@ -1839,7 +1846,7 @@ impl Editor {
         });
 
         if let Some(new_selections) = new_selections {
-            self.update_selections(new_selections, None, None, cx);
+            self.update_selections(new_selections, None, cx);
         }
         if let Some(bracket_pair_state) = bracket_pair_state {
             self.autoclose_stack.push(bracket_pair_state);
@@ -1882,7 +1889,7 @@ impl Editor {
                 })
                 .collect();
             self.autoclose_stack.pop();
-            self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+            self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
             true
         } else {
             false
@@ -2403,7 +2410,7 @@ impl Editor {
                     self.snippet_stack.pop();
                 }
 
-                self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+                self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
                 return true;
             }
             self.snippet_stack.pop();
@@ -2433,7 +2440,7 @@ impl Editor {
                 selection.goal = SelectionGoal::None;
             }
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
         self.insert("", cx);
         self.end_transaction(cx);
     }
@@ -2452,7 +2459,7 @@ impl Editor {
                 selection.goal = SelectionGoal::None;
             }
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
         self.insert(&"", cx);
         self.end_transaction(cx);
     }
@@ -2529,7 +2536,7 @@ impl Editor {
             }
         });
 
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
         self.end_transaction(cx);
     }
 
@@ -2577,7 +2584,6 @@ impl Editor {
 
         self.update_selections(
             self.local_selections::<usize>(cx),
-            None,
             Some(Autoscroll::Fit),
             cx,
         );
@@ -2653,7 +2659,7 @@ impl Editor {
                 }
             })
             .collect();
-        self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
         self.end_transaction(cx);
     }
 
@@ -2806,7 +2812,7 @@ impl Editor {
             }
         });
         self.fold_ranges(refold_ranges, cx);
-        self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
         self.end_transaction(cx);
     }
 
@@ -2909,7 +2915,7 @@ impl Editor {
             }
         });
         self.fold_ranges(refold_ranges, cx);
-        self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
         self.end_transaction(cx);
     }
 
@@ -2939,7 +2945,7 @@ impl Editor {
                 });
             }
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
         self.insert("", cx);
         self.end_transaction(cx);
 
@@ -3026,7 +3032,7 @@ impl Editor {
                         selection.end = selection.start;
                     });
                 }
-                self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+                self.update_selections(selections, Some(Autoscroll::Fit), cx);
             } else {
                 self.insert(clipboard_text, cx);
             }
@@ -3070,7 +3076,7 @@ impl Editor {
             selection.reversed = false;
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_left(&mut self, _: &SelectLeft, cx: &mut ViewContext<Self>) {
@@ -3084,7 +3090,7 @@ impl Editor {
             selection.set_head(cursor);
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn move_right(&mut self, _: &MoveRight, cx: &mut ViewContext<Self>) {
@@ -3106,7 +3112,7 @@ impl Editor {
             selection.reversed = false;
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_right(&mut self, _: &SelectRight, cx: &mut ViewContext<Self>) {
@@ -3120,7 +3126,7 @@ impl Editor {
             selection.set_head(cursor);
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn move_up(&mut self, _: &MoveUp, cx: &mut ViewContext<Self>) {
@@ -3151,7 +3157,7 @@ impl Editor {
             selection.goal = goal;
             selection.reversed = false;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_up(&mut self, _: &SelectUp, cx: &mut ViewContext<Self>) {
@@ -3164,7 +3170,7 @@ impl Editor {
             selection.set_head(cursor);
             selection.goal = goal;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn move_down(&mut self, _: &MoveDown, cx: &mut ViewContext<Self>) {
@@ -3195,7 +3201,7 @@ impl Editor {
             selection.goal = goal;
             selection.reversed = false;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_down(&mut self, _: &SelectDown, cx: &mut ViewContext<Self>) {
@@ -3208,7 +3214,7 @@ impl Editor {
             selection.set_head(cursor);
             selection.goal = goal;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn move_to_previous_word_boundary(
@@ -3226,7 +3232,7 @@ impl Editor {
             selection.reversed = false;
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_to_previous_word_boundary(
@@ -3242,7 +3248,7 @@ impl Editor {
             selection.set_head(cursor);
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn delete_to_previous_word_boundary(
@@ -3262,7 +3268,7 @@ impl Editor {
                 selection.goal = SelectionGoal::None;
             }
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
         self.insert("", cx);
         self.end_transaction(cx);
     }
@@ -3282,7 +3288,7 @@ impl Editor {
             selection.reversed = false;
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_to_next_word_boundary(
@@ -3298,7 +3304,7 @@ impl Editor {
             selection.set_head(cursor);
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn delete_to_next_word_boundary(
@@ -3318,7 +3324,7 @@ impl Editor {
                 selection.goal = SelectionGoal::None;
             }
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
         self.insert("", cx);
         self.end_transaction(cx);
     }
@@ -3339,7 +3345,7 @@ impl Editor {
             selection.reversed = false;
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_to_beginning_of_line(
@@ -3355,7 +3361,7 @@ impl Editor {
             selection.set_head(new_head.to_point(&display_map));
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn delete_to_beginning_of_line(
@@ -3383,7 +3389,7 @@ impl Editor {
                 selection.goal = SelectionGoal::None;
             }
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_to_end_of_line(
@@ -3399,7 +3405,7 @@ impl Editor {
             selection.set_head(new_head.to_point(&display_map));
             selection.goal = SelectionGoal::None;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn delete_to_end_of_line(&mut self, _: &DeleteToEndOfLine, cx: &mut ViewContext<Self>) {
@@ -3429,13 +3435,13 @@ impl Editor {
             reversed: false,
             goal: SelectionGoal::None,
         };
-        self.update_selections(vec![selection], None, Some(Autoscroll::Fit), cx);
+        self.update_selections(vec![selection], Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_to_beginning(&mut self, _: &SelectToBeginning, cx: &mut ViewContext<Self>) {
         let mut selection = self.local_selections::<Point>(cx).last().unwrap().clone();
         selection.set_head(Point::zero());
-        self.update_selections(vec![selection], None, Some(Autoscroll::Fit), cx);
+        self.update_selections(vec![selection], Some(Autoscroll::Fit), cx);
     }
 
     pub fn move_to_end(&mut self, _: &MoveToEnd, cx: &mut ViewContext<Self>) {
@@ -3452,7 +3458,7 @@ impl Editor {
             reversed: false,
             goal: SelectionGoal::None,
         };
-        self.update_selections(vec![selection], None, Some(Autoscroll::Fit), cx);
+        self.update_selections(vec![selection], Some(Autoscroll::Fit), cx);
     }
 
     pub fn set_nav_history(&mut self, nav_history: Option<ItemNavHistory>) {
@@ -3492,7 +3498,7 @@ impl Editor {
     pub fn select_to_end(&mut self, _: &SelectToEnd, cx: &mut ViewContext<Self>) {
         let mut selection = self.local_selections::<usize>(cx).first().unwrap().clone();
         selection.set_head(self.buffer.read(cx).read(cx).len());
-        self.update_selections(vec![selection], None, Some(Autoscroll::Fit), cx);
+        self.update_selections(vec![selection], Some(Autoscroll::Fit), cx);
     }
 
     pub fn select_all(&mut self, _: &SelectAll, cx: &mut ViewContext<Self>) {
@@ -3503,7 +3509,7 @@ impl Editor {
             reversed: false,
             goal: SelectionGoal::None,
         };
-        self.update_selections(vec![selection], None, None, cx);
+        self.update_selections(vec![selection], None, cx);
     }
 
     pub fn select_line(&mut self, _: &SelectLine, cx: &mut ViewContext<Self>) {
@@ -3516,7 +3522,7 @@ impl Editor {
             selection.end = cmp::min(max_point, Point::new(rows.end, 0));
             selection.reversed = false;
         }
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn split_selection_into_lines(
@@ -3551,7 +3557,7 @@ impl Editor {
             }
         }
         self.unfold_ranges(to_unfold, cx);
-        self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn add_selection_above(&mut self, _: &AddSelectionAbove, cx: &mut ViewContext<Self>) {
@@ -3649,7 +3655,7 @@ impl Editor {
             state.stack.pop();
         }
 
-        self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
         if state.stack.len() > 1 {
             self.add_selections_state = Some(state);
         }
@@ -3709,7 +3715,7 @@ impl Editor {
                         reversed: false,
                         goal: SelectionGoal::None,
                     });
-                    self.update_selections(selections, None, Some(Autoscroll::Newest), cx);
+                    self.update_selections(selections, Some(Autoscroll::Newest), cx);
                 } else {
                     select_next_state.done = true;
                 }
@@ -3736,7 +3742,7 @@ impl Editor {
                     wordwise: true,
                     done: false,
                 };
-                self.update_selections(selections, None, Some(Autoscroll::Newest), cx);
+                self.update_selections(selections, Some(Autoscroll::Newest), cx);
                 self.select_next_state = Some(select_state);
             } else {
                 let query = buffer
@@ -3844,7 +3850,6 @@ impl Editor {
 
         self.update_selections(
             self.local_selections::<usize>(cx),
-            None,
             Some(Autoscroll::Fit),
             cx,
         );
@@ -3891,7 +3896,7 @@ impl Editor {
 
         if selected_larger_node {
             stack.push(old_selections);
-            self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+            self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
         }
         self.select_larger_syntax_node_stack = stack;
     }
@@ -3903,7 +3908,7 @@ impl Editor {
     ) {
         let mut stack = mem::take(&mut self.select_larger_syntax_node_stack);
         if let Some(selections) = stack.pop() {
-            self.update_selections(selections.to_vec(), None, Some(Autoscroll::Fit), cx);
+            self.update_selections(selections.to_vec(), Some(Autoscroll::Fit), cx);
         }
         self.select_larger_syntax_node_stack = stack;
     }
@@ -3932,7 +3937,7 @@ impl Editor {
             }
         }
 
-        self.update_selections(selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(selections, Some(Autoscroll::Fit), cx);
     }
 
     pub fn show_next_diagnostic(&mut self, _: &ShowNextDiagnostic, cx: &mut ViewContext<Self>) {
@@ -3978,7 +3983,6 @@ impl Editor {
                         reversed: false,
                         goal: SelectionGoal::None,
                     }],
-                    None,
                     Some(Autoscroll::Center),
                     cx,
                 );
@@ -4345,7 +4349,6 @@ impl Editor {
     pub fn update_selections<T>(
         &mut self,
         mut selections: Vec<Selection<T>>,
-        pending_selection: Option<PendingSelection>,
         autoscroll: Option<Autoscroll>,
         cx: &mut ViewContext<Self>,
     ) where
@@ -4389,7 +4392,7 @@ impl Editor {
                     goal: selection.goal,
                 }
             })),
-            pending_selection,
+            None,
             cx,
         );
     }
@@ -4439,7 +4442,7 @@ impl Editor {
             })
             .collect();
         drop(snapshot);
-        self.update_selections(new_selections, None, Some(Autoscroll::Fit), cx);
+        self.update_selections(new_selections, Some(Autoscroll::Fit), cx);
         selections_with_lost_position
     }