From 4ca6e0e3874feaa939390b191a010ede88109adf Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 16 May 2024 20:28:17 -0700 Subject: [PATCH] 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 | 2 - crates/editor/src/editor.rs | 116 ++++++++++++------------- crates/editor/src/element.rs | 8 +- crates/editor/src/hunk_diff.rs | 6 +- crates/editor/src/scroll/autoscroll.rs | 27 +++--- crates/editor/src/test.rs | 3 +- crates/go_to_line/Cargo.toml | 1 - crates/go_to_line/src/go_to_line.rs | 10 +-- crates/outline/Cargo.toml | 1 - crates/outline/src/outline.rs | 7 +- 10 files changed, 86 insertions(+), 95 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 71675fbee8eef28c5ec50bc481f9d75269976c29..a0e0ffefd8001f843a1a67f2e40924d26b27abb5 100644 --- a/Cargo.lock +++ b/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", diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 2d98f8c3f9dcd7237242b3e342747caba0b18965..82ad51cd6baf470156279c79b9d41739dafa9f85 100644 --- a/crates/editor/src/editor.rs +++ b/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, placeholder_text: Option>, highlight_order: usize, - highlighted_rows: HashMap, Option)>>, + highlighted_rows: HashMap>, background_highlights: TreeMap, scrollbar_marker_state: ScrollbarMarkerState, nav_history: Option, @@ -660,6 +660,13 @@ impl SelectionHistory { } } +struct RowHighlight { + index: usize, + range: RangeInclusive, + color: Option, + should_autoscroll: bool, +} + #[derive(Clone, Debug)] struct AddSelectionsState { above: bool, @@ -9814,47 +9821,30 @@ impl Editor { &mut self, rows: RangeInclusive, color: Option, + should_autoscroll: bool, cx: &mut ViewContext, ) { - let multi_buffer_snapshot = self.buffer().read(cx).snapshot(cx); - match self.highlighted_rows.entry(TypeId::of::()) { - hash_map::Entry::Occupied(o) => { - let row_highlights = o.into_mut(); - let existing_highlight_index = - row_highlights.binary_search_by(|(_, highlight_range, _)| { - highlight_range - .start() - .cmp(&rows.start(), &multi_buffer_snapshot) - .then( - highlight_range - .end() - .cmp(&rows.end(), &multi_buffer_snapshot), - ) - }); - match color { - Some(color) => { - let insert_index = match existing_highlight_index { - Ok(i) => i, - Err(i) => 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)]); + let snapshot = self.buffer().read(cx).snapshot(cx); + let row_highlights = self.highlighted_rows.entry(TypeId::of::()).or_default(); + let existing_highlight_index = row_highlights.binary_search_by(|highlight| { + highlight + .range + .start() + .cmp(&rows.start(), &snapshot) + .then(highlight.range.end().cmp(&rows.end(), &snapshot)) + }); + match (color, existing_highlight_index) { + (Some(_), Ok(ix)) | (_, Err(ix)) => row_highlights.insert( + ix, + RowHighlight { + index: post_inc(&mut self.highlight_order), + range: rows, + should_autoscroll, + color, + }, + ), + (None, Ok(i)) => { + row_highlights.remove(i); } } } @@ -9872,7 +9862,7 @@ impl Editor { self.highlighted_rows .get(&TypeId::of::())? .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, cx: &mut WindowContext, ) -> BTreeMap { 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::::new(), - |mut unique_rows, (highlight_order, anchor_range, hsla)| { - let start_row = anchor_range.start().to_display_point(&snapshot).row(); - let end_row = anchor_range.end().to_display_point(&snapshot).row(); + |mut unique_rows, highlight| { + let start_row = highlight.range.start().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); - if highlight_order >= used_index { - *used_index = *highlight_order; - match hsla { - Some(hsla) => { - unique_rows.insert(DisplayRow(row), *hsla); - } - None => { - unique_rows.remove(&DisplayRow(row)); - } - } + used_highlight_orders.entry(row).or_insert(highlight.index); + if highlight.index >= *used_index { + *used_index = highlight.index; + match highlight.color { + Some(hsla) => unique_rows.insert(DisplayRow(row), hsla), + 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 { + 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], diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 58954c96a4f7d220d249be927027d53c5d188c53..94d8330885e7e5671a34bd5017db48dea1ff0d1b 100644 --- a/crates/editor/src/element.rs +++ b/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| { - editor.highlighted_display_rows(HashSet::default(), cx) - }); + let highlighted_rows = self + .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, diff --git a/crates/editor/src/hunk_diff.rs b/crates/editor/src/hunk_diff.rs index 022fed5835fa3672f4dfab55ed2594c21007c3d0..f9db17ac919e529a2dedd9e73c7f987e0fa05d9b 100644 --- a/crates/editor/src/hunk_diff.rs +++ b/crates/editor/src/hunk_diff.rs @@ -194,6 +194,7 @@ impl Editor { editor.highlight_rows::( to_inclusive_row_range(removed_rows, &snapshot), None, + false, cx, ); } @@ -269,6 +270,7 @@ impl Editor { self.highlight_rows::( 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::( 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::( 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::(start..=end, Some(deleted_color), cx); + editor.highlight_rows::(start..=end, Some(deleted_color), false, cx); let subscription_editor = parent_editor.clone(); editor._subscriptions.extend([ diff --git a/crates/editor/src/scroll/autoscroll.rs b/crates/editor/src/scroll/autoscroll.rs index b24be19ad6ad30329f7ae9b992d3c8385e173d97..5c33532f0c476bf65d27c8667dc45a2fee51cf6b 100644 --- a/crates/editor/src/scroll/autoscroll.rs +++ b/crates/editor/src/scroll/autoscroll.rs @@ -1,13 +1,9 @@ -use std::{any::TypeId, cmp, f32}; - -use collections::HashSet; -use gpui::{px, Bounds, Pixels, ViewContext}; -use language::Point; - use crate::{ - display_map::ToDisplayPoint, DiffRowHighlight, DisplayRow, Editor, EditorMode, - LineWithInvisibles, RowExt, + display_map::ToDisplayPoint, DisplayRow, Editor, EditorMode, LineWithInvisibles, RowExt, }; +use gpui::{px, Bounds, Pixels, ViewContext}; +use language::Point; +use std::{cmp, f32}; #[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 - .highlighted_display_rows( - HashSet::from_iter(Some(TypeId::of::())), - cx, - ) - .first_entry() + if let Some(first_highlighted_row) = + self.highlighted_display_row_for_autoscroll(&display_map) { - 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::(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 { diff --git a/crates/editor/src/test.rs b/crates/editor/src/test.rs index 65fec2ab8ff23ea225b6548ac3d3fd0450561ac3..3a829923c64f41613267a91f5797bb959062848d 100644 --- a/crates/editor/src/test.rs +++ b/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 { diff --git a/crates/go_to_line/Cargo.toml b/crates/go_to_line/Cargo.toml index 9cffedaa01938eececf662d3ca373443f1fc16b9..0e19b41b7566898aa33aa7bba9c85123f6f068af 100644 --- a/crates/go_to_line/Cargo.toml +++ b/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 diff --git a/crates/go_to_line/src/go_to_line.rs b/crates/go_to_line/src/go_to_line.rs index 67cbe6c767bf83cd677f9f498c4a7d8d674e47b2..1376cc70804ba76fb4808fa4eb8fbf318c04dd9c 100644 --- a/crates/go_to_line/src/go_to_line.rs +++ b/crates/go_to_line/src/go_to_line.rs @@ -122,6 +122,7 @@ impl GoToLine { active_editor.highlight_rows::( 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 collections::HashSet; + use super::*; 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, cx: &mut VisualTestContext) -> Vec { editor.update(cx, |editor, cx| { editor - .highlighted_display_rows(HashSet::default(), cx) + .highlighted_display_rows(cx) .into_keys() .map(|r| r.0) .collect() diff --git a/crates/outline/Cargo.toml b/crates/outline/Cargo.toml index 230fb0d358edf7e6be23154d8851ec36a607a62a..6f385f5d8d0821ea9f879793d029ca8eba8bc03c 100644 --- a/crates/outline/Cargo.toml +++ b/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 diff --git a/crates/outline/src/outline.rs b/crates/outline/src/outline.rs index b90e9318764cb78592ba4fcd309dfa0f15fc88a2..8b2e374e3b30b3a2f481f7b727c17ab564847665 100644 --- a/crates/outline/src/outline.rs +++ b/crates/outline/src/outline.rs @@ -144,6 +144,7 @@ impl OutlineViewDelegate { active_editor.highlight_rows::( 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, cx: &mut VisualTestContext) -> Vec { editor.update(cx, |editor, cx| { editor - .highlighted_display_rows(HashSet::default(), cx) + .highlighted_display_rows(cx) .into_keys() .map(|r| r.0) .collect()