Avoid changing selection in buffer navigation dialogs

Max Brunsfeld created

If an editor has highlighted_rows, autoscroll it to reveal those rows instead of
its cursor positions.

Change summary

crates/editor/src/editor.rs         | 46 ++++++++++++++++---------
crates/go_to_line/src/go_to_line.rs | 55 +++++++++++-------------------
crates/outline/src/outline.rs       | 52 ++++++++++-------------------
3 files changed, 68 insertions(+), 85 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -638,7 +638,10 @@ impl Editor {
 
         let first_cursor_top;
         let last_cursor_bottom;
-        if autoscroll == Autoscroll::Newest {
+        if let Some(highlighted_rows) = &self.highlighted_rows {
+            first_cursor_top = highlighted_rows.start as f32;
+            last_cursor_bottom = first_cursor_top + 1.;
+        } else if autoscroll == Autoscroll::Newest {
             let newest_selection = self.newest_selection::<Point>(&display_map.buffer_snapshot);
             first_cursor_top = newest_selection.head().to_display_point(&display_map).row() as f32;
             last_cursor_bottom = first_cursor_top + 1.;
@@ -704,22 +707,33 @@ impl Editor {
     ) -> bool {
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
         let selections = self.local_selections::<Point>(cx);
-        let mut target_left = std::f32::INFINITY;
-        let mut target_right = 0.0_f32;
-        for selection in selections {
-            let head = selection.head().to_display_point(&display_map);
-            if head.row() >= start_row && head.row() < start_row + layouts.len() as u32 {
-                let start_column = head.column().saturating_sub(3);
-                let end_column = cmp::min(display_map.line_len(head.row()), head.column() + 3);
-                target_left = target_left.min(
-                    layouts[(head.row() - start_row) as usize].x_for_index(start_column as usize),
-                );
-                target_right = target_right.max(
-                    layouts[(head.row() - start_row) as usize].x_for_index(end_column as usize)
-                        + max_glyph_width,
-                );
+
+        let mut target_left;
+        let mut target_right;
+
+        if self.highlighted_rows.is_some() {
+            target_left = 0.0_f32;
+            target_right = 0.0_f32;
+        } else {
+            target_left = std::f32::INFINITY;
+            target_right = 0.0_f32;
+            for selection in selections {
+                let head = selection.head().to_display_point(&display_map);
+                if head.row() >= start_row && head.row() < start_row + layouts.len() as u32 {
+                    let start_column = head.column().saturating_sub(3);
+                    let end_column = cmp::min(display_map.line_len(head.row()), head.column() + 3);
+                    target_left = target_left.min(
+                        layouts[(head.row() - start_row) as usize]
+                            .x_for_index(start_column as usize),
+                    );
+                    target_right = target_right.max(
+                        layouts[(head.row() - start_row) as usize].x_for_index(end_column as usize)
+                            + max_glyph_width,
+                    );
+                }
             }
         }
+
         target_right = target_right.min(scroll_width);
 
         if target_right - target_left > viewport_width {
@@ -3400,7 +3414,7 @@ impl Editor {
         });
     }
 
-    fn request_autoscroll(&mut self, autoscroll: Autoscroll, cx: &mut ViewContext<Self>) {
+    pub fn request_autoscroll(&mut self, autoscroll: Autoscroll, cx: &mut ViewContext<Self>) {
         self.autoscroll_request = Some(autoscroll);
         cx.notify();
     }

crates/go_to_line/src/go_to_line.rs 🔗

@@ -1,11 +1,11 @@
-use editor::{display_map::ToDisplayPoint, Autoscroll, Editor, EditorSettings};
+use editor::{display_map::ToDisplayPoint, Autoscroll, DisplayPoint, Editor, EditorSettings};
 use gpui::{
     action, elements::*, geometry::vector::Vector2F, keymap::Binding, Axis, Entity,
     MutableAppContext, RenderContext, View, ViewContext, ViewHandle,
 };
 use postage::watch;
 use std::sync::Arc;
-use text::{Bias, Point, Selection};
+use text::{Bias, Point};
 use workspace::{Settings, Workspace};
 
 action!(Toggle);
@@ -25,17 +25,11 @@ pub struct GoToLine {
     settings: watch::Receiver<Settings>,
     line_editor: ViewHandle<Editor>,
     active_editor: ViewHandle<Editor>,
-    restore_state: Option<RestoreState>,
-    line_selection_id: Option<usize>,
+    prev_scroll_position: Option<Vector2F>,
     cursor_point: Point,
     max_point: Point,
 }
 
-struct RestoreState {
-    scroll_position: Vector2F,
-    selections: Vec<Selection<usize>>,
-}
-
 pub enum Event {
     Dismissed,
 }
@@ -65,15 +59,11 @@ impl GoToLine {
         cx.subscribe(&line_editor, Self::on_line_editor_event)
             .detach();
 
-        let (restore_state, cursor_point, max_point) = active_editor.update(cx, |editor, cx| {
-            let restore_state = Some(RestoreState {
-                scroll_position: editor.scroll_position(cx),
-                selections: editor.local_selections::<usize>(cx),
-            });
-
+        let (scroll_position, cursor_point, max_point) = active_editor.update(cx, |editor, cx| {
+            let scroll_position = editor.scroll_position(cx);
             let buffer = editor.buffer().read(cx).read(cx);
             (
-                restore_state,
+                Some(scroll_position),
                 editor.newest_selection(&buffer).head(),
                 buffer.max_point(),
             )
@@ -83,8 +73,7 @@ impl GoToLine {
             settings: settings.clone(),
             line_editor,
             active_editor,
-            restore_state,
-            line_selection_id: None,
+            prev_scroll_position: scroll_position,
             cursor_point,
             max_point,
         }
@@ -105,7 +94,14 @@ impl GoToLine {
     }
 
     fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext<Self>) {
-        self.restore_state.take();
+        self.prev_scroll_position.take();
+        self.active_editor.update(cx, |active_editor, cx| {
+            if let Some(rows) = active_editor.highlighted_rows() {
+                let snapshot = active_editor.snapshot(cx).display_snapshot;
+                let position = DisplayPoint::new(rows.start, 0).to_point(&snapshot);
+                active_editor.select_ranges([position..position], Some(Autoscroll::Center), cx);
+            }
+        });
         cx.emit(Event::Dismissed);
     }
 
@@ -139,18 +135,13 @@ impl GoToLine {
                         column.map(|column| column.saturating_sub(1)).unwrap_or(0),
                     )
                 }) {
-                    self.line_selection_id = self.active_editor.update(cx, |active_editor, cx| {
+                    self.active_editor.update(cx, |active_editor, cx| {
                         let snapshot = active_editor.snapshot(cx).display_snapshot;
                         let point = snapshot.buffer_snapshot.clip_point(point, Bias::Left);
                         let display_point = point.to_display_point(&snapshot);
                         let row = display_point.row();
-                        active_editor.select_ranges([point..point], Some(Autoscroll::Center), cx);
                         active_editor.set_highlighted_rows(Some(row..row + 1));
-                        Some(
-                            active_editor
-                                .newest_selection::<usize>(&snapshot.buffer_snapshot)
-                                .id,
-                        )
+                        active_editor.request_autoscroll(Autoscroll::Center, cx);
                     });
                     cx.notify();
                 }
@@ -164,17 +155,11 @@ impl Entity for GoToLine {
     type Event = Event;
 
     fn release(&mut self, cx: &mut MutableAppContext) {
-        let line_selection_id = self.line_selection_id.take();
-        let restore_state = self.restore_state.take();
+        let scroll_position = self.prev_scroll_position.take();
         self.active_editor.update(cx, |editor, cx| {
             editor.set_highlighted_rows(None);
-            if let Some((line_selection_id, restore_state)) = line_selection_id.zip(restore_state) {
-                let newest_selection =
-                    editor.newest_selection::<usize>(&editor.buffer().read(cx).read(cx));
-                if line_selection_id == newest_selection.id {
-                    editor.set_scroll_position(restore_state.scroll_position, cx);
-                    editor.update_selections(restore_state.selections, None, cx);
-                }
+            if let Some(scroll_position) = scroll_position {
+                editor.set_scroll_position(scroll_position, cx);
             }
         })
     }

crates/outline/src/outline.rs 🔗

@@ -1,6 +1,6 @@
 use editor::{
-    display_map::ToDisplayPoint, Anchor, AnchorRangeExt, Autoscroll, Editor, EditorSettings,
-    ToPoint,
+    display_map::ToDisplayPoint, Anchor, AnchorRangeExt, Autoscroll, DisplayPoint, Editor,
+    EditorSettings, ToPoint,
 };
 use fuzzy::StringMatch;
 use gpui::{
@@ -12,7 +12,7 @@ use gpui::{
     AppContext, Axis, Entity, MutableAppContext, RenderContext, View, ViewContext, ViewHandle,
     WeakViewHandle,
 };
-use language::{Outline, Selection};
+use language::Outline;
 use ordered_float::OrderedFloat;
 use postage::watch;
 use std::{
@@ -45,19 +45,13 @@ struct OutlineView {
     active_editor: ViewHandle<Editor>,
     outline: Outline<Anchor>,
     selected_match_index: usize,
-    restore_state: Option<RestoreState>,
-    symbol_selection_id: Option<usize>,
+    prev_scroll_position: Option<Vector2F>,
     matches: Vec<StringMatch>,
     query_editor: ViewHandle<Editor>,
     list_state: UniformListState,
     settings: watch::Receiver<Settings>,
 }
 
-struct RestoreState {
-    scroll_position: Vector2F,
-    selections: Vec<Selection<usize>>,
-}
-
 pub enum Event {
     Dismissed,
 }
@@ -132,20 +126,12 @@ impl OutlineView {
         cx.subscribe(&query_editor, Self::on_query_editor_event)
             .detach();
 
-        let restore_state = editor.update(cx, |editor, cx| {
-            Some(RestoreState {
-                scroll_position: editor.scroll_position(cx),
-                selections: editor.local_selections::<usize>(cx),
-            })
-        });
-
         let mut this = Self {
             handle: cx.weak_handle(),
-            active_editor: editor,
             matches: Default::default(),
             selected_match_index: 0,
-            restore_state,
-            symbol_selection_id: None,
+            prev_scroll_position: Some(editor.update(cx, |editor, cx| editor.scroll_position(cx))),
+            active_editor: editor,
             outline,
             query_editor,
             list_state: Default::default(),
@@ -207,39 +193,37 @@ impl OutlineView {
         if navigate {
             let selected_match = &self.matches[self.selected_match_index];
             let outline_item = &self.outline.items[selected_match.candidate_id];
-            self.symbol_selection_id = self.active_editor.update(cx, |active_editor, cx| {
+            self.active_editor.update(cx, |active_editor, cx| {
                 let snapshot = active_editor.snapshot(cx).display_snapshot;
                 let buffer_snapshot = &snapshot.buffer_snapshot;
                 let start = outline_item.range.start.to_point(&buffer_snapshot);
                 let end = outline_item.range.end.to_point(&buffer_snapshot);
                 let display_rows = start.to_display_point(&snapshot).row()
                     ..end.to_display_point(&snapshot).row() + 1;
-                active_editor.select_ranges([start..start], Some(Autoscroll::Center), cx);
                 active_editor.set_highlighted_rows(Some(display_rows));
-                Some(active_editor.newest_selection::<usize>(&buffer_snapshot).id)
+                active_editor.request_autoscroll(Autoscroll::Center, cx);
             });
         }
         cx.notify();
     }
 
     fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext<Self>) {
-        self.restore_state.take();
+        self.prev_scroll_position.take();
+        self.active_editor.update(cx, |active_editor, cx| {
+            if let Some(rows) = active_editor.highlighted_rows() {
+                let snapshot = active_editor.snapshot(cx).display_snapshot;
+                let position = DisplayPoint::new(rows.start, 0).to_point(&snapshot);
+                active_editor.select_ranges([position..position], Some(Autoscroll::Center), cx);
+            }
+        });
         cx.emit(Event::Dismissed);
     }
 
     fn restore_active_editor(&mut self, cx: &mut MutableAppContext) {
-        let symbol_selection_id = self.symbol_selection_id.take();
         self.active_editor.update(cx, |editor, cx| {
             editor.set_highlighted_rows(None);
-            if let Some((symbol_selection_id, restore_state)) =
-                symbol_selection_id.zip(self.restore_state.as_ref())
-            {
-                let newest_selection =
-                    editor.newest_selection::<usize>(&editor.buffer().read(cx).read(cx));
-                if symbol_selection_id == newest_selection.id {
-                    editor.set_scroll_position(restore_state.scroll_position, cx);
-                    editor.update_selections(restore_state.selections.clone(), None, cx);
-                }
+            if let Some(scroll_position) = self.prev_scroll_position {
+                editor.set_scroll_position(scroll_position, cx);
             }
         })
     }