From 70d197876e62ce028950f3cf68c64400e014f37f Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 15 Oct 2025 15:55:00 +0200 Subject: [PATCH] editor: Fix `SelectionsCollection::disjoint` not being ordered correctly (#40249) We've been seeing the occasional `cannot seek backwards` panic within `SelectionsCollection` without means to reproduce. I believe the cause is one of the callers of `MutableSelectionsCollection::select` not passing a well formed `Selection` where `start > end`, so this PR enforces the invariant in `select` by swapping the fields and setting `reversed` as required as the other mutator functions already do that as well. We could also just assert this instead, but it callers usually won't care about this so its the less user facing annoyance to just fix this invariant up internally. Fixes ZED-253 Fixes ZED-ZJ Fixes ZED-23S Fixes ZED-222 Fixes ZED-1ZV Fixes ZED-1SN Fixes ZED-1Z0 Fixes ZED-10E Fixes ZED-1X0 Fixes ZED-12M Fixes ZED-1GR Fixes ZED-1VE Fixes ZED-13X Fixes ZED-1G4 Release Notes: - Fixed occasional panics when querying selections --- crates/editor/src/selections_collection.rs | 32 ++++++++++++++-------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/crates/editor/src/selections_collection.rs b/crates/editor/src/selections_collection.rs index acdece23d9996ae397a6934db43917bf12cecf37..54b8edcdc0b90de3079853d20240b68d2249f976 100644 --- a/crates/editor/src/selections_collection.rs +++ b/crates/editor/src/selections_collection.rs @@ -610,21 +610,32 @@ impl<'a> MutableSelectionsCollection<'a> { self.select(selections); } - pub fn select(&mut self, mut selections: Vec>) + pub fn select(&mut self, selections: Vec>) where - T: ToOffset + ToPoint + Ord + std::marker::Copy + std::fmt::Debug, + T: ToOffset + std::marker::Copy + std::fmt::Debug, { let buffer = self.buffer.read(self.cx).snapshot(self.cx); + let mut selections = selections + .into_iter() + .map(|selection| selection.map(|it| it.to_offset(&buffer))) + .map(|mut selection| { + if selection.start > selection.end { + mem::swap(&mut selection.start, &mut selection.end); + selection.reversed = true + } + selection + }) + .collect::>(); selections.sort_unstable_by_key(|s| s.start); // Merge overlapping selections. let mut i = 1; while i < selections.len() { - if selections[i - 1].end >= selections[i].start { + if selections[i].start <= selections[i - 1].end { let removed = selections.remove(i); if removed.start < selections[i - 1].start { selections[i - 1].start = removed.start; } - if removed.end > selections[i - 1].end { + if selections[i - 1].end < removed.end { selections[i - 1].end = removed.end; } } else { @@ -968,13 +979,10 @@ impl DerefMut for MutableSelectionsCollection<'_> { } } -fn selection_to_anchor_selection( - selection: Selection, +fn selection_to_anchor_selection( + selection: Selection, buffer: &MultiBufferSnapshot, -) -> Selection -where - T: ToOffset + Ord, -{ +) -> Selection { let end_bias = if selection.start == selection.end { Bias::Right } else { @@ -1012,7 +1020,7 @@ fn resolve_selections_point<'a>( }) } -// Panics if passed selections are not in order +/// Panics if passed selections are not in order fn resolve_selections_display<'a>( selections: impl 'a + IntoIterator>, map: &'a DisplaySnapshot, @@ -1044,7 +1052,7 @@ fn resolve_selections_display<'a>( coalesce_selections(selections) } -// Panics if passed selections are not in order +/// Panics if passed selections are not in order pub(crate) fn resolve_selections<'a, D, I>( selections: I, map: &'a DisplaySnapshot,