Skip to content

Commit 70d1978

Browse files
committed
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
1 parent 10c540b commit 70d1978

File tree

1 file changed

+20
-12
lines changed

1 file changed

+20
-12
lines changed

crates/editor/src/selections_collection.rs

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -610,21 +610,32 @@ impl<'a> MutableSelectionsCollection<'a> {
610610
self.select(selections);
611611
}
612612

613-
pub fn select<T>(&mut self, mut selections: Vec<Selection<T>>)
613+
pub fn select<T>(&mut self, selections: Vec<Selection<T>>)
614614
where
615-
T: ToOffset + ToPoint + Ord + std::marker::Copy + std::fmt::Debug,
615+
T: ToOffset + std::marker::Copy + std::fmt::Debug,
616616
{
617617
let buffer = self.buffer.read(self.cx).snapshot(self.cx);
618+
let mut selections = selections
619+
.into_iter()
620+
.map(|selection| selection.map(|it| it.to_offset(&buffer)))
621+
.map(|mut selection| {
622+
if selection.start > selection.end {
623+
mem::swap(&mut selection.start, &mut selection.end);
624+
selection.reversed = true
625+
}
626+
selection
627+
})
628+
.collect::<Vec<_>>();
618629
selections.sort_unstable_by_key(|s| s.start);
619630
// Merge overlapping selections.
620631
let mut i = 1;
621632
while i < selections.len() {
622-
if selections[i - 1].end >= selections[i].start {
633+
if selections[i].start <= selections[i - 1].end {
623634
let removed = selections.remove(i);
624635
if removed.start < selections[i - 1].start {
625636
selections[i - 1].start = removed.start;
626637
}
627-
if removed.end > selections[i - 1].end {
638+
if selections[i - 1].end < removed.end {
628639
selections[i - 1].end = removed.end;
629640
}
630641
} else {
@@ -968,13 +979,10 @@ impl DerefMut for MutableSelectionsCollection<'_> {
968979
}
969980
}
970981

971-
fn selection_to_anchor_selection<T>(
972-
selection: Selection<T>,
982+
fn selection_to_anchor_selection(
983+
selection: Selection<usize>,
973984
buffer: &MultiBufferSnapshot,
974-
) -> Selection<Anchor>
975-
where
976-
T: ToOffset + Ord,
977-
{
985+
) -> Selection<Anchor> {
978986
let end_bias = if selection.start == selection.end {
979987
Bias::Right
980988
} else {
@@ -1012,7 +1020,7 @@ fn resolve_selections_point<'a>(
10121020
})
10131021
}
10141022

1015-
// Panics if passed selections are not in order
1023+
/// Panics if passed selections are not in order
10161024
fn resolve_selections_display<'a>(
10171025
selections: impl 'a + IntoIterator<Item = &'a Selection<Anchor>>,
10181026
map: &'a DisplaySnapshot,
@@ -1044,7 +1052,7 @@ fn resolve_selections_display<'a>(
10441052
coalesce_selections(selections)
10451053
}
10461054

1047-
// Panics if passed selections are not in order
1055+
/// Panics if passed selections are not in order
10481056
pub(crate) fn resolve_selections<'a, D, I>(
10491057
selections: I,
10501058
map: &'a DisplaySnapshot,

0 commit comments

Comments
 (0)