Make autoscroll optional when highlighting editor rows (#11950)

Previously, when highlighting editor rows with a color, we always
auto-scrolled to the first highlighted row. This was useful in contexts
like go-to-line and the outline view. We had an explicit special case
for git diff highlights. Now, part of the `highlight_rows` API, you
specify whether or not you want the autoscroll behavior. This is needed
because we want to highlight rows in the assistant panel, and we don't
want the autoscroll.

Release Notes:

- N/A

Cargo.lock

@@ -4630,7 +4630,6 @@ name = "go_to_line"
 version = "0.1.0"
 dependencies = [
  "anyhow",
- "collections",
  "editor",
  "gpui",
  "indoc",
@@ -7033,7 +7032,6 @@ dependencies = [
 name = "outline"
 version = "0.1.0"
 dependencies = [
- "collections",
  "editor",
  "fuzzy",
  "gpui",

crates/editor/src/editor.rs

@@ -48,7 +48,7 @@ use anyhow::{anyhow, Context as _, Result};
 use blink_manager::BlinkManager;
 use client::{Collaborator, ParticipantIndex};
 use clock::ReplicaId;
-use collections::{hash_map, BTreeMap, Bound, HashMap, HashSet, VecDeque};
+use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque};
 use convert_case::{Case, Casing};
 use debounced_delay::DebouncedDelay;
 pub use display_map::DisplayPoint;
@@ -450,7 +450,7 @@ pub struct Editor {
     show_wrap_guides: Option<bool>,
     placeholder_text: Option<Arc<str>>,
     highlight_order: usize,
-    highlighted_rows: HashMap<TypeId, Vec<(usize, RangeInclusive<Anchor>, Option<Hsla>)>>,
+    highlighted_rows: HashMap<TypeId, Vec<RowHighlight>>,
     background_highlights: TreeMap<TypeId, BackgroundHighlight>,
     scrollbar_marker_state: ScrollbarMarkerState,
     nav_history: Option<ItemNavHistory>,
@@ -660,6 +660,13 @@ impl SelectionHistory {
     }
 }
 
+struct RowHighlight {
+    index: usize,
+    range: RangeInclusive<Anchor>,
+    color: Option<Hsla>,
+    should_autoscroll: bool,
+}
+
 #[derive(Clone, Debug)]
 struct AddSelectionsState {
     above: bool,
@@ -9814,47 +9821,30 @@ impl Editor {
         &mut self,
         rows: RangeInclusive<Anchor>,
         color: Option<Hsla>,
+        should_autoscroll: bool,
         cx: &mut ViewContext<Self>,
     ) {
-        let multi_buffer_snapshot = self.buffer().read(cx).snapshot(cx);
+        let snapshot = self.buffer().read(cx).snapshot(cx);
-        match self.highlighted_rows.entry(TypeId::of::<T>()) {
+        let row_highlights = self.highlighted_rows.entry(TypeId::of::<T>()).or_default();
-            hash_map::Entry::Occupied(o) => {
+        let existing_highlight_index = row_highlights.binary_search_by(|highlight| {
-                let row_highlights = o.into_mut();
+            highlight
-                let existing_highlight_index =
+                .range
-                    row_highlights.binary_search_by(|(_, highlight_range, _)| {
+                .start()
-                        highlight_range
+                .cmp(&rows.start(), &snapshot)
-                            .start()
+                .then(highlight.range.end().cmp(&rows.end(), &snapshot))
-                            .cmp(&rows.start(), &multi_buffer_snapshot)
+        });
-                            .then(
+        match (color, existing_highlight_index) {
-                                highlight_range
+            (Some(_), Ok(ix)) | (_, Err(ix)) => row_highlights.insert(
-                                    .end()
+                ix,
-                                    .cmp(&rows.end(), &multi_buffer_snapshot),
+                RowHighlight {
-                            )
+                    index: post_inc(&mut self.highlight_order),
-                    });
+                    range: rows,
-                match color {
+                    should_autoscroll,
-                    Some(color) => {
+                    color,
-                        let insert_index = match existing_highlight_index {
+                },
-                            Ok(i) => i,
+            ),
-                            Err(i) => i,
+            (None, Ok(i)) => {
-                        };
+                row_highlights.remove(i);
-                        row_highlights.insert(
-                            insert_index,
-                            (post_inc(&mut self.highlight_order), rows, Some(color)),
-                        );
-                    }
-                    None => match existing_highlight_index {
-                        Ok(i) => {
-                            row_highlights.remove(i);
-                        }
-                        Err(i) => {
-                            row_highlights
-                                .insert(i, (post_inc(&mut self.highlight_order), rows, None));
-                        }
-                    },
-                }
-            }
-            hash_map::Entry::Vacant(v) => {
-                v.insert(vec![(post_inc(&mut self.highlight_order), rows, color)]);
             }
         }
     }
@@ -9872,7 +9862,7 @@ impl Editor {
             self.highlighted_rows
                 .get(&TypeId::of::<T>())?
                 .iter()
-                .map(|(_, range, color)| (range, color.as_ref())),
+                .map(|highlight| (&highlight.range, highlight.color.as_ref())),
         )
     }
 
@@ -9881,33 +9871,27 @@ impl Editor {
     /// Allows to ignore certain kinds of highlights.
     pub fn highlighted_display_rows(
         &mut self,
-        exclude_highlights: HashSet<TypeId>,
         cx: &mut WindowContext,
     ) -> BTreeMap<DisplayRow, Hsla> {
         let snapshot = self.snapshot(cx);
         let mut used_highlight_orders = HashMap::default();
         self.highlighted_rows
             .iter()
-            .filter(|(type_id, _)| !exclude_highlights.contains(type_id))
             .flat_map(|(_, highlighted_rows)| highlighted_rows.iter())
             .fold(
                 BTreeMap::<DisplayRow, Hsla>::new(),
-                |mut unique_rows, (highlight_order, anchor_range, hsla)| {
+                |mut unique_rows, highlight| {
-                    let start_row = anchor_range.start().to_display_point(&snapshot).row();
+                    let start_row = highlight.range.start().to_display_point(&snapshot).row();
-                    let end_row = anchor_range.end().to_display_point(&snapshot).row();
+                    let end_row = highlight.range.end().to_display_point(&snapshot).row();
                     for row in start_row.0..=end_row.0 {
                         let used_index =
-                            used_highlight_orders.entry(row).or_insert(*highlight_order);
+                            used_highlight_orders.entry(row).or_insert(highlight.index);
-                        if highlight_order >= used_index {
+                        if highlight.index >= *used_index {
-                            *used_index = *highlight_order;
+                            *used_index = highlight.index;
-                            match hsla {
+                            match highlight.color {
-                                Some(hsla) => {
+                                Some(hsla) => unique_rows.insert(DisplayRow(row), hsla),
-                                    unique_rows.insert(DisplayRow(row), *hsla);
+                                None => unique_rows.remove(&DisplayRow(row)),
-                                }
+                            };
-                                None => {
-                                    unique_rows.remove(&DisplayRow(row));
-                                }
-                            }
                         }
                     }
                     unique_rows
@@ -9915,6 +9899,22 @@ impl Editor {
             )
     }
 
+    pub fn highlighted_display_row_for_autoscroll(
+        &self,
+        snapshot: &DisplaySnapshot,
+    ) -> Option<DisplayRow> {
+        self.highlighted_rows
+            .values()
+            .flat_map(|highlighted_rows| highlighted_rows.iter())
+            .filter_map(|highlight| {
+                if highlight.color.is_none() || !highlight.should_autoscroll {
+                    return None;
+                }
+                Some(highlight.range.start().to_display_point(&snapshot).row())
+            })
+            .min()
+    }
+
     pub fn set_search_within_ranges(
         &mut self,
         ranges: &[Range<Anchor>],

crates/editor/src/element.rs

@@ -26,7 +26,7 @@ use crate::{
 };
 use anyhow::Result;
 use client::ParticipantIndex;
-use collections::{BTreeMap, HashMap, HashSet};
+use collections::{BTreeMap, HashMap};
 use git::{blame::BlameEntry, diff::DiffHunkStatus, Oid};
 use gpui::{
     anchored, deferred, div, fill, outline, point, px, quad, relative, size, svg,
@@ -3948,9 +3948,9 @@ impl Element for EditorElement {
                         )
                     };
 
-                    let highlighted_rows = self.editor.update(cx, |editor, cx| {
+                    let highlighted_rows = self
-                        editor.highlighted_display_rows(HashSet::default(), cx)
+                        .editor
-                    });
+                        .update(cx, |editor, cx| editor.highlighted_display_rows(cx));
                     let highlighted_ranges = self.editor.read(cx).background_highlights_in_range(
                         start_anchor..end_anchor,
                         &snapshot.display_snapshot,

crates/editor/src/hunk_diff.rs

@@ -194,6 +194,7 @@ impl Editor {
                         editor.highlight_rows::<DiffRowHighlight>(
                             to_inclusive_row_range(removed_rows, &snapshot),
                             None,
+                            false,
                             cx,
                         );
                     }
@@ -269,6 +270,7 @@ impl Editor {
                 self.highlight_rows::<DiffRowHighlight>(
                     to_inclusive_row_range(hunk_start..hunk_end, &snapshot),
                     Some(added_hunk_color(cx)),
+                    false,
                     cx,
                 );
                 None
@@ -277,6 +279,7 @@ impl Editor {
                 self.highlight_rows::<DiffRowHighlight>(
                     to_inclusive_row_range(hunk_start..hunk_end, &snapshot),
                     Some(added_hunk_color(cx)),
+                    false,
                     cx,
                 );
                 self.insert_deleted_text_block(diff_base_buffer, deleted_text_lines, &hunk, cx)
@@ -476,6 +479,7 @@ impl Editor {
                         editor.highlight_rows::<DiffRowHighlight>(
                             to_inclusive_row_range(removed_rows, &snapshot),
                             None,
+                            false,
                             cx,
                         );
                     }
@@ -581,7 +585,7 @@ fn editor_with_deleted_text(
             .buffer_snapshot
             .anchor_after(editor.buffer.read(cx).len(cx));
 
-        editor.highlight_rows::<DiffRowHighlight>(start..=end, Some(deleted_color), cx);
+        editor.highlight_rows::<DiffRowHighlight>(start..=end, Some(deleted_color), false, cx);
 
         let subscription_editor = parent_editor.clone();
         editor._subscriptions.extend([

crates/editor/src/scroll/autoscroll.rs

@@ -1,13 +1,9 @@
-use std::{any::TypeId, cmp, f32};
+use crate::{
-
+    display_map::ToDisplayPoint, DisplayRow, Editor, EditorMode, LineWithInvisibles, RowExt,
-use collections::HashSet;
+};
 use gpui::{px, Bounds, Pixels, ViewContext};
 use language::Point;
-
+use std::{cmp, f32};
-use crate::{
-    display_map::ToDisplayPoint, DiffRowHighlight, DisplayRow, Editor, EditorMode,
-    LineWithInvisibles, RowExt,
-};
 
 #[derive(PartialEq, Eq, Clone, Copy)]
 pub enum Autoscroll {
@@ -107,14 +103,10 @@ impl Editor {
 
         let mut target_top;
         let mut target_bottom;
-        if let Some(first_highlighted_row) = &self
+        if let Some(first_highlighted_row) =
-            .highlighted_display_rows(
+            self.highlighted_display_row_for_autoscroll(&display_map)
-                HashSet::from_iter(Some(TypeId::of::<DiffRowHighlight>())),
-                cx,
-            )
-            .first_entry()
         {
-            target_top = first_highlighted_row.key().as_f32();
+            target_top = first_highlighted_row.as_f32();
             target_bottom = target_top + 1.;
         } else {
             let selections = self.selections.all::<Point>(cx);
@@ -244,7 +236,10 @@ impl Editor {
         let mut target_left;
         let mut target_right;
 
-        if self.highlighted_rows.is_empty() {
+        if self
+            .highlighted_display_row_for_autoscroll(&display_map)
+            .is_none()
+        {
             target_left = px(f32::INFINITY);
             target_right = px(0.);
             for selection in selections {

crates/editor/src/test.rs

@@ -168,8 +168,7 @@ pub fn expanded_hunks_background_highlights(
 
     let mut range_start = 0;
     let mut previous_highlighted_row = None;
-    for (highlighted_row, _) in editor.highlighted_display_rows(collections::HashSet::default(), cx)
+    for (highlighted_row, _) in editor.highlighted_display_rows(cx) {
-    {
         match previous_highlighted_row {
             Some(previous_row) => {
                 if previous_row + 1 != highlighted_row.0 {

crates/go_to_line/Cargo.toml

@@ -14,7 +14,6 @@ doctest = false
 
 [dependencies]
 anyhow.workspace = true
-collections.workspace = true
 editor.workspace = true
 gpui.workspace = true
 menu.workspace = true

crates/go_to_line/src/go_to_line.rs

@@ -122,6 +122,7 @@ impl GoToLine {
                 active_editor.highlight_rows::<GoToLineRowHighlights>(
                     anchor..=anchor,
                     Some(cx.theme().colors().editor_highlighted_line_background),
+                    true,
                     cx,
                 );
                 active_editor.request_autoscroll(Autoscroll::center(), cx);
@@ -219,17 +220,14 @@ impl Render for GoToLine {
 
 #[cfg(test)]
 mod tests {
-    use std::sync::Arc;
+    use super::*;
-
-    use collections::HashSet;
     use gpui::{TestAppContext, VisualTestContext};
     use indoc::indoc;
     use project::{FakeFs, Project};
     use serde_json::json;
+    use std::sync::Arc;
     use workspace::{AppState, Workspace};
 
-    use super::*;
-
     #[gpui::test]
     async fn test_go_to_line_view_row_highlights(cx: &mut TestAppContext) {
         init_test(cx);
@@ -350,7 +348,7 @@ mod tests {
     fn highlighted_display_rows(editor: &View<Editor>, cx: &mut VisualTestContext) -> Vec<u32> {
         editor.update(cx, |editor, cx| {
             editor
-                .highlighted_display_rows(HashSet::default(), cx)
+                .highlighted_display_rows(cx)
                 .into_keys()
                 .map(|r| r.0)
                 .collect()

crates/outline/Cargo.toml

@@ -13,7 +13,6 @@ path = "src/outline.rs"
 doctest = false
 
 [dependencies]
-collections.workspace = true
 editor.workspace = true
 fuzzy.workspace = true
 gpui.workspace = true

crates/outline/src/outline.rs

@@ -144,6 +144,7 @@ impl OutlineViewDelegate {
                 active_editor.highlight_rows::<OutlineRowHighlights>(
                     outline_item.range.start..=outline_item.range.end,
                     Some(cx.theme().colors().editor_highlighted_line_background),
+                    true,
                     cx,
                 );
                 active_editor.request_autoscroll(Autoscroll::center(), cx);
@@ -316,7 +317,7 @@ impl PickerDelegate for OutlineViewDelegate {
 
 #[cfg(test)]
 mod tests {
-    use collections::HashSet;
+    use super::*;
     use gpui::{TestAppContext, VisualTestContext};
     use indoc::indoc;
     use language::{Language, LanguageConfig, LanguageMatcher};
@@ -324,8 +325,6 @@ mod tests {
     use serde_json::json;
     use workspace::{AppState, Workspace};
 
-    use super::*;
-
     #[gpui::test]
     async fn test_outline_view_row_highlights(cx: &mut TestAppContext) {
         init_test(cx);
@@ -484,7 +483,7 @@ mod tests {
     fn highlighted_display_rows(editor: &View<Editor>, cx: &mut VisualTestContext) -> Vec<u32> {
         editor.update(cx, |editor, cx| {
             editor
-                .highlighted_display_rows(HashSet::default(), cx)
+                .highlighted_display_rows(cx)
                 .into_keys()
                 .map(|r| r.0)
                 .collect()