From 5ddfcf09f742c762d0c7627e436f92cca2d75901 Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Mon, 12 Jan 2026 13:04:37 +0100 Subject: [PATCH] editor: Remove unnecessary cloning of selections (#46598) Stumbled across this upon a PR review - `unfold_ranges` takes a slice of Ranges and already clones the ranges itself, so we do not need to clone the selections prior to passing these. Release Notes: - N/A --- crates/editor/src/editor.rs | 11 ++++------- crates/editor/src/scroll/autoscroll.rs | 26 +++++++++++--------------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index de2e6b757bdd7968ed08341c5a4fe3c7675c6d23..e2781436d491b8ec7c6237ed4909427c1cdfa084 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -15358,12 +15358,10 @@ impl Editor { let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx)); self.select_next_match_internal(&display_map, false, None, window, cx)?; - let Some(select_next_state) = self.select_next_state.as_mut() else { + let Some(select_next_state) = self.select_next_state.as_mut().filter(|state| !state.done) + else { return Ok(()); }; - if select_next_state.done { - return Ok(()); - } let mut new_selections = Vec::new(); @@ -15399,7 +15397,7 @@ impl Editor { return Ok(()); } - self.unfold_ranges(&new_selections.clone(), false, false, cx); + self.unfold_ranges(&new_selections, false, false, cx); self.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| { selections.select_ranges(new_selections) }); @@ -15421,8 +15419,7 @@ impl Editor { Some(Autoscroll::newest()), window, cx, - )?; - Ok(()) + ) } pub fn select_previous( diff --git a/crates/editor/src/scroll/autoscroll.rs b/crates/editor/src/scroll/autoscroll.rs index fc2ecb9205109532da2b43c97821b5352f27aff2..068acffcad15b0217439efe767518c8cdbc6bb04 100644 --- a/crates/editor/src/scroll/autoscroll.rs +++ b/crates/editor/src/scroll/autoscroll.rs @@ -207,21 +207,17 @@ impl Editor { let strategy = match autoscroll { Autoscroll::Strategy(strategy, _) => strategy, - Autoscroll::Next => { - let last_autoscroll = &self.scroll_manager.last_autoscroll; - if let Some(last_autoscroll) = last_autoscroll { - if self.scroll_manager.anchor.offset == last_autoscroll.0 - && target_top == last_autoscroll.1 - && target_bottom == last_autoscroll.2 - { - last_autoscroll.3.next() - } else { - AutoscrollStrategy::default() - } - } else { - AutoscrollStrategy::default() - } - } + Autoscroll::Next => self + .scroll_manager + .last_autoscroll + .as_ref() + .filter(|(offset, last_target_top, last_target_bottom, _)| { + self.scroll_manager.anchor.offset == *offset + && target_top == *last_target_top + && target_bottom == *last_target_bottom + }) + .map(|(_, _, _, strategy)| strategy.next()) + .unwrap_or_default(), }; if let Autoscroll::Strategy(_, Some(anchor)) = autoscroll { target_top = anchor.to_display_point(&display_map).row().as_f64();