When selections lose their excerpts, move them to the next primary diagnostic

crates/diagnostics/src/diagnostics.rs

@@ -6,7 +6,7 @@ use editor::{
     context_header_renderer, diagnostic_block_renderer, diagnostic_header_renderer,
     display_map::{BlockDisposition, BlockId, BlockProperties},
     items::BufferItemHandle,
-    Autoscroll, BuildSettings, Editor, ExcerptId, ExcerptProperties, MultiBuffer,
+    Autoscroll, BuildSettings, Editor, ExcerptId, ExcerptProperties, MultiBuffer, ToOffset,
 };
 use gpui::{
     action, elements::*, keymap::Binding, AppContext, Entity, ModelHandle, MutableAppContext,
@@ -56,6 +56,7 @@ struct ProjectDiagnosticsEditor {
 
 struct DiagnosticGroupState {
     primary_diagnostic: DiagnosticEntry<language::Anchor>,
+    primary_excerpt_ix: usize,
     excerpts: Vec<ExcerptId>,
     blocks: HashMap<BlockId, DiagnosticBlock>,
     block_count: usize,
@@ -300,6 +301,7 @@ impl ProjectDiagnosticsEditor {
                 if let Some(group) = to_insert {
                     let mut group_state = DiagnosticGroupState {
                         primary_diagnostic: group.entries[group.primary_ix].clone(),
+                        primary_excerpt_ix: 0,
                         excerpts: Default::default(),
                         blocks: Default::default(),
                         block_count: 0,
@@ -370,6 +372,7 @@ impl ProjectDiagnosticsEditor {
                             for entry in &group.entries[*start_ix..ix] {
                                 let mut diagnostic = entry.diagnostic.clone();
                                 if diagnostic.is_primary {
+                                    group_state.primary_excerpt_ix = group_state.excerpts.len() - 1;
                                     diagnostic.message =
                                         entry.diagnostic.message.split('\n').skip(1).collect();
                                 }
@@ -433,22 +436,6 @@ impl ProjectDiagnosticsEditor {
             for group_state in &mut groups_to_add {
                 group_state.blocks = block_ids.by_ref().take(group_state.block_count).collect();
             }
-
-            if was_empty {
-                editor.update_selections(
-                    vec![Selection {
-                        id: 0,
-                        start: 0,
-                        end: 0,
-                        reversed: false,
-                        goal: SelectionGoal::None,
-                    }],
-                    None,
-                    cx,
-                );
-            } else {
-                editor.refresh_selections(cx);
-            }
         });
 
         for ix in group_ixs_to_remove.into_iter().rev() {
@@ -469,6 +456,49 @@ impl ProjectDiagnosticsEditor {
             self.path_states.remove(path_ix);
         }
 
+        self.editor.update(cx, |editor, cx| {
+            let groups = self.path_states.get(path_ix)?.1.as_slice();
+
+            let mut selections;
+            let new_excerpt_ids_by_selection_id;
+            if was_empty {
+                new_excerpt_ids_by_selection_id = [(0, ExcerptId::min())].into_iter().collect();
+                selections = vec![Selection {
+                    id: 0,
+                    start: 0,
+                    end: 0,
+                    reversed: false,
+                    goal: SelectionGoal::None,
+                }];
+            } else {
+                new_excerpt_ids_by_selection_id = editor.refresh_selections(cx);
+                selections = editor.local_selections::<usize>(cx);
+            }
+
+            // If any selection has lost its position, move it to start of the next primary diagnostic.
+            for selection in &mut selections {
+                if let Some(new_excerpt_id) = new_excerpt_ids_by_selection_id.get(&selection.id) {
+                    let group_ix = match groups.binary_search_by(|probe| {
+                        probe.excerpts.last().unwrap().cmp(&new_excerpt_id)
+                    }) {
+                        Ok(ix) | Err(ix) => ix,
+                    };
+                    if let Some(group) = groups.get(group_ix) {
+                        let offset = excerpts_snapshot
+                            .anchor_in_excerpt(
+                                group.excerpts[group.primary_excerpt_ix].clone(),
+                                group.primary_diagnostic.range.start.clone(),
+                            )
+                            .to_offset(&excerpts_snapshot);
+                        selection.start = offset;
+                        selection.end = offset;
+                    }
+                }
+            }
+            editor.update_selections(selections, None, cx);
+            Some(())
+        });
+
         if self.path_states.is_empty() {
             if self.editor.is_focused(cx) {
                 cx.focus_self();

crates/editor/src/editor.rs

@@ -28,8 +28,8 @@ use language::{
     BracketPair, Buffer, Diagnostic, DiagnosticSeverity, Language, Point, Selection, SelectionGoal,
     TransactionId,
 };
-pub use multi_buffer::{Anchor, ExcerptId, ExcerptProperties, MultiBuffer};
+pub use multi_buffer::{Anchor, ExcerptId, ExcerptProperties, MultiBuffer, ToOffset, ToPoint};
-use multi_buffer::{AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot, ToOffset, ToPoint};
+use multi_buffer::{AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot};
 use postage::watch;
 use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
@@ -3297,8 +3297,14 @@ impl Editor {
         );
     }
 
-    pub fn refresh_selections(&mut self, cx: &mut ViewContext<Self>) {
+    /// Compute new ranges for any selections that were located in excerpts that have
-        let anchors = self.buffer.update(cx, |buffer, cx| {
+    /// since been removed.
+    ///
+    /// Returns a `HashMap` indicating which selections whose former head position
+    /// was no longer present. The keys of the map are selection ids. The values are
+    /// the id of the new excerpt where the head of the selection has been moved.
+    pub fn refresh_selections(&mut self, cx: &mut ViewContext<Self>) -> HashMap<usize, ExcerptId> {
+        let anchors_with_status = self.buffer.update(cx, |buffer, cx| {
             let snapshot = buffer.read(cx);
             snapshot.refresh_anchors(
                 self.selections
@@ -3306,17 +3312,28 @@ impl Editor {
                     .flat_map(|selection| [&selection.start, &selection.end]),
             )
         });
+        let mut selections_with_lost_position = HashMap::default();
         self.selections = self
             .selections
             .iter()
             .cloned()
-            .zip(anchors.chunks(2))
+            .zip(anchors_with_status.chunks(2))
             .map(|(mut selection, anchors)| {
-                selection.start = anchors[0].clone();
+                selection.start = anchors[0].0.clone();
-                selection.end = anchors[1].clone();
+                selection.end = anchors[1].0.clone();
+                let kept_head_position = if selection.reversed {
+                    anchors[0].1
+                } else {
+                    anchors[1].1
+                };
+                if !kept_head_position {
+                    selections_with_lost_position
+                        .insert(selection.id, selection.head().excerpt_id.clone());
+                }
                 selection
             })
             .collect();
+        selections_with_lost_position
     }
 
     fn set_selections(&mut self, selections: Arc<[Selection<Anchor>]>, cx: &mut ViewContext<Self>) {

crates/editor/src/multi_buffer.rs

@@ -1342,7 +1342,7 @@ impl MultiBufferSnapshot {
         position
     }
 
-    pub fn refresh_anchors<'a, I>(&'a self, anchors: I) -> Vec<Anchor>
+    pub fn refresh_anchors<'a, I>(&'a self, anchors: I) -> Vec<(Anchor, bool)>
     where
         I: 'a + IntoIterator<Item = &'a Anchor>,
     {
@@ -1352,7 +1352,7 @@ impl MultiBufferSnapshot {
         while let Some(anchor) = anchors.peek() {
             let old_excerpt_id = &anchor.excerpt_id;
 
-            // Find the location where this anchor's excerpt should be,
+            // Find the location where this anchor's excerpt should be.
             cursor.seek_forward(&Some(old_excerpt_id), Bias::Left, &());
             if cursor.item().is_none() {
                 cursor.next(&());
@@ -1366,32 +1366,58 @@ impl MultiBufferSnapshot {
                 if anchor.excerpt_id != *old_excerpt_id {
                     break;
                 }
+                let mut kept_position = false;
                 let mut anchor = anchors.next().unwrap().clone();
 
+                // Leave min and max anchors unchanged.
+                if *old_excerpt_id == ExcerptId::max() || *old_excerpt_id == ExcerptId::min() {
+                    kept_position = true;
+                }
+                // If the old excerpt still exists at this location, then leave
+                // the anchor unchanged.
+                else if next_excerpt.map_or(false, |excerpt| {
+                    excerpt.id == *old_excerpt_id && excerpt.buffer_id == anchor.buffer_id
+                }) {
+                    kept_position = true;
+                }
                 // If the old excerpt no longer exists at this location, then attempt to
                 // find an equivalent position for this anchor in an adjacent excerpt.
-                if next_excerpt.map_or(true, |e| e.id != *old_excerpt_id) {
+                else {
                     for excerpt in [next_excerpt, prev_excerpt].iter().filter_map(|e| *e) {
-                        if excerpt.buffer_id == anchor.buffer_id
+                        if excerpt.contains(&anchor) {
-                            && excerpt
-                                .range
-                                .start
-                                .cmp(&anchor.text_anchor, &excerpt.buffer)
-                                .unwrap()
-                                .is_le()
-                            && excerpt
-                                .range
-                                .end
-                                .cmp(&anchor.text_anchor, &excerpt.buffer)
-                                .unwrap()
-                                .is_ge()
-                        {
                             anchor.excerpt_id = excerpt.id.clone();
+                            kept_position = true;
+                            break;
                         }
                     }
                 }
+                // If there's no adjacent excerpt that contains the anchor's position,
+                // then report that the anchor has lost its position.
+                if !kept_position {
+                    anchor = if let Some(excerpt) = next_excerpt {
+                        Anchor {
+                            buffer_id: excerpt.buffer_id,
+                            excerpt_id: excerpt.id.clone(),
+                            text_anchor: excerpt
+                                .buffer
+                                .anchor_at(&excerpt.range.start, anchor.text_anchor.bias),
+                        }
+                    } else if let Some(excerpt) = prev_excerpt {
+                        Anchor {
+                            buffer_id: excerpt.buffer_id,
+                            excerpt_id: excerpt.id.clone(),
+                            text_anchor: excerpt
+                                .buffer
+                                .anchor_at(&excerpt.range.end, anchor.text_anchor.bias),
+                        }
+                    } else if anchor.text_anchor.bias == Bias::Left {
+                        Anchor::min()
+                    } else {
+                        Anchor::max()
+                    };
+                }
 
-                result.push(anchor);
+                result.push((anchor, kept_position));
             }
         }
         result
@@ -1894,6 +1920,22 @@ impl Excerpt {
             text_anchor
         }
     }
+
+    fn contains(&self, anchor: &Anchor) -> bool {
+        self.buffer_id == anchor.buffer_id
+            && self
+                .range
+                .start
+                .cmp(&anchor.text_anchor, &self.buffer)
+                .unwrap()
+                .is_le()
+            && self
+                .range
+                .end
+                .cmp(&anchor.text_anchor, &self.buffer)
+                .unwrap()
+                .is_ge()
+    }
 }
 
 impl fmt::Debug for Excerpt {
@@ -2523,7 +2565,7 @@ mod tests {
         let snapshot_2 = multibuffer.read(cx).snapshot(cx);
         assert_eq!(snapshot_2.text(), "ABCD\nGHIJ\nMNOP");
 
-        // And excerpt id has been reused.
+        // The old excerpt id has been reused.
         assert_eq!(excerpt_id_2, excerpt_id_1);
 
         // Resolve some anchors from the previous snapshot in the new snapshot.
@@ -2541,6 +2583,15 @@ mod tests {
             ]),
             vec![0, 0]
         );
+        let refresh =
+            snapshot_2.refresh_anchors(&[snapshot_1.anchor_before(2), snapshot_1.anchor_after(3)]);
+        assert_eq!(
+            refresh,
+            &[
+                (snapshot_2.anchor_before(0), false),
+                (snapshot_2.anchor_after(0), false),
+            ]
+        );
 
         // Replace the middle excerpt with a smaller excerpt in buffer 2,
         // that intersects the old excerpt.
@@ -2564,19 +2615,24 @@ mod tests {
         // The anchor in the middle excerpt snaps to the beginning of the
         // excerpt, since it is not
         let anchors = [
+            snapshot_2.anchor_before(0),
             snapshot_2.anchor_after(2),
             snapshot_2.anchor_after(6),
             snapshot_2.anchor_after(14),
         ];
         assert_eq!(
             snapshot_3.summaries_for_anchors::<usize, _>(&anchors),
-            &[2, 9, 13]
+            &[0, 2, 9, 13]
         );
 
         let new_anchors = snapshot_3.refresh_anchors(&anchors);
         assert_eq!(
-            snapshot_3.summaries_for_anchors::<usize, _>(&new_anchors),
+            new_anchors.iter().map(|a| a.1).collect::<Vec<_>>(),
-            &[2, 7, 13]
+            &[true, true, true, true]
+        );
+        assert_eq!(
+            snapshot_3.summaries_for_anchors::<usize, _>(new_anchors.iter().map(|a| &a.0)),
+            &[0, 2, 7, 13]
         );
     }